wbinfo -i output domain realm vs. ntdomain before login

Andreas Schneider asn at samba.org
Mon Apr 23 14:44:37 UTC 2018


On Friday, 20 April 2018 06:52:58 CEST Stefan Metzmacher wrote:
> Hi Samuel,
> 
> > I had a look to the attached patches in bugzilla. The LSA LookupNames
> > is called when the winbind cache is cold and it returns all the
> > necessary information (the referenced domain name and domain SID to
> > which the looked up names belongs), so why can't we pass this up to the
> > caller and use it instead checking the given name format to lookup the
> > domain name after obtaining the SID?
> > 
> > What do you think about this patch?
> 
> It guess it doesn't handle a case the following:
> 
> userPrincipalName: some.one at example.com
> sAMAccountName: some
> 
> REALM: AD.EXAMPLE.PRIVATE
> DOMAIN: ADDOM
> 
> If you ask for 'some.one at example.com' you should get
> back 'ADDOM\some' instead of 'ADDOM\some.one'.
> 
> We may need to avoid using wcache_save_sid_to_name()
> within wb_cache_name_to_sid().

Attached are tests for UPNs and fixes for it.


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From 4e2a4957e2e8f807ac2f73646bcb10946bdbfc96 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 20 Apr 2018 11:24:30 +0200
Subject: [PATCH 1/5] nsswitch: Add a test looking up the user using the upn

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13369

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 nsswitch/tests/test_wbinfo_name_lookup.sh | 9 +++++++--
 source3/selftest/tests.py                 | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/nsswitch/tests/test_wbinfo_name_lookup.sh b/nsswitch/tests/test_wbinfo_name_lookup.sh
index 696e25b3a2a..a8fd5ec4d99 100755
--- a/nsswitch/tests/test_wbinfo_name_lookup.sh
+++ b/nsswitch/tests/test_wbinfo_name_lookup.sh
@@ -8,8 +8,9 @@ exit 1;
 fi
 
 DOMAIN=$1
-DC_USERNAME=$2
-shift 2
+REALM=$2
+DC_USERNAME=$3
+shift 3
 
 failed=0
 sambabindir="$BINDIR"
@@ -22,6 +23,10 @@ testit "name-to-sid.single-separator" \
        $wbinfo -n $DOMAIN/$DC_USERNAME || \
 	failed=$(expr $failed + 1)
 
+testit "name-to-sid.upn" \
+       $wbinfo -n $DC_USERNAME@$REALM || \
+	failed=$(expr $failed + 1)
+
 # Two separator characters should fail
 testit_expect_failure "name-to-sid.double-separator" \
 		      $wbinfo -n $DOMAIN//$DC_USERNAME || \
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 06bda707ddb..5522f4b35d1 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -214,7 +214,7 @@ plantestsuite("samba3.wbinfo_simple.(%s:local).%s" % (env, t), "%s:local" % env,
 plantestsuite("samba3.wbinfo_name_lookup", env,
               [ os.path.join(srcdir(),
                             "nsswitch/tests/test_wbinfo_name_lookup.sh"),
-                '$DOMAIN', '$DC_USERNAME' ])
+                '$DOMAIN', '$REALM', '$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"])
-- 
2.16.3


>From 2887602b86e89f5a102513b78fc3d91f143fd896 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 20 Apr 2018 09:38:24 +0200
Subject: [PATCH 2/5] selftest: Add a user with a different userPrincipalName

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13369

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba4.pm | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 51a175b25e8..5353779292e 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -877,7 +877,7 @@ userPrincipalName: testdenied_upn\@$ctx->{realm}.upn
 	}
 
 	# Create to users alice and bob!
-	my $user_account_array = ["alice", "bob"];
+	my $user_account_array = ["alice", "bob", "jane"];
 
 	foreach my $user_account (@{$user_account_array}) {
 		my $samba_tool_cmd = "";
@@ -892,6 +892,23 @@ userPrincipalName: testdenied_upn\@$ctx->{realm}.upn
 		}
 	}
 
+	my $ldbmodify = "";
+	$ldbmodify .= "KRB5_CONFIG=\"$ret->{KRB5_CONFIG}\" ";
+	$ldbmodify .= "KRB5CCNAME=\"$ret->{KRB5_CCACHE}\" ";
+	$ldbmodify .= Samba::bindir_path($self, "ldbmodify");
+
+	my $base_dn = "DC=".join(",DC=", split(/\./, $ctx->{realm}));
+	my $user_dn = "cn=jane,cn=users,$base_dn";
+
+	open(LDIF, "|$ldbmodify -H $ctx->{privatedir}/sam.ldb");
+	print LDIF "dn: $user_dn
+changetype: modify
+replace: userPrincipalName
+userPrincipalName: jane.doe\@$ctx->{realm}
+-
+";
+	close(LDIF);
+
 	return $ret;
 }
 
-- 
2.16.3


>From f54859bf99e5984013e73a80a171104858692339 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 20 Apr 2018 11:20:44 +0200
Subject: [PATCH 3/5] nsswitch:tests: Add test for wbinfo --user-info

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13369

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 nsswitch/tests/test_wbinfo_user_info.sh | 77 +++++++++++++++++++++++++++++++++
 source3/selftest/tests.py               |  4 ++
 2 files changed, 81 insertions(+)
 create mode 100755 nsswitch/tests/test_wbinfo_user_info.sh

diff --git a/nsswitch/tests/test_wbinfo_user_info.sh b/nsswitch/tests/test_wbinfo_user_info.sh
new file mode 100755
index 00000000000..8d49b2a4f22
--- /dev/null
+++ b/nsswitch/tests/test_wbinfo_user_info.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+# Blackbox test for wbinfo lookup for account name and upn
+# Copyright (c) 2018 Andreas Schneider <asn at samba.org>
+
+if [ $# -lt 5 ]; then
+cat <<EOF
+Usage: $(basename $0) DOMAIN REALM USERNAME1 UPN_NAME1 USERNAME2 UPN_NAME2
+EOF
+exit 1;
+fi
+
+DOMAIN=$1
+REALM=$2
+USERNAME1=$3
+UPN_NAME1=$4
+USERNAME2=$5
+UPN_NAME2=$6
+shift 6
+
+failed=0
+
+samba_bindir="$BINDIR"
+wbinfo_tool="$VALGRIND $samba_bindir/wbinfo"
+
+UPN1="$UPN_NAME1@$REALM"
+UPN2="$UPN_NAME2@$REALM"
+
+. $(dirname $0)/../../testprogs/blackbox/subunit.sh
+
+test_user_info()
+{
+	local cmd out ret user domain upn userinfo
+
+	domain="$1"
+	user="$2"
+	upn="$3"
+
+	if [ $# -lt 3 ]; then
+		userinfo="$domain/$user"
+	else
+		userinfo="$upn"
+	fi
+
+	cmd='$wbinfo_tool --user-info $userinfo'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "failed to lookup $userinfo"
+		echo "$out"
+		return 1
+	fi
+
+	echo "$out" | grep "$domain/$user:.*:.*:.*::/home/$domain/Domain Users/$user"
+	ret=$?
+	if [ $ret != 0 ]; then
+		echo "failed to lookup $userinfo"
+		echo "$out"
+		return 1
+	fi
+
+	return 0
+}
+
+testit "name_to_sid.domain.$USERNAME1" $wbinfo_tool --name-to-sid $DOMAIN/$USERNAME1 || failed=$(expr $failed + 1)
+testit "name_to_sid.upn.$UPN_NAME1" $wbinfo_tool --name-to-sid $UPN1 || failed=$(expr $failed + 1)
+
+testit "user_info.domain.$USERNAME1" test_user_info $DOMAIN $USERNAME1 || failed=$(expr $failed + 1)
+testit "user_info.upn.$UPN_NAME1" test_user_info $DOMAIN $USERNAME1 $UPN1 || failed=$(expr $failed + 1)
+
+testit "name_to_sid.domain.$USERNAME2" $wbinfo_tool --name-to-sid $DOMAIN/$USERNAME2 || failed=$(expr $failed + 1)
+testit_expect_failure "name_to_sid.upn.$UPN_NAME2" $wbinfo_tool --name-to-sid $UPN2 || failed=$(expr $failed + 1)
+
+testit "user_info.domain.$USERNAME2" test_user_info $DOMAIN $USERNAME2 || failed=$(expr $failed + 1)
+testit_expect_failure "user_info.upn.$UPN_NAME2" test_user_info $DOMAIN $USERNAME2 $UPN2 || failed=$(expr $failed + 1)
+
+exit $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 5522f4b35d1..b836a9a1a95 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -215,6 +215,10 @@ plantestsuite("samba3.wbinfo_name_lookup", env,
               [ os.path.join(srcdir(),
                             "nsswitch/tests/test_wbinfo_name_lookup.sh"),
                 '$DOMAIN', '$REALM', '$DC_USERNAME' ])
+plantestsuite("samba3.wbinfo_user_info", env,
+              [ os.path.join(srcdir(),
+                            "nsswitch/tests/test_wbinfo_user_info.sh"),
+                '$DOMAIN', '$REALM', 'alice', 'alice', 'jane', 'jane.doe' ])
 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"])
-- 
2.16.3


>From 1a544bcec42ad0bebdc3ade403e110d8c8c60acb Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 20 Apr 2018 10:46:14 +0200
Subject: [PATCH 4/5] winbind: Fix looking up the user via the UPN

This fixes the lookup if the userPrincipalName is different from the
samAccountName: jane at REALM vs jane.doe at REALM

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13369

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 nsswitch/tests/test_wbinfo_user_info.sh | 4 ++--
 source3/winbindd/winbindd_lookupname.c  | 4 +---
 source3/winbindd/winbindd_util.c        | 1 -
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/nsswitch/tests/test_wbinfo_user_info.sh b/nsswitch/tests/test_wbinfo_user_info.sh
index 8d49b2a4f22..d9c90153631 100755
--- a/nsswitch/tests/test_wbinfo_user_info.sh
+++ b/nsswitch/tests/test_wbinfo_user_info.sh
@@ -69,9 +69,9 @@ testit "user_info.domain.$USERNAME1" test_user_info $DOMAIN $USERNAME1 || failed
 testit "user_info.upn.$UPN_NAME1" test_user_info $DOMAIN $USERNAME1 $UPN1 || failed=$(expr $failed + 1)
 
 testit "name_to_sid.domain.$USERNAME2" $wbinfo_tool --name-to-sid $DOMAIN/$USERNAME2 || failed=$(expr $failed + 1)
-testit_expect_failure "name_to_sid.upn.$UPN_NAME2" $wbinfo_tool --name-to-sid $UPN2 || failed=$(expr $failed + 1)
+testit "name_to_sid.upn.$UPN_NAME2" $wbinfo_tool --name-to-sid $UPN2 || failed=$(expr $failed + 1)
 
 testit "user_info.domain.$USERNAME2" test_user_info $DOMAIN $USERNAME2 || failed=$(expr $failed + 1)
-testit_expect_failure "user_info.upn.$UPN_NAME2" test_user_info $DOMAIN $USERNAME2 $UPN2 || failed=$(expr $failed + 1)
+testit "user_info.upn.$UPN_NAME2" test_user_info $DOMAIN $USERNAME2 $UPN2 || failed=$(expr $failed + 1)
 
 exit $failed
diff --git a/source3/winbindd/winbindd_lookupname.c b/source3/winbindd/winbindd_lookupname.c
index b02269155f1..79d49e33cef 100644
--- a/source3/winbindd/winbindd_lookupname.c
+++ b/source3/winbindd/winbindd_lookupname.c
@@ -62,12 +62,10 @@ struct tevent_req *winbindd_lookupname_send(TALLOC_CTX *mem_ctx,
 			if (p != NULL) {
 				/* upn */
 				domname = p + 1;
-				*p = '\0';
-				name = request->data.name.name;
 			} else {
 				domname = "";
-				name = request->data.name.name;
 			}
+			name = request->data.name.name;
 		}
 	} else {
 		domname = request->data.name.dom_name;
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 9973c78d00f..afd1df05224 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -1589,7 +1589,6 @@ bool parse_domain_user(const char *domuser, fstring domain, fstring user)
 			fstrcpy(domain, lp_workgroup());
 		} else if (p != NULL) {
 			fstrcpy(domain, p + 1);
-			user[PTR_DIFF(p, domuser)] = 0;
 		} else {
 			return False;
 		}
-- 
2.16.3


>From d251d34d42812e9ef5a9b1be37135d277a016015 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 19 Apr 2018 15:05:27 +0200
Subject: [PATCH 5/5] winbind: Ensure we store correct domain name in the cache

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13369

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/winbindd/winbindd_cache.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 9f9e8781c21..5ee59c36ccc 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1838,23 +1838,45 @@ NTSTATUS wb_cache_name_to_sid(struct winbindd_domain *domain,
 	if (domain->online &&
 	    (NT_STATUS_IS_OK(status) || NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))) {
 		enum lsa_SidType save_type = *type;
+		char *real_domain_name = NULL;
+		char *real_name = NULL;
 
 		if (NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED)) {
 			save_type = SID_NAME_UNKNOWN;
 		}
 
-		wcache_save_name_to_sid(domain, status, domain_name, name, sid,
+		status = domain->backend->sid_to_name(domain,
+						      mem_ctx,
+						      sid,
+						      &real_domain_name,
+						      &real_name,
+						      type);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+
+		wcache_save_name_to_sid(domain,
+					status,
+					real_domain_name,
+					real_name,
+					sid,
 					save_type);
 
 		/* Only save the reverse mapping if this was not a UPN */
-		if (!strchr(name, '@')) {
-			if (!strupper_m(discard_const_p(char, domain_name))) {
+		if (name[0] != '\0' && !strchr(name, '@')) {
+			if (!strupper_m(real_domain_name)) {
 				return NT_STATUS_INVALID_PARAMETER;
 			}
-			(void)strlower_m(discard_const_p(char, name));
-			wcache_save_sid_to_name(domain, status, sid,
-						domain_name, name, save_type);
+			(void)strlower_m(real_name);
+			wcache_save_sid_to_name(domain,
+						status,
+						sid,
+						real_domain_name,
+						real_name,
+						save_type);
 		}
+		TALLOC_FREE(real_domain_name);
+		TALLOC_FREE(real_name);
 	}
 
 	return status;
-- 
2.16.3



More information about the samba-technical mailing list