[PATCH] Fix bug #11555 - lookup_names() looks up qualified names as unqualified.
Jeremy Allison
jra at samba.org
Thu Oct 15 17:10:31 UTC 2015
On Thu, Oct 15, 2015 at 09:09:00AM -0700, Jeremy Allison wrote:
> On Thu, Oct 15, 2015 at 04:47:33PM +0200, Ralph Boehme wrote:
> > 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?
>
> No this fix won't work.
>
> You're only checking for "NT Authority" if LOOKUP_NAME_ISOLATED is set.
>
> "NT Authority\Anonymous User" isn't an isolated name, so
> you need to do the check for "NT Authority" *before* you
> bail using:
>
> /*
> * If we're told not to look up 'isolated' names then we're
> * done.
> */
> if (!(flags & LOOKUP_NAME_ISOLATED)) {
>
> I'll re-post a patch.
Here's what I'm proposing as a "final" fix. Starts
with Ralph's fix for the bug Uri spotted, followed by
a modified version of Ralph's cleanup that will actually
pass the "NT Authority\Anonymous User" test :-).
(The key is we must check for "NT Authority" in the
code *before* we go into the unqualified name lookup,
as it's not an unqualified name :-). If we don't
pass LOOKUP_NAME_ISOLATED "NT Authority" should
still be resolved.
Reviews welcome !
Jeremy.
-------------- next part --------------
From 049a4072574e7f3114596fda840f3650ce264804 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>
Reviewed-by: Jeremy Allison <jra 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.6.0.rc2.230.g3dd15c0
From ba92e7db1d2f7fffd2617ee44c1e8779120e3c48 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 15 Oct 2015 09:20:58 -0700
Subject: [PATCH 2/3] s3: lsa: lookup_name() logic for unqualified (no DOMAIN\
component) names is incorrect.
Change so we only use unqualified name lookup logic if
domain component = "" and LOOKUP_NAME_ISOLATED flag is
passed in.
Remember to search for "NT Authority" *before* going
into unqualified name lookup logic.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11555
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/passdb/lookup_sid.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
index 3f99ee1..1ffd657 100644
--- a/source3/passdb/lookup_sid.c
+++ b/source3/passdb/lookup_sid.c
@@ -140,7 +140,31 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
return false;
}
- if ((domain[0] == '\0') && (!(flags & LOOKUP_NAME_ISOLATED))) {
+ /*
+ * Finally check for a well known domain name ("NT Authority"),
+ * this is taken care if in lookup_wellknown_name().
+ */
+ if ((domain[0] != '\0') &&
+ (flags & LOOKUP_NAME_WKN) &&
+ lookup_wellknown_name(tmp_ctx, name, &sid, &domain))
+ {
+ type = SID_NAME_WKN_GRP;
+ goto ok;
+ }
+
+ /*
+ * 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;
+ }
+
+ /*
+ * No domain names beyond this point
+ */
+ if (domain[0] != '\0') {
TALLOC_FREE(tmp_ctx);
return false;
}
@@ -152,6 +176,11 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
/* 1. well-known names */
+ /*
+ * Check for well known names without a domain name.
+ * e.g. \Creator Owner.
+ */
+
if ((flags & LOOKUP_NAME_WKN) &&
lookup_wellknown_name(tmp_ctx, name, &sid, &domain))
{
--
2.6.0.rc2.230.g3dd15c0
From 01452777198fc62238974d7d7479105f98a81937 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>
Reviewed-by: Ralph Boehme <slow 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.6.0.rc2.230.g3dd15c0
More information about the samba-technical
mailing list