From e0e27e20e486bf5b6affc44352832f6741bd043d Mon Sep 17 00:00:00 2001 From: Tom Tucker Date: Thu, 13 Jul 2023 13:33:30 -0500 Subject: [PATCH] Change munge authentication message The munge authentication message previously consisted of the local and remote addresses of the transport between the two peers. This message presumed that both peers see the same addresses (albeit swapped). This assumption is false in many routed and NAT'd environments. This patch changes the message to "MungeAuthentication" which is validated by the receiving peer. In order to support backward compatability with older plugins, an option 'compat' has been added. This option, when set, will use the previous message format so that newer aggregators can connect to older samplers. The receive path supports either, specifically, if the new message doesn't match it tries the old format. --- .github/workflows/4.3.3-compat-test.yaml | 20 ++--- ldms/man/ldms_auth_munge.man | 19 ++-- ldms/src/auth/ldms_auth_munge.c | 105 +++++++++++++---------- ldms/src/auth/ldms_auth_naive.c | 2 +- ldms/src/auth/ldms_auth_ovis.c | 2 +- 5 files changed, 85 insertions(+), 63 deletions(-) diff --git a/.github/workflows/4.3.3-compat-test.yaml b/.github/workflows/4.3.3-compat-test.yaml index d247f3fac..e958c6a98 100644 --- a/.github/workflows/4.3.3-compat-test.yaml +++ b/.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}" diff --git a/ldms/man/ldms_auth_munge.man b/ldms/man/ldms_auth_munge.man index cba74a975..d85eadb26 100644 --- a/ldms/man/ldms_auth_munge.man +++ b/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) diff --git a/ldms/src/auth/ldms_auth_munge.c b/ldms/src/auth/ldms_auth_munge.c index 6bc2c1291..b409238b3 100644 --- a/ldms/src/auth/ldms_auth_munge.c +++ b/ldms/src/auth/ldms_auth_munge.c @@ -101,6 +101,7 @@ struct ldms_auth_munge { struct ldms_auth base; munge_ctx_t mctx; char *local_cred; + int compat; struct sockaddr_storage lsin; struct sockaddr_storage rsin; socklen_t sin_len; @@ -110,7 +111,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 +129,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)); @@ -138,6 +139,10 @@ ldms_auth_t __auth_munge_new(ldms_auth_plugin_t plugin, } } + value = av_value(av_list, "compat"); + if (value) + a->compat = atoi(value); + /* Test munge connection */ merr = munge_encode(&test_cred, mctx, NULL, 0); if (merr != EMUNGE_SUCCESS) { @@ -196,6 +201,8 @@ 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) { @@ -203,26 +210,35 @@ int __auth_munge_xprt_begin(ldms_auth_t auth, ldms_t xprt) 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); - if (rc) { - LOG_ERROR("Failed to get the socket addresses. Error %d\n", rc); - return rc; - } - /* - * zap_rdma from OVIS-4.3.7 and earlier has a bug that swaps - * local/remote addresses. Since the old peers expect to receive the - * swapped address, in order to be compatible with them, we have to send - * the swapped address in the case of rdma transport. - */ - 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 (a->compat) { + /* + * This uses the older payload of the local/remote sin + * to be compatabile with OVIS 4.3.11 and earlier + */ + a->sin_len = sizeof(a->lsin); + rc = ldms_xprt_sockaddr(xprt, (void*)&a->lsin, + (void*)&a->rsin, &a->sin_len); + if (rc) { + LOG_ERROR("Failed to get the socket addresses. Error %d\n", rc); + return rc; + } + /* + * zap_rdma from OVIS-4.3.7 and earlier has a bug that swaps + * local/remote addresses. Since the old peers expect to receive the + * swapped address, in order to be compatible with them, we have to send + * the swapped address in the case of rdma transport. + */ + 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); + } else { + merr = munge_encode(&a->local_cred, a->mctx, + MUNGE_AUTH_MSG, sizeof(MUNGE_AUTH_MSG)); + } 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); @@ -242,22 +258,24 @@ int __auth_munge_xprt_recv_cb(ldms_auth_t auth, ldms_t xprt, 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; + goto out; } - if (len != sizeof(*sin)) { - LOG_ERROR("Bad payload\n"); - goto invalid; /* bad payload */ + + /* Check the authentication message */ + if (0 == strcmp(payload, MUNGE_AUTH_MSG)) { + rc = 0; + goto out; } - /* check if addr match */ + /* Check the expected peer address (compatability mode) */ sin = payload; /* * zap_rdma from OVIS-4.3.7 and earlier has a bug that swaps @@ -266,24 +284,21 @@ int __auth_munge_xprt_recv_cb(ldms_auth_t auth, ldms_t xprt, * 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)); + memcmp(sin, &a->lsin, sizeof(*sin)): + memcmp(sin, &a->rsin, sizeof(*sin)); if (cmp != 0) { - LOG_ERROR("bad address.\n"); - goto invalid; /* bad addr */ + LOG_ERROR("Unexpected authentication message payload.\n"); + goto out; + } + 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 +325,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. " diff --git a/ldms/src/auth/ldms_auth_naive.c b/ldms/src/auth/ldms_auth_naive.c index 3429f5644..468a82e9d 100644 --- a/ldms/src/auth/ldms_auth_naive.c +++ b/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"); } diff --git a/ldms/src/auth/ldms_auth_ovis.c b/ldms/src/auth/ldms_auth_ovis.c index b1bae1b9b..52aeb4445 100644 --- a/ldms/src/auth/ldms_auth_ovis.c +++ b/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);