scripting: Rework samba.upgradehelpers.get_diff_sddls to be get_diff_sds

This moves the SDDL conversion inside the get_diff_sds function and prepares
for removing inherited ACEs from the SD before comparison.

Andrew Bartlett

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

3 files changed, 35 insertions, 20 deletions

diff --git a/python/samba/tests/upgradeprovision.py b/python/samba/tests/upgradeprovision.py
index 93a6731c83..f0d34b12f4 100644
--- a/python/samba/tests/upgradeprovision.py
+++ b/python/samba/tests/upgradeprovision.py
@@ -19,7 +19,7 @@
 
 import os
 from samba.upgradehelpers import (usn_in_range, dn_sort,
-                                  get_diff_sddls, update_secrets,
+                                  get_diff_sds, update_secrets,
                                   construct_existor_expr)
 
 from samba.tests.provision import create_dummy_secretsdb
@@ -27,6 +27,7 @@ from samba.tests import TestCaseInTempDir
 from samba import Ldb
 from ldb import SCOPE_BASE
 import samba.tests
+from samba.dcerpc import security
 
 def dummymessage(a=None, b=None):
     pass
@@ -59,7 +60,9 @@ class UpgradeProvisionTestCase(TestCaseInTempDir):
         self.assertEquals(dn_sort("cn=bar, dc=toto,dc=tata",
                                     "cn=foo, dc=toto,dc=tata"), -1)
 
-    def test_get_diff_sddl(self):
+    def test_get_diff_sds(self):
+        domsid = security.dom_sid('S-1-5-21')
+
         sddl = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
 (A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)S:AI(AU;CIIDSA;WP;;;WD)"
         sddl1 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
@@ -73,18 +76,28 @@ class UpgradeProvisionTestCase(TestCaseInTempDir):
         sddl5 = "O:SAG:DUD:AI(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA)\
 (A;CIID;RP LCLORC;;;AU)(A;CIID;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)"
 
-        self.assertEquals(get_diff_sddls(sddl, sddl1), "")
-        txt = get_diff_sddls(sddl, sddl2)
+        self.assertEquals(get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
+                                       security.descriptor.from_sddl(sddl1, domsid),
+                                       domsid), "")
+        txt = get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
+                           security.descriptor.from_sddl(sddl2, domsid),
+                           domsid)
         self.assertEquals(txt, "\tOwner mismatch: SA (in ref) BA(in current)\n")
-        txt = get_diff_sddls(sddl, sddl3)
+        txt = get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
+                           security.descriptor.from_sddl(sddl3, domsid),
+                           domsid)
         self.assertEquals(txt, "\tGroup mismatch: DU (in ref) BA(in current)\n")
-        txt = get_diff_sddls(sddl, sddl4)
+        txt = get_diff_sds(security.descriptor.from_sddl(sddl, domsid),
+                           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\
  the reference\n\t\t(A;CIID;RPWPCRCCLCLORCWOWDSW;;;SA) ACE is not present in\
  the current\n"
         self.assertEquals(txt, txtmsg)
-        txt = get_diff_sddls(sddl, sddl5)
+        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")
 
     def test_construct_existor_expr(self):
diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
index 1ec19d4ab6..88182bd4a1 100644
--- a/python/samba/upgradehelpers.py
+++ b/python/samba/upgradehelpers.py
@@ -346,8 +346,8 @@ def chunck_sddl(sddl):
     return hash
 
 
-def get_diff_sddls(refsddl, cursddl, checkSacl = True):
-    """Get the difference between 2 sddl
+def get_diff_sds(refsd, cursd, domainsid, checkSacl = True):
+    """Get the difference between 2 sd
 
     This function split the textual representation of ACL into smaller
     chunck in order to not to report a simple permutation as a difference
@@ -358,6 +358,9 @@ def get_diff_sddls(refsddl, cursddl, checkSacl = True):
     :return: A string that explain difference between sddls
     """
 
+    cursddl = cursd.as_sddl(domainsid)
+    refsddl = refsd.as_sddl(domainsid)
+
     txt = ""
     hash_cur = chunck_sddl(cursddl)
     hash_ref = chunck_sddl(refsddl)
diff --git a/source4/scripting/bin/samba_upgradeprovision b/source4/scripting/bin/samba_upgradeprovision
index 36f6a60068..9cd5c8147a 100755
--- a/source4/scripting/bin/samba_upgradeprovision
+++ b/source4/scripting/bin/samba_upgradeprovision
@@ -75,7 +75,7 @@ from samba.dcerpc.security import (
 from samba.ndr import ndr_unpack
 from samba.upgradehelpers import (dn_sort, get_paths, newprovision,
                                  get_ldbs, findprovisionrange,
-                                 usn_in_range, identic_rename, get_diff_sddls,
+                                 usn_in_range, identic_rename, get_diff_sds,
                                  update_secrets, CHANGE, ERROR, SIMPLE,
                                  CHANGEALL, GUESS, CHANGESD, PROVISION,
                                  updateOEMInfo, getOEMInfo, update_gpo,
@@ -963,12 +963,10 @@ def checkKeepAttributeWithMetadata(delta, att, message, reference, current,
         if att == "nTSecurityDescriptor":
             cursd = ndr_unpack(security.descriptor,
                 str(current[0]["nTSecurityDescriptor"]))
-            cursddl = cursd.as_sddl(names.domainsid)
             refsd = ndr_unpack(security.descriptor,
                 str(reference[0]["nTSecurityDescriptor"]))
-            refsddl = refsd.as_sddl(names.domainsid)
 
-            diff = get_diff_sddls(refsddl, cursddl)
+            diff = get_diff_sds(refsd, cursd, names.domainsid)
             if diff == "":
                 # FIXME find a way to have it only with huge huge verbose mode
                 # message(CHANGE, "%ssd are identical" % txt)
@@ -1267,19 +1265,20 @@ def check_updated_sd(ref_sam, cur_sam, names):
                                 controls=["search_options:1:2"])
     hash = {}
     for i in range(0, len(reference)):
-        refsd = ndr_unpack(security.descriptor,
-                    str(reference[i]["nTSecurityDescriptor"]))
-        hash[str(reference[i]["dn"]).lower()] = refsd.as_sddl(names.domainsid)
+        refsd_blob = str(reference[i]["nTSecurityDescriptor"])
+        hash[str(reference[i]["dn"]).lower()] = refsd_blob
 
 
     for i in range(0, len(current)):
         key = str(current[i]["dn"]).lower()
         if hash.has_key(key):
+            cursd_blob = str(current[i]["nTSecurityDescriptor"])
             cursd = ndr_unpack(security.descriptor,
-                        str(current[i]["nTSecurityDescriptor"]))
-            sddl = cursd.as_sddl(names.domainsid)
-            if sddl != hash[key]:
-                txt = get_diff_sddls(hash[key], sddl, False)
+                               cursd_blob)
+            if cursd_blob != hash[key]:
+                refsd = ndr_unpack(security.descriptor,
+                                   hash[key])
+                txt = get_diff_sds(refsd, cursd, names.domainsid, False)
                 if txt != "":
                     message(CHANGESD, "On object %s ACL is different"
                                       " \n%s" % (current[i]["dn"], txt))