[PATCHES BUG 13312] winbindd: Do not ignore domain in LOOKUPNAME request
Christof Schmitt
cs at samba.org
Wed Mar 7 22:19:46 UTC 2018
Is anybody interested in reviewing this change?
On Wed, Feb 28, 2018 at 02:29:02PM -0700, Christof Schmitt via samba-technical wrote:
> 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
> 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