s4-dsdb: rework drs_ObjectIdentifier_to_dn() into drs_ObjectIdentifier_to_dn_and_nc_r...

This make this funciton the gatekeeper between the wire format and the
internal struct ldb_dn, checking if the DN exists and which NC
it belongs to along the way, and presenting only a DB-returned
DN for internal processing.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
index 317d78c41b1d07315b3cfc8638baee0596dd38f7..7415f5727102a2216ac80c355efed2a9abde5ef5 100644 (file)
--- a/selftest/knownfail.d/getncchanges
+++ b/selftest/knownfail.d/getncchanges
@@ -6,12 +6,10 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri
 samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\)
 # New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync
 ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID
-^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID
+^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_SECRET
 ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ
-^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_RID_ALLOC
 ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC
 ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_OBJ
 ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET
-^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_DummyDN_valid_GUID_full_repl
 ^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_InvalidNC_DummyDN_InvalidGUID_full_repl
 ^samba4.drs.repl_rodc.python\(.*\).repl_rodc.DrsRodcTestCase.test_admin_repl_secrets_DummyDN_GUID

diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c
index e348ab6aa949c7a5f12df248c66d032b0a759527..04aab1593b12649e6e8f4c12c55681257cb672bc 100644 (file)
--- a/source4/dsdb/common/dsdb_dn.c
+++ b/source4/dsdb/common/dsdb_dn.c
@@ -396,3 +396,36 @@ struct ldb_dn *drs_ObjectIdentifier_to_dn(TALLOC_CTX *mem_ctx,
        talloc_free(dn_string);
        return new_dn;
 }
+
+/*
+ * Safely convert a drsuapi_DsReplicaObjectIdentifier into a validated
+ * LDB DN of an existing DB entry, and/or find the NC root
+ *
+ * Finally, we must return the DN as found in the DB, as otherwise a
+ * subsequence ldb_dn_compare(dn, nc_root) will fail (as this is based
+ * on the string components).
+ */
+int drs_ObjectIdentifier_to_dn_and_nc_root(TALLOC_CTX *mem_ctx,
+                                          struct ldb_context *ldb,
+                                          struct drsuapi_DsReplicaObjectIdentifier *nc,
+                                          struct ldb_dn **normalised_dn,
+                                          struct ldb_dn **nc_root)
+{
+       int ret;
+       struct ldb_dn *new_dn = NULL;
+
+       new_dn = drs_ObjectIdentifier_to_dn(mem_ctx,
+                                           ldb,
+                                           nc);
+       if (new_dn == NULL) {
+               return LDB_ERR_INVALID_DN_SYNTAX;
+       }
+
+       ret = dsdb_normalise_dn_and_find_nc_root(ldb,
+                                                mem_ctx,
+                                                new_dn,
+                                                normalised_dn,
+                                                nc_root);
+       TALLOC_FREE(new_dn);
+       return ret;
+}

diff --git a/source4/rpc_server/drsuapi/drsutil.c b/source4/rpc_server/drsuapi/drsutil.c
index 7897c35d2e94dcc6f725ac423b299edcfaee8268..48423bb6655a2de13b62b9f90e713563788b8ea0 100644 (file)
--- a/source4/rpc_server/drsuapi/drsutil.c
+++ b/source4/rpc_server/drsuapi/drsutil.c
@@ -191,8 +191,19 @@ WERROR drs_security_access_check(struct ldb_context *sam_ctx,
                                 struct drsuapi_DsReplicaObjectIdentifier *nc,
                                 const char *ext_right)
 {
-       struct ldb_dn *dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, nc);
+       struct ldb_dn *dn;
        WERROR werr;
+       int ret;
+
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
+                                                    sam_ctx,
+                                                    nc,
+                                                    &dn,
+                                                    NULL);
+       if (ret != LDB_SUCCESS) {
+               return WERR_DS_DRA_BAD_DN;
+       }
+
        werr = drs_security_access_check_log(sam_ctx, mem_ctx, token, dn, ext_right);
        talloc_free(dn);
        return werr;
@@ -207,17 +218,20 @@ WERROR drs_security_access_check_nc_root(struct ldb_context *sam_ctx,
                                         struct drsuapi_DsReplicaObjectIdentifier *nc,
                                         const char *ext_right)
 {
-       struct ldb_dn *dn, *nc_root;
+       struct ldb_dn *nc_root;
        WERROR werr;
        int ret;
 
-       dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, nc);
-       W_ERROR_HAVE_NO_MEMORY(dn);
-       ret = dsdb_find_nc_root(sam_ctx, dn, dn, &nc_root);
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
+                                                    sam_ctx,
+                                                    nc,
+                                                    NULL,
+                                                    &nc_root);
        if (ret != LDB_SUCCESS) {
-               return WERR_DS_CANT_FIND_EXPECTED_NC;
+               return WERR_DS_DRA_BAD_NC;
        }
