[PATCH] Fix wbinfo --sids-to-unix_ids with some unmapped sids
Stefan Metzmacher
metze at samba.org
Mon Jan 7 11:36:44 UTC 2019
Hi Volker,
>>> We have to reply all we have if some SIDs fail to map.
>>>
>>> Review appreciated!
>>
>> I'd like to have this, but somethings are not clear to me.
>>
>> Is this an wbinfo only thing?
>
> That is not wbinfo only.
>
>> Or is smbd also affected?
>
> smbd is also affected.
Ok, thanks!
>> Do we need backports?
>
> This only popped up at a customer site. Nobody public has complained
> yet. So it depends if you think that many people are affected.
Ok, we may not need to backport this.
>> Please don't use S-1-5-1000 for the test, it's really
>> a bug (a different one) that lookup_wellknown_sid() doesn't handle
>> it. I'll see if I can come up with a fix for that... (we already handle
>> it in dom_sid_lookup_predefined_name() and we need to unify the code)
>
> Which one do you know for 100% sure will never be mapped? At my
> customer site this is the one that caused trouble. I thought that in
> principle idmap tdb maps everything and I was happy that I found one
> that is not mappable.
I updated the test a bit to use S-1-5-123456789 and made it a bit more
strict, so that it demonstrates the bug with a knownfail entry, which
gets revert together with the fix.
I'll try to fix the S-1-5-1000 case by fixing lookup_wellknown_sid()
later. For admins using selective authentication it might a very
critical to set restrictions for S-1-5-1000 in ACLs.
What about the attached patchset?
metze
-------------- next part --------------
From 6e00b6b10c6da9669fd5f6844aab15de7f406255 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 21 Dec 2018 11:49:33 +0100
Subject: [PATCH 1/3] selftest: Test sids-to-xids with one failing sid
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Volker Lendecke <vl at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
nsswitch/tests/test_wbinfo_sids_to_xids.sh | 32 +++++++++++++++++++
.../knownfail.d/samba3.wbinfo_sids_to_xids | 1 +
source3/selftest/tests.py | 5 +++
3 files changed, 38 insertions(+)
create mode 100755 nsswitch/tests/test_wbinfo_sids_to_xids.sh
create mode 100644 selftest/knownfail.d/samba3.wbinfo_sids_to_xids
diff --git a/nsswitch/tests/test_wbinfo_sids_to_xids.sh b/nsswitch/tests/test_wbinfo_sids_to_xids.sh
new file mode 100755
index 000000000000..a3ab404f6446
--- /dev/null
+++ b/nsswitch/tests/test_wbinfo_sids_to_xids.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+incdir=`dirname $0`/../../testprogs/blackbox
+. $incdir/subunit.sh
+
+#
+# S-1-5-123456789 fails, but S-1-5-11 succeeds. Check that S-1-5-11 is
+# mapped successfully with a GID in the 1000x range
+#
+wbinfo_some_mapped()
+{
+ output=`$VALGRIND $BINDIR/wbinfo --sids-to-unix-ids=S-1-5-123456789,S-1-5-11`
+ test x"$?" = x"0" || {
+ return 1
+ }
+
+ printf '%s' "$output" | grep -q 'S-1-5-123456789 -> unmapped' || {
+ printf '%s' "$output"
+ return 1
+ }
+
+ printf '%s' "$output" | grep -q 'S-1-5-11 -> gid 10000' || {
+ printf '%s' "$output"
+ return 1
+ }
+
+ return 0
+}
+
+testit "wbinfo some mapped" wbinfo_some_mapped || failed=`expr $failed + 1`
+
+testok $0 $failed
diff --git a/selftest/knownfail.d/samba3.wbinfo_sids_to_xids b/selftest/knownfail.d/samba3.wbinfo_sids_to_xids
new file mode 100644
index 000000000000..26c61e361a71
--- /dev/null
+++ b/selftest/knownfail.d/samba3.wbinfo_sids_to_xids
@@ -0,0 +1 @@
+^samba3.wbinfo_sids_to_xids.wbinfo.some.mapped
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index a3bb1c4feac8..c0fde3ad3f0d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -262,6 +262,11 @@ plantestsuite("samba3.wbinfo_user_info", env,
"nsswitch/tests/test_wbinfo_user_info.sh"),
'$TRUST_DOMAIN', '$TRUST_REALM', '$DOMAIN', 'alice', 'alice', 'jane', 'jane.doe', env])
+env = "nt4_member:local"
+plantestsuite("samba3.wbinfo_sids_to_xids", env,
+ [os.path.join(srcdir(),
+ "nsswitch/tests/test_wbinfo_sids_to_xids.sh")])
+
env = "ad_member"
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, ""])
--
2.17.1
From f64d46b23657df14917e1dd1ad42b4738ae20860 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Dec 2018 15:53:21 +0100
Subject: [PATCH 2/3] idmap_tdb: If one SID fails to map, try the rest
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/winbindd/idmap_tdb_common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_tdb_common.c b/source3/winbindd/idmap_tdb_common.c
index e130be08245c..34269e3fe56e 100644
--- a/source3/winbindd/idmap_tdb_common.c
+++ b/source3/winbindd/idmap_tdb_common.c
@@ -577,8 +577,11 @@ static NTSTATUS idmap_tdb_common_sids_to_unixids_action(struct db_context *db,
ret =
idmap_tdb_common_new_mapping(state->dom,
state->ids[i]);
+ DBG_DEBUG("idmap_tdb_common_new_mapping returned %s\n",
+ nt_errstr(ret));
if (!NT_STATUS_IS_OK(ret)) {
- goto done;
+ ret = STATUS_SOME_UNMAPPED;
+ continue;
}
num_mapped += 1;
}
--
2.17.1
From 79c8e22bf0d1e992d465508dd4d417f25146254a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Dec 2018 15:53:48 +0100
Subject: [PATCH 3/3] idmap: In _wbint_Sids2UnixIDs, pass back what we have
SOME_UNMAPPED does not mean that nothing worthwhile is in here. We
need to pass what we have.
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
selftest/knownfail.d/samba3.wbinfo_sids_to_xids | 1 -
source3/winbindd/winbindd_dual_srv.c | 9 +++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
delete mode 100644 selftest/knownfail.d/samba3.wbinfo_sids_to_xids
diff --git a/selftest/knownfail.d/samba3.wbinfo_sids_to_xids b/selftest/knownfail.d/samba3.wbinfo_sids_to_xids
deleted file mode 100644
index 26c61e361a71..000000000000
--- a/selftest/knownfail.d/samba3.wbinfo_sids_to_xids
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.wbinfo_sids_to_xids.wbinfo.some.mapped
diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
index 62224bf313ee..ab14f5d51a04 100644
--- a/source3/winbindd/winbindd_dual_srv.c
+++ b/source3/winbindd/winbindd_dual_srv.c
@@ -204,6 +204,15 @@ NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p,
status = dom->methods->sids_to_unixids(dom, id_map_ptrs);
+ if (NT_STATUS_EQUAL(status, STATUS_SOME_UNMAPPED)) {
+ /*
+ * This is okay. We need to transfer the mapped ones
+ * up to our caller. The individual mappings carry the
+ * information whether they are mapped or not.
+ */
+ status = NT_STATUS_OK;
+ }
+
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("sids_to_unixids returned %s\n",
nt_errstr(status)));
--
2.17.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190107/c6f9b7ef/signature.sig>
More information about the samba-technical
mailing list