[PATCH] Fix Bug 5917 - Samba does not work on site with Read Only Domain Controller

Jeremy Allison jra at samba.org
Tue Sep 3 21:37:13 CEST 2013


Fix inspired by a patch created by hemanth.thummala at gmail.com
on the bug.

>From his comment on the bug:

"I am able to figure out the root cause. Looks like we are not doing domain
level DC discovery if we find few DCs at site level.

In the code , initially discover_dc_dns() will find DCs at site level first.
And if the number of DCs returned from site is zero then it will try to fetch
the DCs at domain level(by setting site_name to NULL).

DC validation is actually done later in process_dc_dns(). There we realize that
the list of DCs are not valid for domain join as in this case they are not
writable."

This patchset fixes the problem in a cleaner
way by creating a wrapper function for
dsgetdcname() that does the sitename
manipulation that callers expect when
the pass in NULL or "" as the sitename,
yet still allows callers who want an
explicit site to pass in a non-NULL
sitename to get returns only from
that site.

Hemanth has confirmed this fixes
the problem for him. Please review
and push. Patchset cleanly applies
to master, 4.1.0, 4.0.next and 3.6.next.

Cheers,

	Jeremy.
-------------- next part --------------
>From 48955bfed5e22adbf4929b7135847740309dd42c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Sep 2013 12:13:45 -0700
Subject: [PATCH 1/4] dsgetdcname_cache_fetch() doesn't use the site_name
 parameter so don't pass it.

