[PATCH] idmap_ad: Retry query_user exactly once if we get TLDAP_SERVER_DOWN
Ralph Böhme
slow at samba.org
Mon Jul 10 14:28:19 UTC 2017
On Mon, Jul 10, 2017 at 11:07:16PM +1200, Andrew Bartlett wrote:
> On Mon, 2017-07-10 at 13:02 +0200, Ralph Böhme via samba-technical
> wrote:
> > On Fri, Jun 30, 2017 at 04:10:01PM -0700, Dustin L. Howett via samba-technical wrote:
> > > All other ldap-querying methods in idmap_ad make a single retry attempt if they get
> > > TLDAP_SERVER_DOWN. This patch brings idmap_ad_query_user in line with that design.
> > >
> > > This fixes the symptom described in 12720 at the cost of an additional reconnect per
> > > failed lookup.
> >
> > lgtm. Can I get a second reviewer?
>
> Can we get a selftest for idmap_ad, like but not re-using the totally
> different idmap_rfc2307 tests, perhaps as simple as running
> nsswitch/tests/test_rfc2307_mapping.sh against an appropriate member
> (rather than DC) environment?
like this?
Cheerio!
-slow
-------------- next part --------------
From 9d43cfc63701d4856bb68fd6e3bea1e83b5d52dd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 10 Jul 2017 16:19:18 +0200
Subject: [PATCH 1/3] selftest: add ad_member_idmap_ad server
Add a member server that uses idmap_ad. Gets used in the next commit.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/target/Samba.pm | 1 +
selftest/target/Samba3.pm | 88 +++++++++++++++++++++++++++++++++++++++++++++++
selftest/target/Samba4.pm | 6 ++++
3 files changed, 95 insertions(+)
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 1600ed8..5968772 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -346,6 +346,7 @@ sub get_interface($)
# 11-16 used by selftest.pl for client interfaces
+ $interfaces{"idmapadmember"} = 19;
$interfaces{"idmapridmember"} = 20;
$interfaces{"localdc"} = 21;
$interfaces{"localvampiredc"} = 22;
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 79b1a53..b81277a 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -689,6 +689,94 @@ sub setup_ad_member_idmap_rid($$$$)
return $ret;
}
+sub setup_ad_member_idmap_ad($$$$)
+{
+ my ($self, $prefix, $dcvars) = @_;
+
+ # If we didn't build with ADS, pretend this env was never available
+ if (not $self->have_ads()) {
+ return "UNKNOWN";
+ }
+
+ print "PROVISIONING S3 AD MEMBER WITH idmap_ad config...";
+
+ my $member_options = "
+ security = ads
+ workgroup = $dcvars->{DOMAIN}
+ realm = $dcvars->{REALM}
+ idmap config * : backend = tdb
+ idmap config * : range = 1000000-1999999
+ idmap config $dcvars->{DOMAIN} : backend = ad
+ idmap config $dcvars->{DOMAIN} : range = 2000000-2999999
+";
+
+ my $ret = $self->provision($prefix, $dcvars->{DOMAIN},
+ "IDMAPADMEMBER",
+ "loCalMemberPass",
+ $member_options,
+ $dcvars->{SERVER_IP},
+ $dcvars->{SERVER_IPV6});
+
+ $ret or return undef;
+
+ close(USERMAP);
+ $ret->{DOMAIN} = $dcvars->{DOMAIN};
+ $ret->{REALM} = $dcvars->{REALM};
+
+ my $ctx;
+ my $prefix_abs = abs_path($prefix);
+ $ctx = {};
+ $ctx->{krb5_conf} = "$prefix_abs/lib/krb5.conf";
+ $ctx->{domain} = $dcvars->{DOMAIN};
+ $ctx->{realm} = $dcvars->{REALM};
+ $ctx->{dnsname} = lc($dcvars->{REALM});
+ $ctx->{kdc_ipv4} = $dcvars->{SERVER_IP};
+ $ctx->{kdc_ipv6} = $dcvars->{SERVER_IPV6};
+ $ctx->{krb5_ccname} = "$prefix_abs/krb5cc_%{uid}";
+ Samba::mk_krb5_conf($ctx, "");
+
+ $ret->{KRB5_CONFIG} = $ctx->{krb5_conf};
+
+ my $net = Samba::bindir_path($self, "net");
+ my $cmd = "";
+ $cmd .= "SOCKET_WRAPPER_DEFAULT_IFACE=\"$ret->{SOCKET_WRAPPER_DEFAULT_IFACE}\" ";
+ if (defined($ret->{RESOLV_WRAPPER_CONF})) {
+ $cmd .= "RESOLV_WRAPPER_CONF=\"$ret->{RESOLV_WRAPPER_CONF}\" ";
+ } else {
+ $cmd .= "RESOLV_WRAPPER_HOSTS=\"$ret->{RESOLV_WRAPPER_HOSTS}\" ";
+ }
+ $cmd .= "KRB5_CONFIG=\"$ret->{KRB5_CONFIG}\" ";
+ $cmd .= "SELFTEST_WINBINDD_SOCKET_DIR=\"$ret->{SELFTEST_WINBINDD_SOCKET_DIR}\" ";
+ $cmd .= "$net join $ret->{CONFIGURATION}";
+ $cmd .= " -U$dcvars->{USERNAME}\%$dcvars->{PASSWORD}";
+
+ if (system($cmd) != 0) {
+ warn("Join failed\n$cmd");
+ return undef;
+ }
+
+ # We need world access to this share, as otherwise the domain
+ # administrator from the AD domain provided by Samba4 can't
+ # access the share for tests.
+ chmod 0777, "$prefix/share";
+
+ if (not $self->check_or_start($ret, "yes", "yes", "yes")) {
+ return undef;
+ }
+
+ $ret->{DC_SERVER} = $dcvars->{SERVER};
+ $ret->{DC_SERVER_IP} = $dcvars->{SERVER_IP};
+ $ret->{DC_SERVER_IPV6} = $dcvars->{SERVER_IPV6};
+ $ret->{DC_NETBIOSNAME} = $dcvars->{NETBIOSNAME};
+ $ret->{DC_USERNAME} = $dcvars->{USERNAME};
+ $ret->{DC_PASSWORD} = $dcvars->{PASSWORD};
+
+ # Special case, this is called from Samba4.pm but needs to use the Samba3 check_env and get_log_env
+ $ret->{target} = $self;
+
+ return $ret;
+}
+
sub setup_simpleserver($$)
{
my ($self, $path) = @_;
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 772f982..205e281 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2130,6 +2130,12 @@ sub setup_env($$$)
}
return $target3->setup_ad_member_idmap_rid("$path/ad_member_idmap_rid",
$self->{vars}->{ad_dc});
+ } elsif ($envname eq "ad_member_idmap_ad") {
+ if (not defined($self->{vars}->{ad_dc})) {
+ $self->setup_ad_dc("$path/ad_dc");
+ }
+ return $target3->setup_ad_member_idmap_ad("$path/ad_member_idmap_ad",
+ $self->{vars}->{ad_dc});
} elsif ($envname eq "none") {
return $self->setup_none("$path/none");
} else {
--
2.9.4
From 32df448dc521069c08a833d994528022d0a66d15 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 10 Jul 2017 16:20:23 +0200
Subject: [PATCH 2/3] selftest: add some basic tests for idmap_ad
Signed-off-by: Ralph Boehme <slow at samba.org>
---
nsswitch/tests/test_idmap_ad.sh | 99 +++++++++++++++++++++++++++++++++++++++++
source3/selftest/tests.py | 4 +-
2 files changed, 102 insertions(+), 1 deletion(-)
create mode 100755 nsswitch/tests/test_idmap_ad.sh
diff --git a/nsswitch/tests/test_idmap_ad.sh b/nsswitch/tests/test_idmap_ad.sh
new file mode 100755
index 0000000..2f4ee32
--- /dev/null
+++ b/nsswitch/tests/test_idmap_ad.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+#
+# Basic testing of id mapping with idmap_ad
+#
+
+if [ $# -ne 3 ]; then
+ echo Usage: $0 DOMAIN DC_SERVER DC_PASSWORD
+ exit 1
+fi
+
+DOMAIN="$1"
+DC_SERVER="$2"
+DC_PASSWORD="$3"
+
+wbinfo="$VALGRIND $BINDIR/wbinfo"
+ldbmodify="$VALGRIND $BINDIR/ldbmodify"
+ldbsearch="$VALGRIND $BINDIR/ldbsearch"
+
+failed=0
+
+. `dirname $0`/../../testprogs/blackbox/subunit.sh
+
+DOMAIN_SID=$($wbinfo -n "@$DOMAIN" | cut -f 1 -d " ")
+if [ $? -ne 0 ] ; then
+ echo "Could not find domain SID" | subunit_fail_test "test_idmap_ad"
+ exit 1
+fi
+
+BASE_DN=$($ldbsearch -H ldap://$DC_SERVER -b "" -s base defaultNamingContext | awk '/^defaultNamingContext/ {print $2}')
+if [ $? -ne 0 ] ; then
+ echo "Could not find base DB" | subunit_fail_test "test_idmap_ad"
+ exit 1
+fi
+
+#
+# Add POSIX ids to AD
+#
+cat <<EOF | $ldbmodify -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+dn: CN=Administrator,CN=Users,$BASE_DN
+changetype: modify
+add: uidNumber
+uidNumber: 2000000
+EOF
+
+cat <<EOF | $ldbmodify -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+dn: CN=Domain Users,CN=Users,$BASE_DN
+changetype: modify
+add: gidNumber
+gidNumber: 2000001
+EOF
+
+#
+# Test 1: Test uid of Administrator, should be 2000000
+#
+
+out="$($wbinfo -S $DOMAIN_SID-500)"
+echo "wbinfo returned: \"$out\", expecting \"2000000\""
+test "$out" = "2000000"
+ret=$?
+testit "Test uid of Administrator is 2000000" test $ret -eq 0 || failed=$(expr $failed + 1)
+
+#
+# Test 2: Test gid of Domain Users, should be 2000001
+#
+
+out="$($wbinfo -Y $DOMAIN_SID-513)"
+echo "wbinfo returned: \"$out\", expecting \"2000001\""
+test "$out" = "2000001"
+ret=$?
+testit "Test uid of Domain Users is 2000001" test $ret -eq 0 || failed=$(expr $failed + 1)
+
+#
+# Test 3: Test get userinfo for Administrator works
+#
+
+out="$($wbinfo -i $DOMAIN/Administrator)"
+echo "wbinfo returned: \"$out\", expecting \"$DOMAIN/administrator:*:2000000:2000001::/home/$DOMAIN/administrator:/bin/false\""
+test "$out" = "$DOMAIN/administrator:*:2000000:2000001::/home/$DOMAIN/administrator:/bin/false"
+ret=$?
+testit "Test get userinfo for Administrator works" test $ret -eq 0 || failed=$(expr $failed + 1)
+
+#
+# Remove POSIX ids from AD
+#
+cat <<EOF | $ldbmodify -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+dn: CN=Administrator,CN=Users,$BASE_DN
+changetype: modify
+delete: uidNumber
+uidNumber: 2000000
+EOF
+
+cat <<EOF | $ldbmodify -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+dn: CN=Domain Users,CN=Users,$BASE_DN
+changetype: modify
+delete: gidNumber
+gidNumber: 2000001
+EOF
+
+exit $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d459ede..d352c14 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -367,7 +367,7 @@ rpc = ["rpc.authcontext", "rpc.samba3.bind", "rpc.samba3.srvsvc", "rpc.samba3.sh
local = ["local.nss"]
-idmap = ["idmap.rfc2307", "idmap.alloc", "idmap.rid"]
+idmap = ["idmap.rfc2307", "idmap.alloc", "idmap.rid", "idmap.ad"]
rap = ["rap.basic", "rap.rpc", "rap.printing", "rap.sam"]
@@ -449,6 +449,8 @@ for t in tests:
plantestsuite(t, "ad_member_rfc2307", [os.path.join(samba3srcdir, "../nsswitch/tests/test_idmap_nss.sh"), '$DOMAIN'])
elif t == "idmap.rid":
plantestsuite(t, "ad_member_idmap_rid", [os.path.join(samba3srcdir, "../nsswitch/tests/test_idmap_rid.sh"), '$DOMAIN', '2000000'])
+ elif t == "idmap.ad":
+ plantestsuite(t, "ad_member_idmap_ad", [os.path.join(samba3srcdir, "../nsswitch/tests/test_idmap_ad.sh"), '$DOMAIN', '$DC_SERVER', '$DC_PASSWORD'])
elif t == "raw.acls":
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/nfs4acl_simple -U$USERNAME%$PASSWORD', description='nfs4acl_xattr-simple')
--
2.9.4
From 8d9266505fa425d5bedd72de7764618d934aa035 Mon Sep 17 00:00:00 2001
From: "Dustin L. Howett via samba-technical" <samba-technical at lists.samba.org>
Date: Fri, 30 Jun 2017 16:10:01 -0700
Subject: [PATCH 3/3] idmap_ad: Retry query_user exactly once if we get
TLDAP_SERVER_DOWN
All other ldap-querying methods in idmap_ad make a single retry attempt if they get
TLDAP_SERVER_DOWN. This patch brings idmap_ad_query_user in line with that design.
This fixes the symptom described in 12720 at the cost of an additional reconnect per
failed lookup.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12720
Signed-off-by: Dustin L. Howett <dustin at howett.net>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/idmap_ad.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_ad.c b/source3/winbindd/idmap_ad.c
index 8c9e97b..315a944 100644
--- a/source3/winbindd/idmap_ad.c
+++ b/source3/winbindd/idmap_ad.c
@@ -502,9 +502,26 @@ static NTSTATUS idmap_ad_query_user(struct idmap_domain *domain,
return NT_STATUS_OK;
}
+static NTSTATUS idmap_ad_query_user_retry(struct idmap_domain *domain,
+ struct wbint_userinfo *info)
+{
+ const NTSTATUS status_server_down =
+ NT_STATUS_LDAP(TLDAP_RC_V(TLDAP_SERVER_DOWN));
+ NTSTATUS status;
+
+ status = idmap_ad_query_user(domain, info);
+
+ if (NT_STATUS_EQUAL(status, status_server_down)) {
+ TALLOC_FREE(domain->private_data);
+ status = idmap_ad_query_user(domain, info);
+ }
+
+ return status;
+}
+
static NTSTATUS idmap_ad_initialize(struct idmap_domain *dom)
{
- dom->query_user = idmap_ad_query_user;
+ dom->query_user = idmap_ad_query_user_retry;
dom->private_data = NULL;
return NT_STATUS_OK;
}
--
2.9.4
More information about the samba-technical
mailing list