wbinfo -i output domain realm vs. ntdomain before login
Jeremy Allison
jra at samba.org
Wed Apr 25 22:45:05 UTC 2018
On Mon, Apr 23, 2018 at 04:44:37PM +0200, Andreas Schneider via samba-technical wrote:
> 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.
OK Andreas, I'm reviewing this and I'd like some clarification
on the changes in:
[PATCH 4/5] winbind: Fix looking up the user via the UPN
source3/winbindd/winbindd_lookupname.c
source3/winbindd/winbindd_util.c
In my understanding both of these fixes are ensuring that
a upn name passed in as:
user at realm
is not being split into domname=realm, name=user
components, but instead passed to winbindd as:
domname=realm, name=user at realm
Yes ? Can you add a comment explaining what
is being passed to winbindd and why that change
is needed, as well as a comment for the change
in parse_domain_user() that explains why returning
the upn user name is correct.
I believe you're right :-), but this stuff is
tricky enough that more comments here might help
in future.
Thanks !
Jeremy.
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> 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