[PATCHES] fix posixacl test
Michael Adam
obnox at samba.org
Thu Mar 12 04:18:36 MDT 2015
pushed to autobuild
On 2015-03-12 at 10:00 +0100, Michael Adam wrote:
> 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/e78a2f5e/attachment.pgp>
More information about the samba-technical
mailing list