[PATCHES BUG 13312] winbindd: Do not ignore domain in LOOKUPNAME request

Andreas Schneider asn at samba.org
Fri Mar 16 07:24:06 UTC 2018


On Thursday, 15 March 2018 21:01:10 CET Christof Schmitt wrote:
> Are you ok with pushing these changes?

Yes, please push.

> Christof
> 
> On Fri, Mar 09, 2018 at 09:39:33AM -0700, Christof Schmitt via samba-
technical wrote:
> > On Fri, Mar 09, 2018 at 09:23:26AM +0100, Andreas Schneider wrote:
> > > On Wednesday, 7 March 2018 23:19:46 CET Christof Schmitt via
> > > samba-technical> > 
> > > wrote:
> > > > Is anybody interested in reviewing this change?
> > > 
> > > -	char *domname, *name, *p;
> > > +	const char *domname, *name;
> > > +	char *p;
> > > 
> > > If you're already modifying those, please initialize the the pointers
> > > with
> > > NULL.
> > > 
> > > +		if (p) {
> > > +			*p = 0;
> > > 
> > > should be
> > > 
> > > if (p != NULL) {
> > > 
> > > 	*p = '\0';
> > > 
> > > I think I would prefer if the block would look like this:
> > > 
> > > +		p = strstr(request->data.name.name, lp_winbind_separator());
> > > +		if (p != NULL) {
> > > +			*p = '\0';
> > > +			domname = request->data.name.name;
> > > +			name = p + 1;
> > > +		} else {
> > > +			p = strchr(request->data.name.name, '@');
> > > +			if (p != NULL) {
> > > +				/* upn */
> > > +				domname = p + 1;
> > > +				*p = '\0';
> > > +				name = request->data.name.name;
> > > +			} else {
> > > +				domname = "";
> > > +				name = request->data.name.name;
> > > +			}
> > > +		}
> > 
> > Thank you. See attached patch with the updates.
> > 
> > Christof
> > 
> > From 719f498329c4174d37febe76f696b9381ac2deea Mon Sep 17 00:00:00 2001
> > From: Christof Schmitt <cs at samba.org>
> > Date: Wed, 28 Feb 2018 13:10:43 -0700
> > Subject: [PATCH 1/2] Add test for wbinfo name lookup
> > 
> > This demonstrates that wbinfo -n / --name-to-sid returns information
> > instead of failing the request. More specifically the query for
> > INVALIDDOMAIN//user returns the user SID for the joined domain, instead
> > of failing the request.
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=13312
> > 
> > Signed-off-by: Christof Schmitt <cs at samba.org>
> > ---
> > 
> >  nsswitch/tests/test_wbinfo_name_lookup.sh | 40
> >  +++++++++++++++++++++++++++++++ selftest/knownfail                      
> >   |  2 ++
> >  source3/selftest/tests.py                 |  4 ++++
> >  3 files changed, 46 insertions(+)
> >  create mode 100755 nsswitch/tests/test_wbinfo_name_lookup.sh
> > 
> > diff --git a/nsswitch/tests/test_wbinfo_name_lookup.sh
> > b/nsswitch/tests/test_wbinfo_name_lookup.sh new file mode 100755
> > index 0000000..696e25b
> > --- /dev/null
> > +++ b/nsswitch/tests/test_wbinfo_name_lookup.sh
> > @@ -0,0 +1,40 @@
> > +#!/bin/sh
> > +# Blackbox test for wbinfo name lookup
> > +if [ $# -lt 2 ]; then
> > +cat <<EOF
> > +Usage: test_wbinfo.sh DOMAIN DC_USERNAME
> > +EOF
> > +exit 1;
> > +fi
> > +
> > +DOMAIN=$1
> > +DC_USERNAME=$2
> > +shift 2
> > +
> > +failed=0
> > +sambabindir="$BINDIR"
> > +wbinfo="$VALGRIND $sambabindir/wbinfo"
> > +
> > +. `dirname $0`/../../testprogs/blackbox/subunit.sh
> > +
> > +# Correct query is expected to work
> > +testit "name-to-sid.single-separator" \
> > +       $wbinfo -n $DOMAIN/$DC_USERNAME || \
> > +	failed=$(expr $failed + 1)
> > +
> > +# Two separator characters should fail
> > +testit_expect_failure "name-to-sid.double-separator" \
> > +		      $wbinfo -n $DOMAIN//$DC_USERNAME || \
> > +	failed=$(expr $failed + 1)
> > +
> > +# Invalid domain is expected to fail
> > +testit_expect_failure "name-to-sid.invalid-domain" \
> > +		      $wbinfo -n INVALID/$DC_USERNAME || \
> > +	failed=$(expr $failed + 1)
> > +
> > +# Invalid domain with two separator characters is expected to fail
> > +testit_expect_failure "name-to-sid.double-separator-invalid-domain" \
> > +		      $wbinfo -n INVALID//$DC_USERNAME || \
> > +	failed=$(expr $failed + 1)
> > +
> > +exit $failed
> > diff --git a/selftest/knownfail b/selftest/knownfail
> > index 710fd33..06d4cd6 100644
> > --- a/selftest/knownfail
> > +++ b/selftest/knownfail
> > @@ -343,3 +343,5 @@
> > 
> >  # Disabling NTLM means you can't use samr to change the password
> >  ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.
> >  test_samr_change_password\(ktest\)
> >  ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).ntlmdisabled.NtlmDisab
> >  ledTests.test_ntlm_connection\(ad_dc_no_ntlm\)> 
> > +samba3.wbinfo_name_lookup.name-to-sid.double-separator\(ad_member\)
> > +samba3.wbinfo_name_lookup.name-to-sid.double-separator-invalid-domain\(ad
> > _member\) diff --git a/source3/selftest/tests.py
> > b/source3/selftest/tests.py index 89c8d60..8af0974 100755
> > --- a/source3/selftest/tests.py
> > +++ b/source3/selftest/tests.py
> > 
> > @@ -206,6 +206,10 @@ for env in ["nt4_member", "ad_member"]:
> >  env = "ad_member"
> >  t = "--krb5auth=$DOMAIN/$DC_USERNAME%$DC_PASSWORD"
> >  plantestsuite("samba3.wbinfo_simple.(%s:local).%s" % (env, t), "%s:local"
> >  % env, [os.path.join(srcdir(), "nsswitch/tests/test_wbinfo_simple.sh"),
> >  t])> 
> > +plantestsuite("samba3.wbinfo_name_lookup", env,
> > +              [ os.path.join(srcdir(),
> > +                            "nsswitch/tests/test_wbinfo_name_lookup.sh"),
> > +                '$DOMAIN', '$DC_USERNAME' ])
> > 
> >  t = "WBCLIENT-MULTI-PING"
> >  plantestsuite("samba3.smbtorture_s3.%s" % t, env,
> >  [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t,
> >  '//foo/bar', '""', '""', smbtorture3, ""])
> >  plantestsuite("samba3.substitutions", env, [os.path.join(samba3srcdir,
> >  "script/tests/test_substitutions.sh"), "$SERVER", "alice", "Secret007",
> >  "$PREFIX"])


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org





More information about the samba-technical mailing list