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

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 65d7e7c7271e7bc8685cfb0686cd270f29bfa363..eaeab236fc0111d41ef6c67db685d012df387e7f 100644 (file)
--- 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 bc30e73e06bf4c511a5a1105ed9cc66365c4274c..3b1d674573ff9ae71bdad86853d9d76724cda174 100644 (file)
--- 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 51c6666a1649618e45f55e51767d9a49aca30b71..1aecd65bb618ad137038a412e19e7b7f62ec7aca 100644 (file)
--- 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: