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

Christof Schmitt cs at samba.org
Wed Feb 28 21:29:02 UTC 2018


See the bugzilla for some more details. The problem here is that
winbindd returns misleading information for a trusted or invalid domain
when the domain\user syntax is wrong. With the default backslash
separator and the approach of adding backslashes until the escaping
works, this can easily lead to wrong SIDs being used.

Christof
-------------- next part --------------
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.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 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"])
-- 
1.8.3.1


From 50a7b975f5955c55c6af0100ad57e56221a44f68 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 2/2] 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>
---
 selftest/knownfail                     |  2 --
 source3/winbindd/winbindd_lookupname.c | 30 ++++++++++++++++++------------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 06d4cd6..710fd33 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..b486469 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, *name;
+	char *p;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct winbindd_lookupname_state);
@@ -49,17 +50,22 @@ 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) {
+			*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;
+		} 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