+
        werr = drs_security_access_check_log(sam_ctx, mem_ctx, token, nc_root, ext_right);
-       talloc_free(dn);
+       talloc_free(nc_root);
        return werr;
 }

diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index 7f50587dd1e87c1d1992b858fcf476b23fb1cc03..b2dea15ac8ebd87aff1f98dd38587a2a4a36e6d5 100644 (file)
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -1091,9 +1091,20 @@ static WERROR getncchanges_rid_alloc(struct drsuapi_bind_state *b_state,
                return WERR_DS_DRA_INTERNAL_ERROR;
        }
 
-       req_dn = drs_ObjectIdentifier_to_dn(mem_ctx, ldb, req10->naming_context);
-       if (!ldb_dn_validate(req_dn) ||
-           ldb_dn_compare(req_dn, *rid_manager_dn) != 0) {
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
+                                                    ldb,
+                                                    req10->naming_context,
+                                                    &req_dn,
+                                                    NULL);
+       if (ret != LDB_SUCCESS) {
+               DBG_ERR("RID Alloc request for invalid DN %s: %s\n",
+                       drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context),
+                       ldb_strerror(ret));
+               ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH;
+               return WERR_OK;
+       }
+
+       if (ldb_dn_compare(req_dn, *rid_manager_dn) != 0) {
                /* that isn't the RID Manager DN */
                DEBUG(0,(__location__ ": RID Alloc request for wrong DN %s\n",
                         drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context)));
@@ -1250,7 +1261,17 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state,
         * Which basically means that if you have GET_ALL_CHANGES rights (~== RWDC)
         * then you can do EXOP_REPL_SECRETS
         */
-       obj_dn = drs_ObjectIdentifier_to_dn(mem_ctx, b_state->sam_ctx_system, ncRoot);
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
+                                                       b_state->sam_ctx_system,
+                                                       ncRoot,
+                                                       &obj_dn,
+                                                       NULL);
+       if (ret != LDB_SUCCESS) {
+               DBG_ERR("RevealSecretRequest for for invalid DN %s\n",
+                        drs_ObjectIdentifier_to_string(mem_ctx, ncRoot));
+               goto failed;
+       }
+
        if (!ldb_dn_validate(obj_dn)) goto failed;
 
        if (has_get_all_changes) {
@@ -1362,8 +1383,9 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state,
            - verify that we are the current master
         */
 
-       req_dn = drs_ObjectIdentifier_to_dn(mem_ctx, ldb, req10->naming_context);
-       if (!ldb_dn_validate(req_dn)) {
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, ldb, req10->naming_context,
+                                                    &req_dn, NULL);
+       if (ret != LDB_SUCCESS) {
                /* that is not a valid dn */
                DEBUG(0,(__location__ ": FSMO role transfer request for invalid DN %s\n",
                         drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context)));
@@ -1389,8 +1411,16 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state,
        /* change the current master */
        msg = ldb_msg_new(ldb);
        W_ERROR_HAVE_NO_MEMORY(msg);
-       msg->dn = drs_ObjectIdentifier_to_dn(msg, ldb, req10->naming_context);
-       W_ERROR_HAVE_NO_MEMORY(msg->dn);
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(msg, ldb, req10->naming_context,
+                                                    &msg->dn, NULL);
+       if (ret != LDB_SUCCESS) {
+               /* that is not a valid dn */
+               DBG_ERR("FSMO role transfer request for invalid DN %s: %s\n",
+                       drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context),
+                       ldb_strerror(ret));
+               ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH;
+               return WERR_OK;
+       }
 
        /* TODO: make sure ntds_dn is a valid nTDSDSA object */
        ret = dsdb_find_dn_by_guid(ldb, msg, &req10->destination_dsa_guid, 0, &ntds_dn);
