Change munge error message to info

The munge authentication message consists of the
local and remote addresses of the transport between
the two peers. This message presumes that both peers see
the same addresses (albeit swapped). This assumption is
false in many routed and NAT'd environments.

This change replaces the error with an informational message
so that administrators will be aware of this difference
and can make a determination whether or not this difference
is expected.

.github/workflows/4.3.3-compat-test.yaml

@@ -153,12 +153,12 @@ jobs:
         echo -n "ldms_ls agg-4 ... "
         D1=$( ./ldms_ls.sh -x sock -p 10002 )
         echo "result: ${D1}"
-        [[ "${D0}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls agg-4.3.3 result"
-        [[ "${D1}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls agg-4 result"
         # ldms_ls-4.3.3 to agg-4
         echo -n "ldms_ls(4.3.3) agg-4 ... "
         D2=$( ./ldms_ls-4.3.3.sh -x sock -p 10002 )
         echo "result: ${D2}"
+        [[ "${D0}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls agg-4.3.3 result"
+        [[ "${D1}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls agg-4 result"
         [[ "${D2}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls(4.3.3) agg-4 result"
         # ldms_ls-4 to agg-4.3.3 -l
         echo -n "ldms_ls -l agg-4.3.3 ... "
@@ -247,7 +247,7 @@ jobs:
         sleep 10
         # agg2
         echo "starting agg-4 w/munge"
-        ./ldmsd-4.sh -c agg-4.conf -l logs/agg-4.log -v INFO -x sock:10002 -a munge
+        ./ldmsd-4.sh -c agg-4.conf -l logs/agg-4.log -v INFO -x sock:10002 -a munge -A compat=1
         sleep 10
         # check that they're running
         SAMP_433_PID=$(pgrep -f samp-4.3.3.conf)
@@ -259,11 +259,11 @@ jobs:
         [[ -n "${SAMP_433_PID}" ]] || error "samp-4.3.3 is not running"
         [[ -n "${AGG_433_PID}" ]] || error "agg-4.3.3 is not running"
         [[ -n "${AGG_4_PID}" ]] || error "agg-4 is not running"
-        # ldms_ls-4 to agg-4.3.3
-        echo -n "ldms_ls agg-4.3.3 -a munge ... "
-        D0=$( ./ldms_ls.sh -x sock -p 10001 -a munge )
+        # ldms_ls-4 to agg-4.3.3 (requires compat flag)
+        echo -n "ldms_ls agg-4.3.3 -a munge -A compat=1 ... "
+        D0=$( ./ldms_ls.sh -x sock -p 10001 -a munge -A compat=1 )
         echo "result: ${D0}"
-        # ldms_ls-4 to agg-4
+        # ldms_ls-4 to agg-4 (does not require compat flag)
         echo -n "ldms_ls agg-4 -a munge ... "
         D1=$( ./ldms_ls.sh -x sock -p 10002 -a munge )
         echo "result: ${D1}"
@@ -274,12 +274,12 @@ jobs:
         D2=$( ./ldms_ls-4.3.3.sh -x sock -p 10002 -a munge )
         echo "result: ${D2}"
         [[ "${D2}" = "ovis-4.3.3/meminfo" ]] || error "unexpected ldms_ls(4.3.3) agg-4 result"
-        # ldms_ls-4 to agg-4.3.3 -l
+        # ldms_ls-4 to agg-4.3.3 -l (requires compat flag)
         echo -n "ldms_ls -l agg-4.3.3 -a munge ... "
-        D0=$( ./ldms_ls.sh -l -x sock -p 10001 -a munge | grep MemTotal )
+        D0=$( ./ldms_ls.sh -l -x sock -p 10001 -a munge -A compat=1 | grep MemTotal )
         echo "${D0}"
         [[ -n "${D0}" ]] || error "cannot get MemTotal from ldms_ls -l"
-        # ldms_ls-4 to agg-4 -l
+        # ldms_ls-4 to agg-4 -l (does not require compat flag)
         echo -n "ldms_ls -l agg-4 -a munge ... "
         D1=$( ./ldms_ls.sh -l -x sock -p 10002 -a munge | grep MemTotal )
         echo "${D1}"

ldms/man/ldms_auth_munge.man

@@ -10,15 +10,22 @@ ldms_auth_munge \- LDMS authentication using munge
 .SH SYNOPSIS
 .HP
 .I ldms_app
-.BI "-a munge [-A socket=" PATH ]
+.BI "-a munge [-A socket=PATH,compat=1 ]"
 
 
 .SH DESCRIPTION
-\fBldms_auth_munge\fR relies on \fBmunge\fR service (see \fBmunge\fR(7)) to
-authenticate users. Munge daemon (\fBmunged\fR) must be up and running. The
-optional \fBsocket\fR option can be used to specify the path to munged unix
-domain socket in the case that munged wasn't using the default path.
-
+\fBldms_auth_munge\fR relies on the \fBmunge\fR service (see \fBmunge\fR(7)) to
+authenticate users. The munge daemon (\fBmunged\fR) must be up and running.
+
+The optional \fBsocket\fR option can be used to specify the path to
+the munged unix domain socket in the case that munged wasn't using the
+default path or there are multiple munge domains configured.
+
+The optional \fBcompat\fR option is used to be backward compatible
+with older versions of the auth plugin that sent authentication
+messages that assumed that the remote and local IP addresses would be
+the same on both sides of the connection. This is not true in many
+environments.
 
 .SH SEE ALSO
 \fBmunge\fR(7), \fBmunged\fR(8)

ldms/src/auth/ldms_auth_munge.c

@@ -51,6 +51,9 @@
 
 #include <munge.h>
 #include <assert.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
 
 #include "ovis_log/ovis_log.h"
 #include "../core/ldms_auth.h"
@@ -69,6 +72,10 @@ static ovis_log_t munge_log = NULL;
 	ovis_log(munge_log, OVIS_LERROR, _fmt_, ##__VA_ARGS__); \
 } while (0);
 
+#define LOG_INFO(_fmt_, ...) do { \
+	ovis_log(munge_log, OVIS_LINFO, _fmt_, ##__VA_ARGS__); \
+} while (0);
+
 static
 ldms_auth_t __auth_munge_new(ldms_auth_plugin_t plugin,
 			     struct attr_value_list *av_list);
@@ -110,7 +117,7 @@ static
 ldms_auth_t __auth_munge_new(ldms_auth_plugin_t plugin,
 		       struct attr_value_list *av_list)
 {
-	const char *munge_sock;
+	const char *value;
 	struct ldms_auth_munge *a;
 	munge_ctx_t mctx;
 	munge_err_t merr;
@@ -128,9 +135,9 @@ ldms_auth_t __auth_munge_new(ldms_auth_plugin_t plugin,
 		goto err1;
 	}
 
-	munge_sock = av_value(av_list, "socket");
-	if (munge_sock) {
-		merr = munge_ctx_set(mctx, MUNGE_OPT_SOCKET, munge_sock);
+	value = av_value(av_list, "socket");
+	if (value) {
+		merr = munge_ctx_set(mctx, MUNGE_OPT_SOCKET, value);
 		if (merr != EMUNGE_SUCCESS) {
 			LOG_ERROR("Failed to set MUNGE context. %s\n",
 				   munge_strerror(merr));
@@ -196,13 +203,16 @@ int __auth_munge_xprt_bind(ldms_auth_t auth, ldms_t xprt)
 	return 0;
 }
 
+#define MUNGE_AUTH_MSG "MungeAuthenticate"
+
 static
 int __auth_munge_xprt_begin(ldms_auth_t auth, ldms_t xprt)
 {
 	struct ldms_auth_munge *a = (void*)auth;
 	munge_err_t merr;
 	int rc;
 	int len;
+
 	a->sin_len = sizeof(a->lsin);
 	rc = ldms_xprt_sockaddr(xprt, (void*)&a->lsin,
 				(void*)&a->rsin, &a->sin_len);
@@ -219,10 +229,9 @@ int __auth_munge_xprt_begin(ldms_auth_t auth, ldms_t xprt)
 	merr = (strncmp(xprt->name, "rdma", 4) == 0)?
 		munge_encode(&a->local_cred, a->mctx, &a->rsin, a->sin_len):
 		munge_encode(&a->local_cred, a->mctx, &a->lsin, a->sin_len);
-
 	if (merr) {
 		LOG_ERROR("munge_encode() failed. %s\n", munge_strerror(merr));
-		return EBADR; /* bad request */
+		return EBADR;
 	}
 	len = strlen(a->local_cred);
 	rc = ldms_xprt_auth_send(xprt, a->local_cred, len + 1);
@@ -238,52 +247,51 @@ int __auth_munge_xprt_recv_cb(ldms_auth_t auth, ldms_t xprt,
 		const char *data, uint32_t data_len)
 {
 	struct ldms_auth_munge *a = (void*)auth;
-	struct sockaddr_in *sin;
+	struct sockaddr_in *payload_sin;
+	struct sockaddr_in *xprt_sin;
 	void *payload = NULL;
 	uid_t uid;
 	gid_t gid;
+	munge_err_t merr;
 	int len;
 	int cmp;
-	munge_err_t merr;
-	if (data[data_len-1] != 0)
-		goto invalid;
+	int rc = EINVAL;
+
 	merr = munge_decode(data, a->mctx, &payload, &len, &uid, &gid);
 	if (merr != EMUNGE_SUCCESS) {
 		LOG_ERROR("munge_decode() failed. %s\n", munge_strerror(merr));
-		goto invalid;
-	}
-	if (len != sizeof(*sin)) {
-		LOG_ERROR("Bad payload\n");
-		goto invalid; /* bad payload */
+		goto out;
 	}
 
-	/* check if addr match */
-	sin = payload;
+	/* Check the expected peer address (compatability mode) */
+	payload_sin = payload;
 	/*
 	 * zap_rdma from OVIS-4.3.7 and earlier has a bug that swaps
 	 * local/remote addresses. Since the old peers send the swapped
 	 * address, in order to be compatible with them, we have to expect the
 	 * swapped address in the case of rdma transport.
 	 */
-	cmp = (strncmp(xprt->name, "rdma", 4) == 0)?
-			memcmp(sin, &a->lsin, sizeof(*sin)):
-			memcmp(sin, &a->rsin, sizeof(*sin));
+	xprt_sin = (strncmp(xprt->name, "rdma", 4) == 0 ?
+		    (struct sockaddr_in *)&a->lsin :
+		    (struct sockaddr_in *)&a->rsin);
+	cmp = memcmp(payload_sin, xprt_sin, sizeof(*payload_sin));
 	if (cmp != 0) {
-		LOG_ERROR("bad address.\n");
-		goto invalid; /* bad addr */
+		char ipa[16];
+		char ipb[16];
+		strcpy(ipa, inet_ntoa(payload_sin->sin_addr));
+		strcpy(ipb, inet_ntoa(xprt_sin->sin_addr));
+		LOG_INFO("Unexpected authentication message payload "
+			  "'%s' != '%s'.\n", ipa, ipb);
+	}
+	rc = 0;
+ out:
+	/* Cache the peer's verified uid and gid in the transport handle. */
+	if (!rc) {
+		xprt->ruid = uid;
+		xprt->rgid = gid;
 	}
-	/* verified */
-	xprt->ruid = uid;
-	xprt->rgid = gid;
-
 	free(payload);
-	ldms_xprt_auth_end(xprt, 0);
-	return 0;
-
-invalid:
-	if (payload)
-		free(payload);
-	ldms_xprt_auth_end(xprt, EINVAL);
+	ldms_xprt_auth_end(xprt, rc);
 	return 0;
 }
 
@@ -310,7 +318,7 @@ int __auth_munge_cred_get(ldms_auth_t auth, ldms_cred_t cred)
 ldms_auth_plugin_t __ldms_auth_plugin_get()
 {
 	if (!munge_log) {
-		munge_log = ovis_log_register("auth_munge",
+		munge_log = ovis_log_register("auth.munge",
 					      "Messages for ldms_auth_munge");
 		if (!munge_log) {
 			LOG_ERROR("Failed to register auth_munge's log. "

ldms/src/auth/ldms_auth_naive.c

@@ -102,7 +102,7 @@ struct ldms_auth_naive {
 ldms_auth_plugin_t __ldms_auth_plugin_get()
 {
 	if (!nlog) {
-		nlog = ovis_log_register("auth_naive", "Messages for auth_naive");
+		nlog = ovis_log_register("auth.naive", "Messages for auth_naive");
 		if (!nlog) {
 			LOG_ERROR("Failed to register the auth_naive log.\n");
 		}

ldms/src/auth/ldms_auth_ovis.c

@@ -119,7 +119,7 @@ struct ldms_auth_ovis {
 ldms_auth_plugin_t __ldms_auth_plugin_get()
 {
 	if (!aolog) {
-		aolog = ovis_log_register("auth_ovis", "Messages for ldms_auth_ovis");
+		aolog = ovis_log_register("auth.ovis", "Messages for ldms_auth_ovis");
 		if (!aolog) {
 			LOG_ERROR("Failed to register %s's log. Error %d\n",
 					__FILE__, errno);