From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 30 Sep 2021 22:38:16 +0000 (+1300)
Subject: CVE-2020-25718 s4-rpc_server: Put RODC reveal/never reveal logic into a single helper... 
X-Git-Tag: samba-4.13.14~52
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=43f321dce53fbc7865933041ba3c877b9ee5cb6c;p=thirdparty%2Fsamba.git

CVE-2020-25718 s4-rpc_server: Put RODC reveal/never reveal logic into a single helper function

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14558

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
---

diff --git a/source4/rpc_server/common/sid_helper.c b/source4/rpc_server/common/sid_helper.c
index 65d7e7c7271..eaeab236fc0 100644
--- a/source4/rpc_server/common/sid_helper.c
+++ b/source4/rpc_server/common/sid_helper.c
@@ -130,3 +130,52 @@ WERROR samdb_result_sid_array_dn(struct ldb_context *sam_ctx,
 
 	return WERR_OK;
 }
+
+WERROR samdb_confirm_rodc_allowed_to_repl_to_sid_list(struct ldb_context *sam_ctx,
+						      struct ldb_message *rodc_msg,
+						      uint32_t num_token_sids,
+						      struct dom_sid *token_sids)
+{
+	uint32_t num_never_reveal_sids, num_reveal_sids;
+	struct dom_sid *never_reveal_sids, *reveal_sids;
+	TALLOC_CTX *frame = talloc_stackframe();
+	WERROR werr = samdb_result_sid_array_dn(sam_ctx, rodc_msg,
+						frame, "msDS-NeverRevealGroup",
+						&num_never_reveal_sids,
+						&never_reveal_sids);
+	if (!W_ERROR_IS_OK(werr)) {
+		TALLOC_FREE(frame);
+		return WERR_DS_DRA_SECRETS_DENIED;
+	}
+
+	werr = samdb_result_sid_array_dn(sam_ctx, rodc_msg,
+					 frame, "msDS-RevealOnDemandGroup",
+					 &num_reveal_sids,
+					 &reveal_sids);
+	if (!W_ERROR_IS_OK(werr)) {
+		TALLOC_FREE(frame);
+		return WERR_DS_DRA_SECRETS_DENIED;
+	}
+
+	if (never_reveal_sids &&
+	    sid_list_match(num_token_sids,
+			   token_sids,
+			   num_never_reveal_sids,
+			   never_reveal_sids)) {
+		TALLOC_FREE(frame);
+		return WERR_DS_DRA_SECRETS_DENIED;
+	}
+
+	if (reveal_sids &&
+	    sid_list_match(num_token_sids,
+			   token_sids,
+			   num_reveal_sids,
+			   reveal_sids)) {
+		TALLOC_FREE(frame);
+		return WERR_OK;
+	}
+
+	TALLOC_FREE(frame);
+	return WERR_DS_DRA_SECRETS_DENIED;
+
+}
diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index bc30e73e06b..3b1d674573f 100644
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -1171,8 +1171,8 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state,
 	const char *rodc_attrs[] = { "msDS-KrbTgtLink", "msDS-NeverRevealGroup", "msDS-RevealOnDemandGroup", "objectGUID", NULL };
 	const char *obj_attrs[] = { "tokenGroups", "objectSid", "UserAccountControl", "msDS-KrbTgtLinkBL", NULL };
 	struct ldb_result *rodc_res = NULL, *obj_res = NULL;
-	uint32_t num_never_reveal_sids, num_reveal_sids, num_token_sids;
-	struct dom_sid *never_reveal_sids, *reveal_sids, *token_sids;
+	uint32_t num_token_sids;
+	struct dom_sid *token_sids;
 	const struct dom_sid *object_sid = NULL;
 	WERROR werr;
 
@@ -1308,35 +1308,12 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state,
 		goto denied;
 	}
 
-	werr = samdb_result_sid_array_dn(b_state->sam_ctx_system, rodc_res->msgs[0],
-					 mem_ctx, "msDS-NeverRevealGroup",
-					 &num_never_reveal_sids,
-					 &never_reveal_sids);
-	if (!W_ERROR_IS_OK(werr)) {
-		goto denied;
-	}
-
-	werr = samdb_result_sid_array_dn(b_state->sam_ctx_system, rodc_res->msgs[0],
-					 mem_ctx, "msDS-RevealOnDemandGroup",
-					 &num_reveal_sids,
-					 &reveal_sids);
-	if (!W_ERROR_IS_OK(werr)) {
-		goto denied;
-	}
-
-	if (never_reveal_sids &&
-	    sid_list_match(num_token_sids,
-			   token_sids,
-			   num_never_reveal_sids,
-			   never_reveal_sids)) {
-		goto denied;
-	}
+	werr = samdb_confirm_rodc_allowed_to_repl_to_sid_list(b_state->sam_ctx_system,
+							      rodc_res->msgs[0],
+							      num_token_sids,
+							      token_sids);
 
-	if (reveal_sids &&
-	    sid_list_match(num_token_sids,
-			   token_sids,
-			   num_reveal_sids,
-			   reveal_sids)) {
+	if (W_ERROR_IS_OK(werr)) {
 		goto allowed;
 	}
 
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 51c6666a164..1aecd65bb61 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2852,8 +2852,8 @@ static bool sam_rodc_access_check(struct ldb_context *sam_ctx,
 	struct ldb_result *rodc_res = NULL, *obj_res = NULL;
 	WERROR werr;
 	struct dom_sid *object_sid;
-	uint32_t num_never_reveal_sids, num_reveal_sids, num_token_sids;
-	struct dom_sid *never_reveal_sids, *reveal_sids, *token_sids;
+	uint32_t num_token_sids;
+	struct dom_sid *token_sids;
 
 	rodc_dn = ldb_dn_new_fmt(mem_ctx, sam_ctx, "<SID=%s>",
 				 dom_sid_string(mem_ctx, user_sid));
@@ -2886,38 +2886,14 @@ static bool sam_rodc_access_check(struct ldb_context *sam_ctx,
 		goto denied;
 	}
 
-	werr = samdb_result_sid_array_dn(sam_ctx, rodc_res->msgs[0],
-					 mem_ctx, "msDS-NeverRevealGroup",
-					 &num_never_reveal_sids,
-					 &never_reveal_sids);
-	if (!W_ERROR_IS_OK(werr)) {
-		goto denied;
-	}
-
-	werr = samdb_result_sid_array_dn(sam_ctx, rodc_res->msgs[0],
-					 mem_ctx, "msDS-RevealOnDemandGroup",
-					 &num_reveal_sids,
-					 &reveal_sids);
-	if (!W_ERROR_IS_OK(werr)) {
-		goto denied;
-	}
+	werr = samdb_confirm_rodc_allowed_to_repl_to_sid_list(sam_ctx,
+							      rodc_res->msgs[0],
+							      num_token_sids,
+							      token_sids);
 
-	if (never_reveal_sids &&
-	    sid_list_match(num_token_sids,
-			   token_sids,
-			   num_never_reveal_sids,
-			   never_reveal_sids)) {
-		goto denied;
-	}
-
-	if (reveal_sids &&
-	    sid_list_match(num_token_sids,
-			   token_sids,
-			   num_reveal_sids,
-			   reveal_sids)) {
+	if (W_ERROR_IS_OK(werr)) {
 		goto allowed;
 	}
-
 denied:
 	return false;
 allowed: