[PATCHES] fix posixacl test
Michael Adam
obnox at samba.org
Tue Mar 10 14:47:39 MDT 2015
Hi,
can we push the proposed patches for the time being?
Metze, was your comment a real NACK?
Andrew, what do you think?
Doesn't the patch at least address the issues
we discussed in that it re-establishes the
orignal test behaviour and adds a test for
the yet not fully understood changed behaviour
with nss_winbindd active?
Thanks - Michael
On 2015-03-06 at 17:24 +0100, Michael Adam wrote:
> 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
>
>
> 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/20150310/2e804033/attachment.pgp>
More information about the samba-technical
mailing list