Attachment 9230 Details for Bug 10157 – proposed patch to fix this in master

[patch] proposed patch to fix this in master

fix-delete-deleted-objects.patch (text/plain), 13.58 KB, created by Andrew Bartlett on 2013-09-19 16:24:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2013-09-19 16:24:16 UTC

Size: 13.58 KB

patch

obsolete

>From 415648060feebcd26dc5f5327f11cf9fd278f0e7 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>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 <Python.h>
> #include "includes.h"
> #include <pyldb.h>
>+#include <pytalloc.h>
> #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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <abartlet@samba.org>
>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
>

Actions: View

Attachments on bug 10157: 9230 | 9240