[PATCHES BUG 13312] winbindd: Do not ignore domain in LOOKUPNAME request
Christof Schmitt
cs at samba.org
Sat Mar 31 04:43:31 UTC 2018
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
-------------- next part --------------
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