[PATCH] Some minor DRS changes

Tim Beale timbeale at catalyst.net.nz
Tue Nov 6 01:11:51 UTC 2018


Attached are some minor DRS changes. The main one is that we noticed
that Bug #13612 could still occur when trying to join a Windows DC with
a large database. The problem was Windows was returning a slightly
different NT_STATUS code to Samba, so we didn't try to re-establish the
LDAP connection.

The other changes improve some debug, fix up some long lines, and fix up
a spurious Python exception being thrown in a corner case.

CI pass: https://gitlab.com/catalyst-samba/samba/pipelines/35410080

Review appreciated. Thanks.

-------------- next part --------------
From e9df70df5543269cb52825a0b5d36d31de167277 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 5 Nov 2018 14:30:14 +1300
Subject: [PATCH 1/4] join: Fix join large-DB timeout against Windows

The LDAP connection can also timeout when trying to join a Windows DC
with a very large database. However, in this case Windows gives a
slightly different error message (NT_STATUS_CONNECTION_RESET instead of
NT_STATUS_CONNECTION_DISCONNECTED).

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/join.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index c2d9254..343b1a5 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -1031,7 +1031,8 @@ class DCJoinContext(object):
 
             # if the connection was disconnected, then reconnect
             if (enum == ldb.ERR_OPERATIONS_ERROR and
-                'NT_STATUS_CONNECTION_DISCONNECTED' in estr):
+                ('NT_STATUS_CONNECTION_DISCONNECTED' in estr or
+                 'NT_STATUS_CONNECTION_RESET' in estr)):
                 ctx.logger.warning("LDB connection disconnected. Reconnecting")
                 ctx.samdb = SamDB(url="ldap://%s" % ctx.server,
                                   session_info=system_session(),
-- 
2.7.4


From a87c3c0fd9697aa321851e98bdab0b88b777cb03 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 5 Nov 2018 16:34:15 +1300
Subject: [PATCH 2/4] libnet: Reset debug counters after replicating critical
 objects

Reset the debug counters once we have finished replicating a given
partition. This helps if we replicate the same partition immediately
afterward with different options.

This helps the DC join debug look less weird. Because it replicates the
critical objects first, and then the base partition, previously it
always ended up overcounting, e.g.

Partition[DC=addom,DC=samba,DC=example,DC=com] objects[314/218]
  linked_values[48/24]

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/libnet/libnet_vampire.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
index 5bd8df1..6167493 100644
--- a/source4/libnet/libnet_vampire.c
+++ b/source4/libnet/libnet_vampire.c
@@ -781,6 +781,12 @@ WERROR libnet_vampire_cb_store_chunk(void *private_data,
 		return status;
 	}
 
+	/* reset debug counters once we've finished replicating the partition */
+	if (!c->partition->more_data) {
+		s->total_objects = 0;
+		s->total_links = 0;
+	}
+
 	talloc_free(s_dsa);
 	talloc_free(objs);
 
-- 
2.7.4


From ca57a96e8e4777642d39f4ed9391663612da0a32 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 5 Nov 2018 16:54:05 +1300
Subject: [PATCH 3/4] drs_utils: Fix some long lines

Tweak the code slightly to avoid some 80+ character lines.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/drs_utils.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py
index dcd4d73..be2b606 100644
--- a/python/samba/drs_utils.py
+++ b/python/samba/drs_utils.py
@@ -25,7 +25,9 @@ from samba import werror
 from samba import WERRORError
 import samba
 import ldb
-from samba.dcerpc.drsuapi import DRSUAPI_ATTID_name
+from samba.dcerpc.drsuapi import (DRSUAPI_ATTID_name,
+                                  DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8,
+                                  DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10)
 import re
 
 
@@ -204,7 +206,7 @@ class drs_Replicate(object):
 
     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.drs_handle, self.supports_ext) = drs_DsBind(self.drs)
         self.net = Net(creds=creds, lp=lp)
         self.samdb = samdb
         if not isinstance(invocation_id, misc.GUID):
@@ -225,7 +227,7 @@ class drs_Replicate(object):
         # return (error_code == werror.WERR_DS_DRA_RECYCLED_TARGET and
         return (error_code == 0x21bf and
                 (req.more_flags & drsuapi.DRSUAPI_DRS_GET_TGT) == 0 and
-                self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10)
+                self.supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10)
 
     def process_chunk(self, level, ctr, schema, req_level, req, first_chunk):
         '''Processes a single chunk of received replication data'''
@@ -239,7 +241,7 @@ class drs_Replicate(object):
         '''replicate a single DN'''
 
         # setup for a GetNCChanges call
-        if self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10:
+        if self.supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10:
             req = drsuapi.DsGetNCChangesRequest10()
             req.more_flags = (more_flags | self.more_flags)
             req_level = 10
@@ -319,7 +321,7 @@ class drs_Replicate(object):
         if not schema and rodc:
             req.partial_attribute_set = drs_get_rodc_partial_attribute_set(self.samdb)
 
-        if not self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8:
+        if not self.supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8:
             req_level = 5
             req5 = drsuapi.DsGetNCChangesRequest5()
             for a in dir(req5):
-- 
2.7.4


From 3e3a5d737e57ddbd0c33c5c314c328437f00313d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 5 Nov 2018 17:01:55 +1300
Subject: [PATCH 4/4] drs_utils: Avoid invalid dereference of v8 requests

req.more_flags only exists for v10 requests, so we throw an exception if
we try to dereference that field on a v8 (or v5) request. Unfortunately,
we were checking that we support v10 *after* we had tried to access the
more_flags. This patch fixes up the order of the checks.

This may be a problem trying to replicate with an older Windows DC
(pre-2008R2), and was reported on the samba mailing-list at one point:
https://lists.samba.org/archive/samba/2018-June/216541.html

Unfortunately this patch doesn't help the overall situation at all (the
join will fail because we can't resolve the link target and we can't use
GET_TGT). But it now gives you a more meaningful error, i.e.

  ERROR(runtime): uncaught exception - (8639, "Failed to process 'chunk'
    of DRS replicated objects: DOS code 0x000021bf"
instead of:
  ERROR(<type 'exceptions.AttributeError'>): uncaught exception -
    'drsuapi.DsGetNCChangesRequest8' object has no attribute 'more_flags'

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/drs_utils.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py
index be2b606..43d1b4a 100644
--- a/python/samba/drs_utils.py
+++ b/python/samba/drs_utils.py
@@ -221,13 +221,14 @@ class drs_Replicate(object):
         # If the error indicates we fail to resolve a target object for a
         # linked attribute, then we should retry the request with GET_TGT
         # (if we support it and haven't already tried that)
+        supports_ext = self.supports_ext
 
         # TODO fix up the below line when we next update werror_err_table.txt
         # and pull in the new error-code
         # return (error_code == werror.WERR_DS_DRA_RECYCLED_TARGET and
         return (error_code == 0x21bf and
-                (req.more_flags & drsuapi.DRSUAPI_DRS_GET_TGT) == 0 and
-                self.supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10)
+                supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10 and
+                (req.more_flags & drsuapi.DRSUAPI_DRS_GET_TGT) == 0)
 
     def process_chunk(self, level, ctr, schema, req_level, req, first_chunk):
         '''Processes a single chunk of received replication data'''
-- 
2.7.4



More information about the samba-technical mailing list