[PATCHES BUG 13312] winbindd: Do not ignore domain in LOOKUPNAME request
Christof Schmitt
cs at samba.org
Fri Mar 9 16:39:33 UTC 2018
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
-------------- 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 802f282919a0c805c0b992867002dd401b4da7c7 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 | 33 +++++++++++++++++++++------------
2 files changed, 21 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..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