selftest: re-enable nss_winbind via nss_wrapper in the test-envs.

Michael Adam obnox at samba.org
Tue Feb 17 09:57:47 MST 2015


Hi Andrew,

we debugged this a little more, and
indeed adding explicit ACEs for LA (the domain admin)
to the NT acls in the test and adapting the posix
checks to the correponding permission bits fixes
this in a slightly more satisfactory way.

I think this (attached) might be the better fix.

What do you think?

Michael


On 2015-02-17 at 16:35 +0100, Michael Adam wrote:
> Hi Andrew,
> 
> On 2015-02-17 at 15:25 +1300, Andrew Bartlett wrote:
> > On Fri, 2015-02-13 at 20:58 +0100, Günther Deschner wrote:
> > > +#
> > > +# 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\)$
> > 
> > G'Day Günther,
> > 
> > Could you please fix these tests rather than just disabling them?  They
> > don't run in any other environments, and so they are know just removed
> > from our selftest. 
> > 
> > The purpose of this test is to ensure we set the correct posix ACL,
> > which otherwise we would not notice in selftest as we don't have the
> > kernel doing any access control.
> 
> Right, but the question is whether the test is wrong or the
> setup. The observation is that the test fails once the
> environment is fixed to include libnss_winbind just like a
> real environment does. So the test was always running against
> a wrong setup in the past...
> 
> Btw, the test is still run, but failure considered success,
> but of course you know that... :-)
> 
> We did in fact look into the failing subtests to some extent
> when we observed them, but could not make a lot of sense. So
> pushing the patches with this comment in knownfail served the
> purpose we had in mind, namely to start the discussion and to
> have someone alerted who knows more about the background of
> this test! :-)
> 
> Maybe you can shed some light:
> 
> What happens is this:
> 
> - An acl is put onto a file from an sddl acl via smbd.set_nt_acl().
> - Then the acl is read as posix acl via smbd.get_sys_acl().
> - The result is compared ACE by ACE against hard coded values.
> 
> The problem seems to be that the acl is not completely fixed:
> Here is the ACL (lines wrapped):
> 
> SYSVOL_ACL = "
> O:LA
> G:BA
> D:P
>  (A;OICI;0x001f01ff;;;BA)
>  (A;OICI;0x001200a9;;;SO)
>  (A;OICI;0x001f01ff;;;SY)
>  (A;OICI;0x001200a9;;;AU)"
> 
> So there is no explicit acl for LA (the domain admin)
> and no acl for creator owner, either.
> 
> The test checks for this:
> 
>   self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_USER)
>   self.assertEquals(posix_acl.acl[1].a_perm, 6)
>   self.assertEquals(posix_acl.acl[1].info.uid, LA_uid)
> 
>   ...
> 
>   self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER)
>   self.assertEquals(posix_acl.acl[4].a_perm, 6)
>   self.assertEquals(posix_acl.acl[4].info.uid, BA_gid)
> 
> These things seem to be automatically incurred by the
> posix acl mapping code. This seems so random.
> Now with libnss_winbind enabled, we get 7 instead of 6,
> i.e. additional write permissions.
> That is all that changes. All the ACEs that have corresponding
> ACEs in the sddl still work correctly.
> The attached patch fixes the test case, but I doubt that it
> is the correct fix.
> 
> So why does this happen once the domain admin shows up
> in nss with the same UID as the calling user.
> 
> After all, isn't it a broken concept that the domain
> admin gets the same uid as root (or whatever user
> passed in as --root to provision)?
> 
> Do we need to fix the ACL for the test? It seems incomplete.
> 
> Any hints appreciated!...
> 
> Michael

> From 6af22cf5b23fbd5039d870e9c2076ad618be95f4 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] TODO: fix the posixacl test for the fixed environment.
> 
> ---
>  python/samba/tests/posixacl.py | 8 ++++----
>  selftest/knownfail             | 8 --------
>  2 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
> index a6b5118..0ec3a38 100644
> --- a/python/samba/tests/posixacl.py
> +++ b/python/samba/tests/posixacl.py
> @@ -345,14 +345,14 @@ 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)
> +        self.assertEquals(posix_acl.acl[1].a_perm, 7)
>          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)
> +        self.assertEquals(posix_acl.acl[3].a_perm, 7)
>  
>          self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER)
>          self.assertEquals(posix_acl.acl[4].a_perm, 7)
> @@ -682,14 +682,14 @@ 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)
> +        self.assertEquals(posix_acl.acl[1].a_perm, 7)
>          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)
> +        self.assertEquals(posix_acl.acl[3].a_perm, 7)
>  
>          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
> 



-------------- next part --------------
From c3e7cc43a2c74c6cf366c5fb5bc59253db938be1 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] selftest: fix the posixacl test for the fixed plugin_s4_dc
 env

This is done by explicitly setting ACES for the domain admin
in the NT ACLs and adapting the POSIX checks to the corresponding
permissions.

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/provision/__init__.py | 4 ++--
 python/samba/tests/posixacl.py     | 8 ++++----
 selftest/knownfail                 | 8 --------
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 1603321..3e2397d 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1476,8 +1476,8 @@ def fill_samdb(samdb, lp, names, logger, policyguid,
         return samdb
 
 
-SYSVOL_ACL = "O:LAG:BAD:P(A;OICI;0x001f01ff;;;BA)(A;OICI;0x001200a9;;;SO)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)"
-POLICIES_ACL = "O:LAG:BAD:P(A;OICI;0x001f01ff;;;BA)(A;OICI;0x001200a9;;;SO)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)(A;OICI;0x001301bf;;;PA)"
+SYSVOL_ACL = "O:LAG:BAD:P(A;OICI;0x001200a9;;;LA)(A;OICI;0x001f01ff;;;BA)(A;OICI;0x001200a9;;;SO)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)"
+POLICIES_ACL = "O:LAG:BAD:P(A;OICI;0x001200a9;;;LA)(A;OICI;0x001f01ff;;;BA)(A;OICI;0x001200a9;;;SO)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)(A;OICI;0x001301bf;;;PA)"
 SYSVOL_SERVICE="sysvol"
 
 def set_dir_acl(path, acl, lp, domsid, use_ntvfs, passdb, service=SYSVOL_SERVICE):
diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index a6b5118..280e3d3 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -345,14 +345,14 @@ 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)
+        self.assertEquals(posix_acl.acl[1].a_perm, 5)
         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)
+        self.assertEquals(posix_acl.acl[3].a_perm, 5)
 
         self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER)
         self.assertEquals(posix_acl.acl[4].a_perm, 7)
@@ -682,14 +682,14 @@ 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)
+        self.assertEquals(posix_acl.acl[1].a_perm, 5)
         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)
+        self.assertEquals(posix_acl.acl[3].a_perm, 5)
 
         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

-------------- 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/20150217/c8f4edf1/attachment.pgp>


More information about the samba-technical mailing list