samba_upgradeprovision: Remove inherited ACEs before comparing the SDs

This avoids changing an SD when it is not really required.

Andrew Bartlett

Reviewed-by: Stefan Metzmacher <metze@samba.org>

python/samba/tests/upgradeprovision.py

@@ -63,18 +63,22 @@ class UpgradeProvisionTestCase(TestCaseInTempDir):
     def test_get_diff_sds(self):
         domsid = security.dom_sid('S-1-5-21')
 
-        sddl = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+        sddl = "O:SAG:DUD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)"
-        sddl1 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+        sddl1 = "O:SAG:DUD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)"
-        sddl2 = "O:BAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+        sddl2 = "O:BAG:DUD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)"
-        sddl3 = "O:SAG:BAD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+        sddl3 = "O:SAG:BAD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)"
-        sddl4 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;BA)\
+        sddl4 = "O:SAG:DUD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;BA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)"
-        sddl5 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+        sddl5 = "O:SAG:DUD:AI(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
-(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)"
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)"
+        sddl6 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
+(A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)\
+(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA)\
+(A;CI;RP LCLORC;;;AU)(A;CI;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CISA;WP;;;WD)(AU;CIIDSA;WP;;;WD)"
 
         self.assertEquals(get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
                                        security.descriptor.from_sddl(sddl1, domsid),
@@ -91,14 +95,18 @@ class UpgradeProvisionTestCase(TestCaseInTempDir):
                            security.descriptor.from_sddl(sddl4, domsid),
                            domsid)
         txtmsg = "\tPart dacl is different between reference and current here\
- is the detail:\n\t\t(A;CIID;RPWPCRCCLCLORCWOWDSW;;;BA) ACE is not present in\
+ is the detail:\n\t\t(A;CI;RPWPCRCCLCLORCWOWDSW;;;BA) ACE is not present in\
- the reference\n\t\t(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA) ACE is not present in\
+ the reference\n\t\t(A;CI;RPWPCRCCLCLORCWOWDSW;;;SA) ACE is not present in\
  the current\n"
         self.assertEquals(txt, txtmsg)
+
         txt = get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
                            security.descriptor.from_sddl(sddl5, domsid),
                            domsid)
         self.assertEquals(txt, "\tCurrent ACL hasn't a sacl part\n")
+        self.assertEquals(get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
+                                       security.descriptor.from_sddl(sddl6, domsid),
+                                       domsid), "")
 
     def test_construct_existor_expr(self):
         res = construct_existor_expr([])

python/samba/upgradehelpers.py

@@ -33,7 +33,7 @@ from samba.provision import (provision_paths_from_lp,
                             getpolicypath, set_gpos_acl, create_gpo_struct,
                             FILL_FULL, provision, ProvisioningError,
                             setsysvolacl, secretsdb_self_join)
-from samba.dcerpc import xattr, drsblobs
+from samba.dcerpc import xattr, drsblobs, security
 from samba.dcerpc.misc import SEC_CHAN_BDC
 from samba.ndr import ndr_unpack
 from samba.samdb import SamDB
@@ -346,6 +346,46 @@ def chunck_sddl(sddl):
     return hash
 
 
+def get_clean_sd(sd):
+    """Get the SD without difference between 2 sddl
+
+    This function split the textual representation of ACL into smaller
+    chunck in order to not to report a simple permutation as a difference
+
+    :param refsddl: First sddl to compare
+    :param cursddl: Second sddl to compare
+    :param checkSacl: If false we skip the sacl checks
+    :return: A string that explain difference between sddls
+    """
+
+    sd_clean = security.descriptor()
+    sd_clean.owner_sid = sd.owner_sid
+    sd_clean.group_sid = sd.group_sid
+    sd_clean.type = sd.type
+    sd_clean.revision = sd.revision
+
+    aces = []
+    if sd.sacl is not None:
+        aces = sd.sacl.aces
+    for i in range(0, len(aces)):
+        ace = aces[i]
+
+        if not ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE:
+            sd_clean.sacl_add(ace)
+            continue
+
+    aces = []
+    if sd.dacl is not None:
+        aces = sd.dacl.aces
+    for i in range(0, len(aces)):
+        ace = aces[i]
+
+        if not ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE:
+            sd_clean.dacl_add(ace)
+            continue
+    return sd_clean
+
+
 def get_diff_sds(refsd, cursd, domainsid, checkSacl = True):
     """Get the difference between 2 sd
 
@@ -358,8 +398,8 @@ def get_diff_sds(refsd, cursd, domainsid, checkSacl = True):
     :return: A string that explain difference between sddls
     """
 
-    cursddl = cursd.as_sddl(domainsid)
+    cursddl = get_clean_sd(cursd).as_sddl(domainsid)
-    refsddl = refsd.as_sddl(domainsid)
+    refsddl = get_clean_sd(refsd).as_sddl(domainsid)
 
     txt = ""
     hash_cur = chunck_sddl(cursddl)