From 415648060feebcd26dc5f5327f11cf9fd278f0e7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 18 Sep 2013 14:27:26 -0700 Subject: [PATCH 1/6] python/drs: Ensure to pass in the local invocationID during the domain join This ensures (and asserts) that we never write an all-zero GUID as an invocationID to the database in replPropertyMetaData. Andrew Bartlett --- python/samba/drs_utils.py | 8 ++++++-- python/samba/join.py | 2 +- python/samba/netcmd/drs.py | 4 +++- source4/dsdb/common/util.c | 2 ++ source4/dsdb/pydsdb.c | 5 +++++ source4/libnet/py_net.c | 17 +++++++++++++---- 6 files changed, 30 insertions(+), 8 deletions(-) diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py index 6e2cfea..4983749 100644 --- a/python/samba/drs_utils.py +++ b/python/samba/drs_utils.py @@ -147,12 +147,16 @@ def drs_DsBind(drs): class drs_Replicate(object): '''DRS replication calls''' - def __init__(self, binding_string, lp, creds, samdb): + def __init__(self, binding_string, lp, creds, samdb, invocation_id): self.drs = drsuapi.drsuapi(binding_string, lp, creds) (self.drs_handle, self.supported_extensions) = drs_DsBind(self.drs) self.net = Net(creds=creds, lp=lp) self.samdb = samdb - self.replication_state = self.net.replicate_init(self.samdb, lp, self.drs) + if not isinstance(invocation_id, misc.GUID): + raise RuntimeError("Must supply GUID for invocation_id") + if invocation_id == misc.GUID("00000000-0000-0000-0000-000000000000"): + raise RuntimeError("Must not set GUID 00000000-0000-0000-0000-000000000000 as invocation_id") + self.replication_state = self.net.replicate_init(self.samdb, lp, self.drs, invocation_id) def drs_get_rodc_partial_attribute_set(self): '''get a list of attributes for RODC replication''' diff --git a/python/samba/join.py b/python/samba/join.py index 15db67f..2379d5f 100644 --- a/python/samba/join.py +++ b/python/samba/join.py @@ -799,7 +799,7 @@ class dc_join(object): binding_options += ",print" repl = drs_utils.drs_Replicate( "ncacn_ip_tcp:%s[%s]" % (ctx.server, binding_options), - ctx.lp, repl_creds, ctx.local_samdb) + ctx.lp, repl_creds, ctx.local_samdb, ctx.invocation_id) repl.replicate(ctx.schema_dn, source_dsa_invocation_id, destination_dsa_guid, schema=True, rodc=ctx.RODC, diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py index de78ac7..36dc48e 100644 --- a/python/samba/netcmd/drs.py +++ b/python/samba/netcmd/drs.py @@ -258,11 +258,13 @@ def drs_local_replicate(self, SOURCE_DC, NC): source_dsa_invocation_id = misc.GUID(self.samdb.get_invocation_id()) + dest_dsa_invocation_id = misc.GUID(self.local_samdb.get_invocation_id()) destination_dsa_guid = self.ntds_guid self.samdb.transaction_start() repl = drs_utils.drs_Replicate("ncacn_ip_tcp:%s[seal]" % self.server, self.lp, - self.creds, self.local_samdb) + self.creds, self.local_samdb, dest_dsa_invocation_id) + try: repl.replicate(NC, source_dsa_invocation_id, destination_dsa_guid) except Exception, e: diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 7a243c3..55bd73e 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -1302,6 +1302,7 @@ const struct GUID *samdb_ntds_invocation_id(struct ldb_context *ldb) /* see if we have a cached copy */ invocation_id = (struct GUID *)ldb_get_opaque(ldb, "cache.invocation_id"); if (invocation_id) { + SMB_ASSERT(!GUID_all_zero(invocation_id)); return invocation_id; } @@ -1362,6 +1363,7 @@ bool samdb_set_ntds_invocation_id(struct ldb_context *ldb, const struct GUID *in goto failed; } + SMB_ASSERT(!GUID_all_zero(invocation_id_in)); *invocation_id_new = *invocation_id_in; /* cache the domain_sid in the ldb */ diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c index 99e239e..c9e80c2 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -727,6 +727,11 @@ static PyObject *py_dsdb_set_ntds_invocation_id(PyObject *self, PyObject *args) PyErr_LDB_OR_RAISE(py_ldb, ldb); GUID_from_string(PyString_AsString(py_guid), &guid); + if (GUID_all_zero(&guid)) { + PyErr_SetString(PyExc_RuntimeError, "set_ntds_invocation_id rejected due to all-zero invocation ID"); + return NULL; + } + ret = samdb_set_ntds_invocation_id(ldb, &guid); if (!ret) { PyErr_SetString(PyExc_RuntimeError, "set_ntds_invocation_id failed"); diff --git a/source4/libnet/py_net.c b/source4/libnet/py_net.c index acb0a37..7981aad 100644 --- a/source4/libnet/py_net.c +++ b/source4/libnet/py_net.c @@ -22,6 +22,7 @@ #include #include "includes.h" #include +#include #include "libnet.h" #include "auth/credentials/pycredentials.h" #include "libcli/security/security.h" @@ -33,6 +34,7 @@ #include "libcli/finddc.h" #include "dsdb/samdb/samdb.h" #include "py_net.h" +#include "librpc/rpc/pyrpc_util.h" void initnet(void); @@ -363,16 +365,17 @@ struct replicate_state { */ static PyObject *py_net_replicate_init(py_net_Object *self, PyObject *args, PyObject *kwargs) { - const char *kwnames[] = { "samdb", "lp", "drspipe", NULL }; - PyObject *py_ldb, *py_lp, *py_drspipe; + const char *kwnames[] = { "samdb", "lp", "drspipe", "invocation_id", NULL }; + PyObject *py_ldb, *py_lp, *py_drspipe, *py_invocation_id; struct ldb_context *samdb; struct loadparm_context *lp; struct replicate_state *s; NTSTATUS status; - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OOO", + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OOOO", discard_const_p(char *, kwnames), - &py_ldb, &py_lp, &py_drspipe)) { + &py_ldb, &py_lp, &py_drspipe, + &py_invocation_id)) { return NULL; } @@ -392,6 +395,12 @@ static PyObject *py_net_replicate_init(py_net_Object *self, PyObject *args, PyOb talloc_free(s); return NULL; } + if (!py_check_dcerpc_type(py_invocation_id, "samba.dcerpc.misc", "GUID")) { + + talloc_free(s); + return NULL; + } + s->dest_dsa.invocation_id = *pytalloc_get_type(py_invocation_id, struct GUID); s->drs_pipe = (dcerpc_InterfaceObject *)(py_drspipe); -- 1.8.3.1 From 8e541a5fdccba306f785f0e7c6149f040a5e04cc Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 17 Sep 2013 15:31:51 -0700 Subject: [PATCH 2/6] dsdb-repl_meta_data: Check for a NULL invocationID and do not proceed This can happen if we do not find the invocationID, with later patches. Andrew Bartlett Signed-off-by: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index c8cdfec..7bd0265 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -4839,6 +4839,10 @@ static int replmd_replicated_uptodate_modify(struct replmd_replicated_request *a /* get our invocation_id if we have one already attached to the ldb */ our_invocation_id = samdb_ntds_invocation_id(ldb); + if (our_invocation_id == NULL) { + DEBUG(0, ("repl_meta_data: Could not find our own server's invocationID!\n")); + return replmd_replicated_request_werror(ar, WERR_DS_DRA_INTERNAL_ERROR); + } /* merge in the source_dsa vector is available */ for (i=0; (ruv && i < ruv->count); i++) { -- 1.8.3.1 From b4bbf910f67002f4ca93c1cd7e11644f2d84525b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 17 Sep 2013 15:20:48 -0700 Subject: [PATCH 3/6] dsdb: Refuse to return an all-zero invocationID This could cause an all-zero GUID to be entered into the replPropertyMetaData, which will then fail to be replicated to other DCs. Signed-off-by: Andrew Bartlett --- source4/dsdb/common/util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 55bd73e..904ca1d 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -1326,6 +1326,14 @@ const struct GUID *samdb_ntds_invocation_id(struct ldb_context *ldb) } *invocation_id = samdb_result_guid(res->msgs[0], "invocationId"); + if (GUID_all_zero(invocation_id)) { + if (ldb_msg_find_ldb_val(res->msgs[0], "invocationId")) { + DEBUG(0, ("Failed to find our own NTDS Settings invocationId in the ldb!\n")); + } else { + DEBUG(0, ("Failed to find parse own NTDS Settings invocationId from the ldb!\n")); + } + goto failed; + } /* cache the domain_sid in the ldb */ if (ldb_set_opaque(ldb, "cache.invocation_id", invocation_id) != LDB_SUCCESS) { -- 1.8.3.1 From 21e4c93214b6b0dfbb99fb73e38debe5a4042433 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 17 Sep 2013 15:28:32 -0700 Subject: [PATCH 4/6] dsdb-repl_meta_data: Do not re-delete the Deleted Objects DN during replication We need to ensure we do not re-delete the Deleted Objects DN during replication. It itself not entirely a deleted object, but has isDeleted set. Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 7bd0265..e562e24 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -4655,7 +4655,11 @@ static int replmd_replicated_apply_next(struct replmd_replicated_request *ar) */ static int replmd_replicated_apply_isDeleted(struct replmd_replicated_request *ar) { - if (ar->isDeleted) { + struct ldb_dn *deleted_objects_dn; + struct ldb_message *msg = ar->objs->objects[ar->index_current].msg; + int ret = dsdb_get_deleted_objects_dn(ldb_module_get_ctx(ar->module), msg, msg->dn, + &deleted_objects_dn); + if (ar->isDeleted && (ret != LDB_SUCCESS || ldb_dn_compare(msg->dn, deleted_objects_dn) != 0)) { /* * Do a delete here again, so that if there is * anything local that conflicts with this @@ -4669,11 +4673,9 @@ static int replmd_replicated_apply_isDeleted(struct replmd_replicated_request *a */ /* This has been updated to point to the DN we eventually did the modify on */ - struct ldb_message *msg = ar->objs->objects[ar->index_current].msg; struct ldb_request *del_req; struct ldb_result *res; - int ret; TALLOC_CTX *tmp_ctx = talloc_new(ar); if (!tmp_ctx) { -- 1.8.3.1 From d23022cb7542730b6dc044b912169ff59c23f465 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 17 Sep 2013 15:31:04 -0700 Subject: [PATCH 5/6] dsdb-repl_meta_data: Make handling of Deleted Objects DN clearer in delete This code no longer needs to handle not renaming Deleted Objects during a re-delete, because it is no longer called in that case. Andrew Bartlett Signed-off-by: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index e562e24..91a5d92 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -3001,14 +3001,17 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request /* work out where we will be renaming this object to */ if (!disallow_move_on_delete) { + struct ldb_dn *deleted_objects_dn; ret = dsdb_get_deleted_objects_dn(ldb, tmp_ctx, old_dn, - &new_dn); + &deleted_objects_dn); + /* - * Deleted Objects itself appears to be deleted, but - * should also not be moved, and we should not move - * objects if we can't find the deleted objects DN + * We should not move objects if we can't find the + * deleted objects DN. Not moving (or otherwise + * harming) the Deleted Objects DN itself is handled + * in the caller. */ - if (re_delete && (ret != LDB_SUCCESS || ldb_dn_compare(old_dn, new_dn) == 0)) { + if (re_delete && (ret != LDB_SUCCESS)) { new_dn = ldb_dn_get_parent(tmp_ctx, old_dn); if (new_dn == NULL) { ldb_module_oom(module); @@ -3023,6 +3026,8 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request ldb_dn_get_linearized(old_dn)); talloc_free(tmp_ctx); return LDB_ERR_UNWILLING_TO_PERFORM; + } else { + new_dn = deleted_objects_dn; } } else { new_dn = ldb_dn_get_parent(tmp_ctx, old_dn); -- 1.8.3.1 From ffb7384fd7341ae93e80986d76c95d2f4f76fa94 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 18 Sep 2013 14:29:26 -0700 Subject: [PATCH 6/6] lib/messaging: Check the GUID type correctly --- source4/lib/messaging/pymessaging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index d035fb0..62370ae 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -42,7 +42,7 @@ extern PyTypeObject imessaging_Type; static bool server_id_from_py(PyObject *object, struct server_id *server_id) { if (!PyTuple_Check(object)) { - if (!py_check_dcerpc_type(object, "server_id", "server_id")) { + if (!py_check_dcerpc_type(object, "samba.dcerpc.server_id", "server_id")) { PyErr_SetString(PyExc_ValueError, "Expected tuple or server_id"); return false; -- 1.8.3.1