@@ -2864,7 +2894,21 @@ allowed:
 
        /* see if a previous replication has been abandoned */
        if (getnc_state) {
-               struct ldb_dn *new_dn = drs_ObjectIdentifier_to_dn(getnc_state, sam_ctx, ncRoot);
+               struct ldb_dn *new_dn;
+               ret = drs_ObjectIdentifier_to_dn_and_nc_root(getnc_state,
+                                                            sam_ctx,
+                                                            ncRoot,
+                                                            &new_dn,
+                                                            NULL);
+               if (ret != LDB_SUCCESS) {
+                       /*
+                        * This can't fail as we have done this above
+                        * implicitly but not got the DN out
+                        */
+                       DBG_ERR("Bad DN '%s'\n",
+                               drs_ObjectIdentifier_to_string(mem_ctx, ncRoot));
+                       return WERR_DS_DRA_INVALID_PARAMETER;
+               }
                if (ldb_dn_compare(new_dn, getnc_state->ncRoot_dn) != 0) {
                        DEBUG(0,(__location__ ": DsGetNCChanges 2nd replication on different DN %s %s (last_dn %s)\n",
                                 ldb_dn_get_linearized(new_dn),
@@ -2899,9 +2943,13 @@ allowed:
                uint32_t nc_instanceType;
                struct ldb_dn *ncRoot_dn;
 
-               ncRoot_dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, ncRoot);
-               if (ncRoot_dn == NULL) {
-                       return WERR_NOT_ENOUGH_MEMORY;
+               ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
+                                                            sam_ctx,
+                                                            ncRoot,
+                                                            &ncRoot_dn,
+                                                            NULL);
+               if (ret != LDB_SUCCESS) {
+                       return WERR_DS_DRA_BAD_DN;
                }
 
                ret = dsdb_search_dn(sam_ctx, mem_ctx, &res,

diff --git a/source4/rpc_server/drsuapi/updaterefs.c b/source4/rpc_server/drsuapi/updaterefs.c
index 7450ddd3a31400706779505f14d3b13d4723673a..5d2bc6e949cd060c7b8760139cfd6da753cebb00 100644 (file)
--- a/source4/rpc_server/drsuapi/updaterefs.c
+++ b/source4/rpc_server/drsuapi/updaterefs.c
@@ -195,7 +195,6 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx,
 {
        WERROR werr;
        int ret;
-       struct ldb_dn *dn;
        struct ldb_dn *dn_normalised;
        struct ldb_dn *nc_root;
        struct ldb_context *sam_ctx = b_state->sam_ctx_system?b_state->sam_ctx_system:b_state->sam_ctx;
@@ -226,14 +225,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx,
                return WERR_DS_DRA_INVALID_PARAMETER;
        }
 
-       dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, req->naming_context);
-       W_ERROR_HAVE_NO_MEMORY(dn);
-       ret = dsdb_normalise_dn_and_find_nc_root(sam_ctx, dn,
-                                                dn,
-                                                &dn_normalised,
-                                                &nc_root);
+       ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, sam_ctx, req->naming_context,
+                                                    &dn_normalised, &nc_root);
        if (ret != LDB_SUCCESS) {
-               DEBUG(2, ("Didn't find a nc for %s\n", ldb_dn_get_linearized(dn)));
+               DBG_WARNING("Didn't find a nc for %s\n",
+                           ldb_dn_get_linearized(dn_normalised));
                return WERR_DS_DRA_BAD_NC;
        }
        if (ldb_dn_compare(dn_normalised, nc_root) != 0) {
@@ -249,7 +245,10 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx,
         * This means that in the usual case, it will never open it and never
         * bother to refresh the dreplsrv.
         */
-       werr = uref_check_dest(sam_ctx, mem_ctx, dn, &req->dest_dsa_guid,
+       werr = uref_check_dest(sam_ctx,
+                              mem_ctx,
+                              dn_normalised,
+                              &req->dest_dsa_guid,
                               req->options);
        if (W_ERROR_EQUAL(werr, WERR_DS_DRA_REF_ALREADY_EXISTS) ||
            W_ERROR_EQUAL(werr, WERR_DS_DRA_REF_NOT_FOUND)) {
@@ -266,7 +265,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx,
        }
 
        if (req->options & DRSUAPI_DRS_DEL_REF) {
-               werr = uref_del_dest(sam_ctx, mem_ctx, dn, &req->dest_dsa_guid, req->options);
+               werr = uref_del_dest(sam_ctx,
+                                    mem_ctx,
+                                    dn_normalised,
+                                    &req->dest_dsa_guid,
+                                    req->options);
                if (!W_ERROR_IS_OK(werr)) {
                        DEBUG(0,("Failed to delete repsTo for %s: %s\n",
                                 GUID_string(mem_ctx, &req->dest_dsa_guid),
@@ -287,7 +290,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx,
                dest.source_dsa_obj_guid = req->dest_dsa_guid;
                dest.replica_flags       = req->options;
 
-               werr = uref_add_dest(sam_ctx, mem_ctx, dn, &dest, req->options);
+               werr = uref_add_dest(sam_ctx,
+                                    mem_ctx,
+                                    dn_normalised,
+                                    &dest,
+                                    req->options);
                if (!W_ERROR_IS_OK(werr)) {
                        DEBUG(0,("Failed to add repsTo for %s: %s\n",
                                 GUID_string(mem_ctx, &dest.source_dsa_obj_guid),