Bug 5917 - Samba does not work on site with Read Only Domain Controller

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/libsmb/dsgetdcname.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 442f8ed..305baca 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -321,7 +321,6 @@ static NTSTATUS dsgetdcname_cache_fetch(TALLOC_CTX *mem_ctx,
 					const char *domain_name,
 					const struct GUID *domain_guid,
 					uint32_t flags,
-					const char *site_name,
 					struct netr_DsRGetDCNameInfo **info_p)
 {
 	char *key;
@@ -394,7 +393,7 @@ static NTSTATUS dsgetdcname_cached(TALLOC_CTX *mem_ctx,
 	NTSTATUS status;
 
 	status = dsgetdcname_cache_fetch(mem_ctx, domain_name, domain_guid,
-					 flags, site_name, info);
+					 flags, info);
 	if (!NT_STATUS_IS_OK(status)
 	    && !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
 		DEBUG(10,("dsgetdcname_cached: cache fetch failed with: %s\n",
-- 
1.8.4


>From 8377c85cc6993d0b81a7607dddc033bf01cbaca3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Sep 2013 12:04:37 -0700
Subject: [PATCH 2/4] Refactor dsgetdcname to be called via a wrapper function.

Bug 5917 - Samba does not work on site with Read Only Domain Controller

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/libsmb/dsgetdcname.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 305baca..6e0d935 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -1082,12 +1082,10 @@ static bool is_closest_site(struct netr_DsRGetDCNameInfo *info)
 }
 
 /********************************************************************
- dsgetdcname.
-
- This will be the only public function here.
+ Internal dsgetdcname.
 ********************************************************************/
 
-NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
+static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 		     struct messaging_context *msg_ctx,
 		     const char *domain_name,
 		     const struct GUID *domain_guid,
@@ -1101,7 +1099,7 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
 	bool first = true;
 	struct netr_DsRGetDCNameInfo *first_info = NULL;
 
-	DEBUG(10,("dsgetdcname: domain_name: %s, "
+	DEBUG(10,("dsgetdcname_internal: domain_name: %s, "
 		  "domain_guid: %s, site_name: %s, flags: 0x%08x\n",
 		  domain_name,
 		  domain_guid ? GUID_string(mem_ctx, domain_guid) : "(null)",
@@ -1163,3 +1161,26 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
 	*info = myinfo;
 	return NT_STATUS_OK;
 }
+
+/********************************************************************
+ dsgetdcname.
+
+ This will be the only public function here.
+********************************************************************/
+
+NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
+		     struct messaging_context *msg_ctx,
+		     const char *domain_name,
+		     const struct GUID *domain_guid,
+		     const char *site_name,
+		     uint32_t flags,
+		     struct netr_DsRGetDCNameInfo **info)
+{
+	return dsgetdcname_internal(mem_ctx,
+				msg_ctx,
+				domain_name,
+				domain_guid,
+				site_name,
+				flags,
+				info);
+}
-- 
1.8.4


>From 319611bffc55b2924e3b1d7655e958c6838ec28f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Sep 2013 12:08:46 -0700
Subject: [PATCH 3/4] Move the manipulation of site_name into the caller
 function dsgetdcname().

Leave dsgetdcname_internal() only using const char *site_name.

Bug 5917 - Samba does not work on site with Read Only Domain Controller

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/libsmb/dsgetdcname.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 6e0d935..2318dfb 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -1095,7 +1095,6 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 {
 	NTSTATUS status = NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND;
 	struct netr_DsRGetDCNameInfo *myinfo = NULL;
-	char *query_site = NULL;
 	bool first = true;
 	struct netr_DsRGetDCNameInfo *first_info = NULL;
 
@@ -1103,7 +1102,7 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 		  "domain_guid: %s, site_name: %s, flags: 0x%08x\n",
 		  domain_name,
 		  domain_guid ? GUID_string(mem_ctx, domain_guid) : "(null)",
-		  site_name, flags));
+		  site_name ? site_name : "(null)", flags));
 
 	*info = NULL;
 
@@ -1112,18 +1111,12 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	if ((site_name == NULL) || (site_name[0] == '\0')) {
-		query_site = sitename_fetch(domain_name);
-	} else {
-		query_site = SMB_STRDUP(site_name);
-	}
-
 	if (flags & DS_FORCE_REDISCOVERY) {
 		goto rediscover;
 	}
 
 	status = dsgetdcname_cached(mem_ctx, msg_ctx, domain_name, domain_guid,
-				    flags, query_site, &myinfo);
+				    flags, site_name, &myinfo);
 	if (NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -1134,12 +1127,10 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 
  rediscover:
 	status = dsgetdcname_rediscover(mem_ctx, msg_ctx, domain_name,
-					domain_guid, flags, query_site,
+					domain_guid, flags, site_name,
 					&myinfo);
 
  done:
-	SAFE_FREE(query_site);
-
 	if (!NT_STATUS_IS_OK(status)) {
 		if (!first) {
 			*info = first_info;
@@ -1154,7 +1145,7 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
 		first = false;
 		first_info = myinfo;
 		/* TODO: may use the next_closest_site here */
-		query_site = SMB_STRDUP(myinfo->client_site_name);
+		site_name = myinfo->client_site_name;
 		goto rediscover;
 	}
 
@@ -1176,11 +1167,24 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
 		     uint32_t flags,
 		     struct netr_DsRGetDCNameInfo **info)
 {
-	return dsgetdcname_internal(mem_ctx,
+	NTSTATUS status;
+	char *query_site = NULL;
+
+	if ((site_name == NULL) || (site_name[0] == '\0')) {
+		query_site = sitename_fetch(domain_name);
+	} else {
+		query_site = SMB_STRDUP(site_name);
+	}
+
+	status = dsgetdcname_internal(mem_ctx,
 				msg_ctx,
 				domain_name,
 				domain_guid,
-				site_name,
+				query_site,
 				flags,
 				info);
+
+	SAFE_FREE(query_site);
+
+	return status;
 }
-- 
1.8.4


>From 9cdc8a8fd4cbad22c463ee9d83548fa2cafd6bf6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Sep 2013 12:20:52 -0700
Subject: [PATCH 4/4] Move the retry logic when site_name is passed in a NULL
 or "" to the wrapper function.

Bug 5917 - Samba does not work on site with Read Only Domain Controller

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/libsmb/dsgetdcname.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 2318dfb..43b3fd8 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -1168,12 +1168,14 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
 		     struct netr_DsRGetDCNameInfo **info)
 {
 	NTSTATUS status;
-	char *query_site = NULL;
+	const char *query_site = NULL;
+	char *ptr_to_free = NULL;
 
 	if ((site_name == NULL) || (site_name[0] == '\0')) {
-		query_site = sitename_fetch(domain_name);
+		ptr_to_free = sitename_fetch(domain_name);
+		query_site = ptr_to_free;
 	} else {
-		query_site = SMB_STRDUP(site_name);
+		query_site = site_name;
 	}
 
 	status = dsgetdcname_internal(mem_ctx,
@@ -1184,7 +1186,22 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
 				flags,
 				info);
 
-	SAFE_FREE(query_site);
+	SAFE_FREE(ptr_to_free);
+
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND)) {
+		return status;
+	}
+
+	/* Should we try again with site_name == NULL ? */
+	if ((site_name == NULL) || (site_name[0] == '\0')) {
+		status = dsgetdcname_internal(mem_ctx,
+					msg_ctx,
+					domain_name,
+					domain_guid,
+					NULL,
+					flags,
+					info);
+	}
 
 	return status;
 }
-- 
1.8.4



More information about the samba-technical mailing list