[PATCHES BUG 13312] winbindd: Do not ignore domain in LOOKUPNAME request
Christof Schmitt
cs at samba.org
Thu Apr 5 18:19:35 UTC 2018
Andreas,
did you have a chance to see the additional patches to fix the existing
testcases?
Christof
On Fri, Mar 30, 2018 at 09:43:31PM -0700, Christof Schmitt via samba-technical wrote:
> On Fri, Mar 16, 2018 at 10:12:42AM -0700, Christof Schmitt via samba-technical wrote:
> > On Fri, Mar 16, 2018 at 08:24:06AM +0100, Andreas Schneider via samba-technical wrote:
> > > On Thursday, 15 March 2018 21:01:10 CET Christof Schmitt wrote:
> > > > Are you ok with pushing these changes?
> > >
> > > Yes, please push.
> >
> > Thank you. Pushed with your RB.
>
> Three more existing tests needed to be adapted. They basically tried to
> issue a winbindd query for something like domain\domain\user which
> succeeded before, but is now rejected. Please see updated patch series
> with the test fixes. That finally passes a private autobuild.
>
> Christof
> From 2216f68ae10de2590275467edca93a0575936d48 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Fri, 16 Mar 2018 13:52:14 -0700
> Subject: [PATCH 1/5] test_smbclient_s3.sh: Use correct separator in "list
> with backup privilege" test
>
> Samba selftest uses the forward slash as winbind separator and in the
> USERNAME passed to the test. "net sam rights" expect the backslash. Map
> the separator used in selftest to a backslash to avoid creating an
> incorrect username DOMAIN\DOMAIN/USERNAME.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13312
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/script/tests/test_smbclient_s3.sh | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
> index 03f7b27..cc0d69d 100755
> --- a/source3/script/tests/test_smbclient_s3.sh
> +++ b/source3/script/tests/test_smbclient_s3.sh
> @@ -643,13 +643,17 @@ test_backup_privilege_list()
> {
> tmpfile=$PREFIX/smbclient_backup_privilege_list
>
> + # selftest uses the forward slash as a separator, but "net sam rights
> + # grant" requires the backslash separator
> + USER_TMP=$(printf '%s' "$USERNAME" | tr '/' '\\')
> +
> # If we don't have a DOMAIN component to the username, add it.
> - echo "$USERNAME" | grep '\\' 2>&1
> + printf '%s' "$USER_TMP" | grep '\\' 2>&1
> ret=$?
> if [ $ret != 0 ] ; then
> - priv_username="$DOMAIN\\$USERNAME"
> + priv_username="$DOMAIN\\$USER_TMP"
> else
> - priv_username=$USERNAME
> + priv_username="$USER_TMP"
> fi
>
> $NET sam rights grant $priv_username SeBackupPrivilege 2>&1
> --
> 1.8.3.1
>
>
> From 7036c7835b8b816b8acb438618820eedb6133b78 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Fri, 30 Mar 2018 14:28:46 -0700
> Subject: [PATCH 2/5] nsswitch: Fix wbcListUsers test
>
> With an AD DC, wbcListUsers returns the users in the DOMAIN SEPARATOR
> USERNAME format. The test then calls wbcLookupName with the domain name
> and the previous string (including domain and separator) as username.
> Fix this by passing the correct username and adding some additional
> checks.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13312
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> nsswitch/libwbclient/tests/wbclient.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/nsswitch/libwbclient/tests/wbclient.c b/nsswitch/libwbclient/tests/wbclient.c
> index e80afc4..8c532bb 100644
> --- a/nsswitch/libwbclient/tests/wbclient.c
> +++ b/nsswitch/libwbclient/tests/wbclient.c
> @@ -296,6 +296,7 @@ static bool test_wbc_users(struct torture_context *tctx)
> char *name = NULL;
> char *sid_string = NULL;
> wbcErr ret = false;
> + char separator;
>
> torture_assert_wbc_ok(tctx, wbcInterfaceDetails(&details),
> "%s", "wbcInterfaceDetails failed");
> @@ -306,6 +307,7 @@ static bool test_wbc_users(struct torture_context *tctx)
> ret,
> fail,
> "Failed to allocate domain_name");
> + separator = details->winbind_separator;
> wbcFreeMemory(details);
> details = NULL;
>
> @@ -323,9 +325,38 @@ static bool test_wbc_users(struct torture_context *tctx)
> struct wbcDomainSid sid;
> enum wbcSidType name_type;
> uint32_t num_sids;
> + const char *user;
> + char *c;
> +
> + c = strchr(users[i], separator);
> +
> + if (c == NULL) {
> + /*
> + * NT4 DC
> + * user name does not contain DOMAIN SEPARATOR prefix.
> + */
> +
> + user = users[i];
> + } else {
> + /*
> + * AD DC
> + * user name starts with DOMAIN SEPARATOR prefix.
> + */
> + const char *dom;
> +
> + *c = '\0';
> + dom = users[i];
> + user = c + 1;
> +
> + torture_assert_str_equal_goto(tctx, dom, domain_name,
> + ret, fail, "Domain part "
> + "of user name does not "
> + "match domain name.\n");
> + }
>
> torture_assert_wbc_ok_goto_fail(tctx,
> - wbcLookupName(domain_name, users[i], &sid, &name_type),
> + wbcLookupName(domain_name, user,
> + &sid, &name_type),
> "wbcLookupName of %s failed",
> users[i]);
> torture_assert_int_equal_goto(tctx,
> --
> 1.8.3.1
>
>
> From fd1f7288a41da986f765bcbc7e9b060ae3141445 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Fri, 30 Mar 2018 14:35:03 -0700
> Subject: [PATCH 3/5] nsswitch: Fix wbcListGroups test
>
> With an AD DC, wbcListGroups returns the users in the DOMAIN SEPARATOR
> GROUPNAME format. The test then calls wbcLookupName with the domain
> name and the previous string (including domain and separator) as
> username. Fix this by passing the correct username and adding some
> additional checks.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13312
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> nsswitch/libwbclient/tests/wbclient.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/nsswitch/libwbclient/tests/wbclient.c b/nsswitch/libwbclient/tests/wbclient.c
> index 8c532bb..d107942 100644
> --- a/nsswitch/libwbclient/tests/wbclient.c
> +++ b/nsswitch/libwbclient/tests/wbclient.c
> @@ -430,6 +430,7 @@ static bool test_wbc_groups(struct torture_context *tctx)
> char *domain = NULL;
> char *name = NULL;
> char *sid_string = NULL;
> + char separator;
>
> torture_assert_wbc_ok(tctx, wbcInterfaceDetails(&details),
> "%s", "wbcInterfaceDetails failed");
> @@ -440,6 +441,7 @@ static bool test_wbc_groups(struct torture_context *tctx)
> ret,
> fail,
> "Failed to allocate domain_name");
> + separator = details->winbind_separator;
> wbcFreeMemory(details);
> details = NULL;
>
> @@ -456,10 +458,39 @@ static bool test_wbc_groups(struct torture_context *tctx)
> for (i=0; i < MIN(num_groups,100); i++) {
> struct wbcDomainSid sid;
> enum wbcSidType name_type;
> + const char *group;
> + char *c;
> +
> + c = strchr(groups[i], separator);
> +
> + if (c == NULL) {
> + /*
> + * NT4 DC
> + * group name does not contain DOMAIN SEPARATOR prefix.
> + */
> +
> + group = groups[i];
> + } else {
> + /*
> + * AD DC
> + * group name starts with DOMAIN SEPARATOR prefix.
> + */
> + const char *dom;
> +
> +
> + *c = '\0';
> + dom = groups[i];
> + group = c + 1;
> +
> + torture_assert_str_equal_goto(tctx, dom, domain_name,
> + ret, fail, "Domain part "
> + "of group name does not "
> + "match domain name.\n");
> + }
>
> torture_assert_wbc_ok_goto_fail(tctx,
> wbcLookupName(domain_name,
> - groups[i],
> + group,
> &sid,
> &name_type),
> "wbcLookupName for %s failed",
> --
> 1.8.3.1
>
>
> From 99db7ff86ba963e5aaa51839b71b03336ff56173 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 4/5] 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>
> Reviewed-by: Andreas Schneider <asn 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 a2aeed2..6ef61db 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.NtlmDisabledTests.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 a9b7c20..a22d5e6 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -210,6 +210,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"])
> --
> 1.8.3.1
>
>
> From 15687b07fcb62eb2064b914457b67bd4865159a2 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 28 Feb 2018 12:05:34 -0700
> Subject: [PATCH 5/5] winbindd: Do not ignore domain in LOOKUPNAME request
>
> A LOOKUPNAME request with a domain and a name containing a winbind
> separator character would return the result for the joined domain,
> instead of the specified domain.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13312
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> Reviewed-by: Andreas Schneider <asn at samba.org>
> ---
> selftest/knownfail | 2 --
> source3/winbindd/winbindd_lookupname.c | 33 +++++++++++++++++++++------------
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 6ef61db..a2aeed2 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -343,5 +343,3 @@
> # 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.NtlmDisabledTests.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/winbindd/winbindd_lookupname.c b/source3/winbindd/winbindd_lookupname.c
> index 1be29fd..b022691 100644
> --- a/source3/winbindd/winbindd_lookupname.c
> +++ b/source3/winbindd/winbindd_lookupname.c
> @@ -35,7 +35,8 @@ struct tevent_req *winbindd_lookupname_send(TALLOC_CTX *mem_ctx,
> {
> struct tevent_req *req, *subreq;
> struct winbindd_lookupname_state *state;
> - char *domname, *name, *p;
> + const char *domname = NULL, *name = NULL;
> + char *p = NULL;
>
> req = tevent_req_create(mem_ctx, &state,
> struct winbindd_lookupname_state);
> @@ -49,17 +50,25 @@ struct tevent_req *winbindd_lookupname_send(TALLOC_CTX *mem_ctx,
> sizeof(request->data.name.dom_name)-1]='\0';
> request->data.name.name[sizeof(request->data.name.name)-1]='\0';
>
> - /* cope with the name being a fully qualified name */
> - p = strstr(request->data.name.name, lp_winbind_separator());
> - if (p) {
> - *p = 0;
> - domname = request->data.name.name;
> - name = p+1;
> - } else if ((p = strchr(request->data.name.name, '@')) != NULL) {
> - /* upn */
> - domname = p + 1;
> - *p = 0;
> - name = request->data.name.name;
> + if (strlen(request->data.name.dom_name) == 0) {
> + /* cope with the name being a fully qualified name */
> + 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;
> + }
> + }
> } else {
> domname = request->data.name.dom_name;
> name = request->data.name.name;
> --
> 1.8.3.1
>
More information about the samba-technical
mailing list