[PATCHES] fix posixacl test
Michael Adam
obnox at samba.org
Fri Mar 6 09:24:03 MST 2015
Updated patchet (rebased non-trivially) on top of
Jelmer's latest patches.
Any further comments so far?
Michael
On 2015-03-05 at 21:28 +0100, Michael Adam wrote:
> Hi Metze,
>
> On 2015-03-05 at 19:58 +0100, Stefan (metze) Metzmacher wrote:
> >
> > > this is the promised fix for the posixacl test.
> > > We have duplicated the plugin_s4_dc env to
> > > one that does not use nss_winbind, adapted the
> > > test to be able to run against the two variants
> > > and enabled the test for the new environment,
> > > removing the knownfail entries.
> >
> > I think we should not expect different behaviour depending
> > on nss_winbind being used. We need to find the reason
> > for this difference and fix that.
>
> Of course. But I personally currently don't have
> to the time to spend many more days on this.
>
> I happily summarize some of this lengthy mail thread
> as justification:
>
> The test is slightly strange anyways in that the
> nt acl that is put through the translation machinery
> does not contain explicit acls for the owner/domain admin
> and hence the corresponding aces in the resulting posix
> acl are only subject to the translation code and the
> environment. So it is not entirely surprising that the
> result changes in this respect when the environment
> is changed. And the environment changes relevantly because
> with nss_winbind active, the domain admin which is now
> also available via nss and unfortunately carries the same
> UID as the root user (which I consider problematic). The
> original test was created by simply looking at the result
> and hard-coding the result into the test. This patch does
> the same for the changed environment.
>
> This is the compromise we agreed upon.
>
> Of course we should understand the effects better,
> and if anybody can provide insight / fixes now, I am
> more than happy to withdraw these. But Andrew has
> requested that we do somthing about the test that I
> initially marked as knownfail and this is what I can
> offer now. Alternatively, we can leave the test
> unchanged and simply mark the test in the plugin_s4_dc
> env as knownfail and have it succeed against
> plugin_s4_dc_no_nss, but I prefer the present solution.
>
> Cheers - Michael
-------------- next part --------------
From 16e1143ae1056e44d3b5b97c886ddaec3dd3a7f4 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 17 Feb 2015 16:06:49 +0100
Subject: [PATCH 1/4] selftest: modify python.samba.test.posixacl to cope with
nss_winbind active
It was observed that adding libnss_winbind (via nss_wrapper) lets
the posix acl mapping come out slightly differently with respect
to the owner/domain admin who is not explicitly nailed down in
the original NT acl.
This patch extends the test to react to the presence of
nss_winbind in environment and adapts the expected results.
This in particular fixes the run of the test against the
(changed) plugin_s4_dc environment while keeping the possibility
to successfully run it against an env without nss_winbind.
Pair-Programmed-With: Guenther Deschner <gd at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Guenther Deschner <gd at samba.org>
---
python/samba/tests/posixacl.py | 32 ++++++++++++++++++++++++++++----
selftest/knownfail | 8 --------
2 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index a6b5118..d8c0fcd 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -316,6 +316,12 @@ class PosixAclMappingTests(TestCaseInTempDir):
self.assertEquals(facl.as_sddl(domsid),acl)
posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
+ nwrap_module_so_path = os.getenv('NSS_WRAPPER_MODULE_SO_PATH')
+ nwrap_module_fn_prefix = os.getenv('NSS_WRAPPER_MODULE_FN_PREFIX')
+
+ nwrap_winbind_active = (nwrap_module_so_path != "" and
+ nwrap_module_fn_prefix == "winbind")
+
LA_sid = security.dom_sid(str(domsid)+"-"+str(security.DOMAIN_RID_ADMINISTRATOR))
BA_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS)
SO_sid = security.dom_sid(security.SID_BUILTIN_SERVER_OPERATORS)
@@ -345,14 +351,20 @@ class PosixAclMappingTests(TestCaseInTempDir):
self.assertEquals(posix_acl.acl[0].info.gid, BA_gid)
self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_USER)
- self.assertEquals(posix_acl.acl[1].a_perm, 6)
+ if nwrap_winbind_active:
+ self.assertEquals(posix_acl.acl[1].a_perm, 7)
+ else:
+ self.assertEquals(posix_acl.acl[1].a_perm, 6)
self.assertEquals(posix_acl.acl[1].info.uid, LA_uid)
self.assertEquals(posix_acl.acl[2].a_type, smb_acl.SMB_ACL_OTHER)
self.assertEquals(posix_acl.acl[2].a_perm, 0)
self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ)
- self.assertEquals(posix_acl.acl[3].a_perm, 6)
+ if nwrap_winbind_active:
+ self.assertEquals(posix_acl.acl[3].a_perm, 7)
+ else:
+ self.assertEquals(posix_acl.acl[3].a_perm, 6)
self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER)
self.assertEquals(posix_acl.acl[4].a_perm, 7)
@@ -650,6 +662,12 @@ class PosixAclMappingTests(TestCaseInTempDir):
self.assertEquals(facl.as_sddl(domsid),acl)
posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
+ nwrap_module_so_path = os.getenv('NSS_WRAPPER_MODULE_SO_PATH')
+ nwrap_module_fn_prefix = os.getenv('NSS_WRAPPER_MODULE_FN_PREFIX')
+
+ nwrap_winbind_active = (nwrap_module_so_path != "" and
+ nwrap_module_fn_prefix == "winbind")
+
LA_sid = security.dom_sid(str(domsid)+"-"+str(security.DOMAIN_RID_ADMINISTRATOR))
BA_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS)
SO_sid = security.dom_sid(security.SID_BUILTIN_SERVER_OPERATORS)
@@ -682,14 +700,20 @@ class PosixAclMappingTests(TestCaseInTempDir):
self.assertEquals(posix_acl.acl[0].info.gid, BA_gid)
self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_USER)
- self.assertEquals(posix_acl.acl[1].a_perm, 6)
+ if nwrap_winbind_active:
+ self.assertEquals(posix_acl.acl[1].a_perm, 7)
+ else:
+ self.assertEquals(posix_acl.acl[1].a_perm, 6)
self.assertEquals(posix_acl.acl[1].info.uid, LA_uid)
self.assertEquals(posix_acl.acl[2].a_type, smb_acl.SMB_ACL_OTHER)
self.assertEquals(posix_acl.acl[2].a_perm, 0)
self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ)
- self.assertEquals(posix_acl.acl[3].a_perm, 6)
+ if nwrap_winbind_active:
+ self.assertEquals(posix_acl.acl[3].a_perm, 7)
+ else:
+ self.assertEquals(posix_acl.acl[3].a_perm, 6)
self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER)
self.assertEquals(posix_acl.acl[4].a_perm, 7)
diff --git a/selftest/knownfail b/selftest/knownfail
index b3cc2d6..64fc2cd 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -314,11 +314,3 @@
# Differences in our KDC compared to windows
#
^samba4.krb5.kdc .*.as-req-pac-request # We should reply to a request for a PAC over UDP with KRB5KRB_ERR_RESPONSE_TOO_BIG unconditionally
-#
-# Test does not work, apparently because the calling user and
-# the domain admin use the same uid. This was uncovered by
-# enabling libnss_winbindd in the nsswrapper environment.
-# TODO: fix the test.
-#
-^samba.tests.posixacl.samba.tests.posixacl.PosixAclMappingTests.test_setntacl_sysvol_check_getposixacl\(plugin_s4_dc:local\)$
-^samba.tests.posixacl.samba.tests.posixacl.PosixAclMappingTests.test_setntacl_policies_check_getposixacl\(plugin_s4_dc:local\)$
--
2.1.0
From 9c0db305178d2354fbb9f98293a2bcf465e6acd0 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 5 Mar 2015 13:22:07 +0100
Subject: [PATCH 2/4] selftest: extend setup_plugin_s4_dc to allow for not
using nss_winbindd
Pair-Programmed-With: Guenther Deschner <gd at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Guenther Deschner <gd at samba.org>
---
selftest/target/Samba4.pm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index da2009d..2535ab6 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2274,7 +2274,7 @@ sub setup_rodc($$$)
sub setup_plugin_s4_dc($$)
{
- my ($self, $path) = @_;
+ my ($self, $path, $no_nss) = @_;
# If we didn't build with ADS, pretend this env was never available
if (not $self->{target3}->have_ads()) {
@@ -2286,6 +2286,11 @@ sub setup_plugin_s4_dc($$)
return undef;
}
+ if (defined($no_nss) and $no_nss) {
+ $env->{NSS_WRAPPER_MODULE_SO_PATH} = undef;
+ $env->{NSS_WRAPPER_MODULE_FN_PREFIX} = undef;
+ }
+
$self->check_or_start($env, "single");
$self->wait_for_start($env);
--
2.1.0
From 681a099ab94f4db1ea29f30472760953ef03e67d Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 5 Mar 2015 13:22:35 +0100
Subject: [PATCH 3/4] selftest: add a new environment plugin_s4_dc_no_nss
Pair-Programmed-With: Guenther Deschner <gd at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Guenther Deschner <gd at samba.org>
---
selftest/target/Samba4.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 2535ab6..b2417b8 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1962,6 +1962,8 @@ sub setup_env($$$)
return $target3->setup_admember("$path/s3member", $self->{vars}->{dc}, 29);
} elsif ($envname eq "plugin_s4_dc") {
return $self->setup_plugin_s4_dc("$path/plugin_s4_dc");
+ } elsif ($envname eq "plugin_s4_dc_no_nss") {
+ return $self->setup_plugin_s4_dc("$path/plugin_s4_dc_no_nss", "no_nss");
} elsif ($envname eq "s3member_rfc2307") {
if (not defined($self->{vars}->{dc})) {
$self->setup_dc("$path/dc");
--
2.1.0
From 19b1f7047ec100cf6e8b9bf8c1956192dcd07936 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 5 Mar 2015 14:43:54 +0100
Subject: [PATCH 4/4] selftest: also test python.samba.tests.posixacl against
plugin_s4_dc_no_nss
Pair-Programmed-With: Guenther Deschner <gd at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Guenther Deschner <gd at samba.org>
---
source4/selftest/tests.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index a104e43..925ad73 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -493,6 +493,7 @@ for env in ["dc", "fl2000dc", "fl2003dc", "fl2008r2dc"]:
planpythontestsuite("dc:local", "samba.tests.upgradeprovisionneeddc")
planpythontestsuite("plugin_s4_dc:local", "samba.tests.posixacl")
+planpythontestsuite("plugin_s4_dc_no_nss:local", "samba.tests.posixacl")
plantestsuite_loadlist("samba4.deletetest.python(dc)", "dc", [python, os.path.join(samba4srcdir, "dsdb/tests/python/deletetest.py"),
'$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite("samba4.blackbox.samba3dump", "none", [os.path.join(samba4srcdir, "selftest/test_samba3dump.sh")])
--
2.1.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150306/30859e15/attachment.pgp>
More information about the samba-technical
mailing list