From: Tim Wickberg <tim@schedmd.com>
Date: Wed Nov 29 14:17:31 2023 -0700
Subject: [PATCH 15/28]Prevent modified sbcast RPCs from opening a file with the wrong group permissions.
Patch-mainline: Upstream
Git-repo: https://github.com/SchedMD/slurm
Git-commit: a33c6e1f6c188b880c926f60f60d718d3cf05822
References: bsc#1218046, bsc#1218050, bsc#1218051, bsc#1218053
Signed-off-by: Egbert Eich <eich@suse.de>

CVE-2023-49938.

Signed-off-by: Egbert Eich <eich@suse.com>
---
 NEWS                                |  2 +
 src/common/slurm_cred.c             | 79 +++----------------------------------
 src/plugins/cred/munge/cred_munge.c | 22 ++++++-----
 src/plugins/cred/none/cred_none.c   |  3 +-
 4 files changed, 22 insertions(+), 84 deletions(-)

diff --git a/NEWS b/NEWS
index 908cca0ca3..e7cf8d91b5 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ documents those changes that are of interest to users and administrators.
     CVE-2023-49933.
  -- Prevent NULL pointer dereference on size_valp overflow. CVE-2023-49936.
  -- Prevent double-xfree() on error in _unpack_node_reg_resp(). CVE-2023-49937.
+ -- Prevent modified sbcast RPCs from opening a file with the wrong group
+    permissions. CVE-2023-49938.
  -- Fix filesystem handling race conditions that could lead to an attacker
     taking control of an arbitrary file, or removing entire directories'
     contents. CVE-2023-41914.
diff --git a/src/common/slurm_cred.c b/src/common/slurm_cred.c
index 31ef7d8c10..5e26f260c3 100644
--- a/src/common/slurm_cred.c
+++ b/src/common/slurm_cred.c
@@ -213,7 +213,8 @@ typedef struct {
 	int   (*cred_verify_sign)	(void *key, char *buffer,
 					 uint32_t buf_size,
 					 char *signature,
-					 uint32_t sig_size);
+					 uint32_t sig_size,
+					 bool replay_okay);
 	const char *(*cred_str_error)	(int);
 } slurm_cred_ops_t;
 
@@ -292,8 +293,6 @@ static void _cred_state_pack(slurm_cred_ctx_t ctx, Buf buffer);
 static void _job_state_pack_one(job_state_t *j, Buf buffer);
 static void _cred_state_pack_one(cred_state_t *s, Buf buffer);
 
-static void _sbast_cache_add(sbcast_cred_t *sbcast_cred);
-
 static int _slurm_cred_init(void)
 {
 	char *tok;
@@ -1729,13 +1728,13 @@ _slurm_cred_verify_signature(slurm_cred_ctx_t ctx, slurm_cred_t *cred,
 				       get_buf_data(buffer),
 				       get_buf_offset(buffer),
 				       cred->signature,
-				       cred->siglen);
+				       cred->siglen, false);
 	if (rc && _exkey_is_valid(ctx)) {
 		rc = (*(ops.cred_verify_sign))(ctx->exkey,
 					       get_buf_data(buffer),
 					       get_buf_offset(buffer),
 					       cred->signature,
-					       cred->siglen);
+					       cred->siglen, false);
 	}
 	free_buf(buffer);
 
@@ -2323,25 +2322,6 @@ void delete_sbcast_cred(sbcast_cred_t *sbcast_cred)
 	xfree(sbcast_cred);
 }
 
-static void _sbast_cache_add(sbcast_cred_t *sbcast_cred)
-{
-	int i;
-	uint32_t sig_num = 0;
-	struct sbcast_cache *new_cache_rec;
-
-	/* Using two bytes at a time gives us a larger number
-	 * and reduces the possibility of a duplicate value */
-	for (i = 0; i < sbcast_cred->siglen; i += 2) {
-		sig_num += (sbcast_cred->signature[i] << 8) +
-			   sbcast_cred->signature[i+1];
-	}
-
-	new_cache_rec = xmalloc(sizeof(struct sbcast_cache));
-	new_cache_rec->expire = sbcast_cred->expiration;
-	new_cache_rec->value  = sig_num;
-	list_append(sbcast_cache_list, new_cache_rec);
-}
-
 /* Extract contents of an sbcast credential verifying the digital signature.
  * NOTE: We can only perform the full credential validation once with
  *	Munge without generating a credential replay error, so we only
@@ -2355,9 +2335,7 @@ sbcast_cred_arg_t *extract_sbcast_cred(slurm_cred_ctx_t ctx,
 				       uint16_t protocol_version)
 {
 	sbcast_cred_arg_t *arg;
-	struct sbcast_cache *next_cache_rec;
-	uint32_t sig_num = 0;
-	int i, rc;
+	int rc;
 	time_t now = time(NULL);
 	Buf buffer;
 
@@ -2376,7 +2354,7 @@ sbcast_cred_arg_t *extract_sbcast_cred(slurm_cred_ctx_t ctx,
 		 * created by SlurmUser or root */
 		rc = (*(ops.cred_verify_sign)) (
 			ctx->key, get_buf_data(buffer), get_buf_offset(buffer),
-			sbcast_cred->signature, sbcast_cred->siglen);
+			sbcast_cred->signature, sbcast_cred->siglen, true);
 		free_buf(buffer);
 
 		if (rc) {
@@ -2384,51 +2362,6 @@ sbcast_cred_arg_t *extract_sbcast_cred(slurm_cred_ctx_t ctx,
 			      (*(ops.cred_str_error))(rc));
 			return NULL;
 		}
