summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Bartlett <abartlet@samba.org>2022-12-12 09:47:36 +1300
committerJule Anger <janger@samba.org>2023-02-03 09:35:08 +0000
commit7c32d3d75aa31b868527d992e08e8d63fc76faac (patch)
treecabfafea0ea6db52786e2518a63bc66acf5ba440
parent85cc464195bf9cf70159d113f33582af02ac6614 (diff)
downloadsamba-7c32d3d75aa31b868527d992e08e8d63fc76faac.tar.gz
s4-dsdb: rework drs_ObjectIdentifier_to_dn() into drs_ObjectIdentifier_to_dn_and_nc_root()
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> (cherry picked from commit aee2039e63ceeb5e69a0461fb77e0f18278e4dc4)
-rw-r--r--selftest/knownfail.d/getncchanges4
-rw-r--r--source4/dsdb/common/dsdb_dn.c33
-rw-r--r--source4/rpc_server/drsuapi/drsutil.c28
-rw-r--r--source4/rpc_server/drsuapi/getncchanges.c72
-rw-r--r--source4/rpc_server/drsuapi/updaterefs.c29
5 files changed, 133 insertions, 33 deletions
diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
index 317d78c41b1..7415f572710 100644
--- 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 e348ab6aa94..04aab1593b1 100644
--- 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 7897c35d2e9..48423bb6655 100644
--- 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 7f50587dd1e..b2dea15ac8e 100644
--- 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 7450ddd3a31..5d2bc6e949c 100644
--- 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),