[PATCHES] fix posixacl test
Michael Adam
obnox at samba.org
Thu Mar 12 03:00:12 MDT 2015
Hi,
FYI:
since there has been no reaction since a week, and after
briefly having checked with Metze on IRC that he does not hold
up a NACK, I will push the patches (which do have sufficient
signoff/review). It is definitely better than leaving the
state as it is now and forgetting about it. And it is is the
previously discussed compromise of rooting the current state
into tests that are expected to pass.
Michael
On 2015-03-10 at 21:47 +0100, Michael Adam wrote:
> 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/20150312/e3c48427/attachment.pgp>
More information about the samba-technical
mailing list