[PATCH] Fix bug #11555 - lookup_names() looks up qualified names as unqualified.
Jeremy Allison
jra at samba.org
Thu Oct 15 16:09:00 UTC 2015
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.
> 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
> 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