-		_sbast_cache_add(sbcast_cred);
-
-	} else {
-		char *err_str = NULL;
-		bool cache_match_found = false;
-		ListIterator sbcast_iter;
-		for (i = 0; i < sbcast_cred->siglen; i += 2) {
-			sig_num += (sbcast_cred->signature[i] << 8) +
-				   sbcast_cred->signature[i+1];
-		}
-
-		sbcast_iter = list_iterator_create(sbcast_cache_list);
-		while ((next_cache_rec = 
-			(struct sbcast_cache *) list_next(sbcast_iter))) {
-			if ((next_cache_rec->expire == sbcast_cred->expiration) &&
-			    (next_cache_rec->value  == sig_num)) {
-				cache_match_found = true;
-				break;
-			}
-			if (next_cache_rec->expire <= now)
-				list_delete_item(sbcast_iter);
-		}
-		list_iterator_destroy(sbcast_iter);
-
-		if (!cache_match_found) {
-			error("sbcast_cred verify: signature not in cache");
-			if (SLURM_DIFFTIME(now, cred_restart_time) > 60)
-				return NULL;	/* restarted >60 secs ago */
-			buffer = init_buf(4096);
-			_pack_sbcast_cred(sbcast_cred, buffer,
-					  protocol_version);
-			rc = (*(ops.cred_verify_sign)) (
-				ctx->key, get_buf_data(buffer),
-				get_buf_offset(buffer),
-				sbcast_cred->signature, sbcast_cred->siglen);
-			free_buf(buffer);
-			if (rc)
-				err_str = (char *)(*(ops.cred_str_error))(rc);
-			if (err_str && xstrcmp(err_str, "Credential replayed")){
-				error("sbcast_cred verify: %s", err_str);
-				return NULL;
-			}
-			info("sbcast_cred verify: signature revalidated");
-			_sbast_cache_add(sbcast_cred);
-		}
 	}
 
 	arg = xmalloc(sizeof(sbcast_cred_arg_t));
diff --git a/src/plugins/cred/munge/cred_munge.c b/src/plugins/cred/munge/cred_munge.c
index e7d052eddc..1c2b3f174a 100644
--- a/src/plugins/cred/munge/cred_munge.c
+++ b/src/plugins/cred/munge/cred_munge.c
@@ -234,7 +234,8 @@ again:
 }
 
 extern int cred_p_verify_sign(void *key, char *buffer, uint32_t buf_size,
-			      char *signature, uint32_t sig_size)
+			      char *signature, uint32_t sig_size,
+			      bool replay_okay)
 {
 	int retry = RETRY_COUNT;
 	uid_t uid;
@@ -245,6 +246,10 @@ extern int cred_p_verify_sign(void *key, char *buffer, uint32_t buf_size,
 	munge_err_t err;
 	munge_ctx_t ctx = (munge_ctx_t) key;
 
+#ifdef MULTIPLE_SLURMD
+	replay_okay = true;
+#endif
+
 again:
 	err = munge_decode(signature, ctx, &buf_out, &buf_out_size,
 			   &uid, &gid);
@@ -259,20 +264,17 @@ again:
 		if (err == EMUNGE_SOCKET)
 			error("If munged is up, restart with --num-threads=10");
 
-#ifdef MULTIPLE_SLURMD
 		if (err != EMUNGE_CRED_REPLAYED) {
 			rc = err;
 			goto end_it;
-		} else {
-			debug2("We had a replayed credential, but this is expected in multiple slurmd mode.");
 		}
-#else
-		if (err == EMUNGE_CRED_REPLAYED)
+
+		if (!replay_okay) {
 			rc = ESIG_CRED_REPLAYED;
-		else
-			rc = err;
-		goto end_it;
-#endif
+			goto end_it;
+		}
+
+		debug2("We had a replayed credential, but this is expected.");
 	}
 
 	if ((uid != slurm_conf.slurm_user_id) && (uid != 0)) {
diff --git a/src/plugins/cred/none/cred_none.c b/src/plugins/cred/none/cred_none.c
index e89ec17822..1ee42b1ef1 100644
--- a/src/plugins/cred/none/cred_none.c
+++ b/src/plugins/cred/none/cred_none.c
@@ -129,7 +129,8 @@ extern int cred_p_sign(void *key, char *buffer, int buf_size,
 }
 
 extern int cred_p_verify_sign(void *key, char *buffer, uint32_t buf_size,
-			      char *signature, uint32_t sig_size)
+			      char *signature, uint32_t sig_size,
+			      bool replay_okay)
 {
 	char *correct_signature = "fake signature";
 	if (xstrncmp(signature, correct_signature, sig_size))
