[PATCH] Fix bug #11555 - lookup_names() looks up qualified names as unqualified.

Ralph Boehme rb at sernet.de
Thu Oct 15 14:47:33 UTC 2015


On Thu, Oct 15, 2015 at 09:09:45AM +0300, Uri Simchoni wrote:
> The use of the "NT Authority" literal string got me suspicious :)
> 
> I think this code would accept "NT Authority\Creator Group" and translate it
> into "\Creator Group" with a SID that's not "NT Authority".

:) Good catch!

> 
> I would also be happier if:
> 1. The handling of NT Authority was placed next to the builtin stuff - put
> the builtin and well known near each other - they are almost the same from
> the perspective of most of the code and where I see builtin I also wonder
> about well-known.

maybe something like this?

This is a WIP patch, the second commit needs a nice commit message.

Cheerio!
-slow

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From bf45ded279b71c4dab9ed2453d3bbed8a832af53 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Oct 2015 12:35:26 +0200
Subject: [PATCH 1/3] s3:lib: validate domain name in lookup_wellknown_name()

If domain argument is not an empty string, only search the matching
wellknown domain name.

As the only wellknown domain with a name is "NT Authority", passing ""
to lookup_wellknown_name() will search all domains inlcuding "NT
Authority".

Passing "NT Authority" otoh will obviously only search that domain.

This change makes lookup_wellknown_name() behave like this:

in domain         | in name       | ok | out sid | out domain
========================================================
                    Dialup          +    S-1-5-1   NT Authority
NT Authority        Dialup          +    S-1-5-1   NT Authority
Creator Authority   Dialup          -    -         -
                    Creator Owner   +    S-1-3-0   ""
Creator Authority   Creator Owner   -    -         -
NT Authority        Creator Owner   -    -         -

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/lib/util_wellknown.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/source3/lib/util_wellknown.c b/source3/lib/util_wellknown.c
index 0f627d1..a3db9ab 100644
--- a/source3/lib/util_wellknown.c
+++ b/source3/lib/util_wellknown.c
@@ -154,16 +154,23 @@ bool lookup_wellknown_sid(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
 ***************************************************************************/
 
 bool lookup_wellknown_name(TALLOC_CTX *mem_ctx, const char *name,
-			   struct dom_sid *sid, const char **domain)
+			   struct dom_sid *sid, const char **pdomain)
 {
 	int i, j;
+	const char *domain = *pdomain;
 
-	DEBUG(10,("map_name_to_wellknown_sid: looking up %s\n", name));
+	DEBUG(10,("map_name_to_wellknown_sid: looking up %s\\%s\n", domain, name));
 
 	for (i=0; special_domains[i].sid != NULL; i++) {
 		const struct rid_name_map *users =
 			special_domains[i].known_users;
 
+		if (domain[0] != '\0') {
+			if (!strequal(domain, special_domains[i].name)) {
+				continue;
+			}
+		}
+
 		if (users == NULL)
 			continue;
 
@@ -171,7 +178,7 @@ bool lookup_wellknown_name(TALLOC_CTX *mem_ctx, const char *name,
 			if ( strequal(users[j].name, name) ) {
 				sid_compose(sid, special_domains[i].sid,
 					    users[j].rid);
-				*domain = talloc_strdup(
+				*pdomain = talloc_strdup(
 					mem_ctx, special_domains[i].name);
 				return True;
 			}
-- 
2.1.0


From c86a33a09f09580a417c53da518cd233bb84ae3c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Oct 2015 12:34:59 +0200
Subject: [PATCH 2/3] WIP: lookup well-known domains

---
 source3/passdb/lookup_sid.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
index 3f99ee1..e6c817d 100644
--- a/source3/passdb/lookup_sid.c
+++ b/source3/passdb/lookup_sid.c
@@ -140,7 +140,11 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 		return false;
 	}
 
-	if ((domain[0] == '\0') && (!(flags & LOOKUP_NAME_ISOLATED))) {
+	/*
+	 * If we're told not to look up 'isolated' names then we're
+	 * done.
+	 */
+	if (!(flags & LOOKUP_NAME_ISOLATED)) {
 		TALLOC_FREE(tmp_ctx);
 		return false;
 	}
@@ -152,6 +156,12 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 
 	/* 1. well-known names */
 
+	/*
+	 * well-known names are the only ones at this point that allow
+	 * a domain name ("NT Authority"), this is taken care if in
+	 * lookup_wellknown_name().
+	 */
+
 	if ((flags & LOOKUP_NAME_WKN) &&
 	    lookup_wellknown_name(tmp_ctx, name, &sid, &domain))
 	{
@@ -159,6 +169,14 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 		goto ok;
 	}
 
+	/*
+	 * No domain names beyond this point
+	 */
+	if (domain[0] != '\0') {
+		TALLOC_FREE(tmp_ctx);
+		return false;
+	}
+
 	/* 2. Builtin domain as such */
 
 	if ((flags & (LOOKUP_NAME_BUILTIN|LOOKUP_NAME_REMOTE)) &&
-- 
2.1.0


From a1c54c9febd340a62968e2780b678289539cebe2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 14 Oct 2015 11:20:08 -0700
Subject: [PATCH 3/3] s3: test: Fix standalone valid users fileserver test.

Test was originally added for bug #11320. At the time
I remarked the only way I could get this to reproduce
the issue was to use "+WORKGROUP\userdup" instead of
just "+userdup" (which was the actual problem reported),
but I didn't investigage enough to discover the underlying
problem which is actually bug:

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

(lookup_names() logic for unqualified (no DOMAIN\
component) names is incorrect). On a standalone
fileserver "WORKGROUP\name" should not resolve,
but "NETBIOS-NAME\name" and just "name" should.

This corrects the test now that lookups for unqualified
names are now being done correctly.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index de4346e..15423fe 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -608,7 +608,7 @@ sub setup_fileserver($$)
 	dfree command = $srcdir_abs/testprogs/blackbox/dfree.sh
 [valid-users-access]
 	path = $valid_users_sharedir
-	valid users = +SAMBA-TEST/userdup
+	valid users = +userdup
 	";
 
 	my $vars = $self->provision($path,
-- 
2.1.0



More information about the samba-technical mailing list