python/ntacls.py: only allow allow and deny ACEs in setntacl()

Commit 27dd0afb62d4f7427c966e984c7c8b01bc4d93b5 introduced a
regression.

Before that commit we included only SEC_ACE_TYPE_ACCESS_ALLOWED(0)
as 'not type & SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT' filtered out
SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT(5), but also
SEC_ACE_TYPE_ACCESS_DENIED and SEC_ACE_TYPE_ACCESS_DENIED_OBJECT.

After that commit we started to include
SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT, which is wrong.

It was also always wrong to exclude SEC_ACE_TYPE_ACCESS_DENIED(1).

So now we make it explicit that we only include
SEC_ACE_TYPE_ACCESS_ALLOWED and SEC_ACE_TYPE_ACCESS_DENIED.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14927

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>

diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
index 24af056d2a4be8d06770d1bfd19216067ccb0af7..7d315ed5127d779dd9bb421f1bd78349adffc0cc 100644 (file)
--- a/python/samba/ntacls.py
+++ b/python/samba/ntacls.py
@@ -300,17 +300,33 @@ def dsacl2fsacl(dssddl, sid, as_sddl=True):
     fdescr.type = ref.type
     fdescr.revision = ref.revision
     aces = ref.dacl.aces
+
     for i in range(0, len(aces)):
         ace = aces[i]
-        if ace.type in (security.SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT,
-                        security.SEC_ACE_TYPE_ACCESS_ALLOWED) and str(ace.trustee) != security.SID_BUILTIN_PREW2K:
-           #    if fdescr.type & security.SEC_DESC_DACL_AUTO_INHERITED:
-            ace.flags = ace.flags | security.SEC_ACE_FLAG_OBJECT_INHERIT | security.SEC_ACE_FLAG_CONTAINER_INHERIT
-            if str(ace.trustee) == security.SID_CREATOR_OWNER:
-                # For Creator/Owner the IO flag is set as this ACE has only a sense for child objects
-                ace.flags = ace.flags | security.SEC_ACE_FLAG_INHERIT_ONLY
-            ace.access_mask = ldapmask2filemask(ace.access_mask)
-            fdescr.dacl_add(ace)
+
+        # Only apply allowed and deny ACEs, as they are the only ones
+        # we can map to filesystem aces.
+        #
+        # In future we may need to include resource based aces...
+        allowed_ace_types = [
+            security.SEC_ACE_TYPE_ACCESS_ALLOWED,
+            security.SEC_ACE_TYPE_ACCESS_DENIED,
+        ]
+        if not ace.type in allowed_ace_types:
+            continue
+
+        # Don't add the allow for SID_BUILTIN_PREW2K as in
+        # gp_create_gpt_security_descriptor()
+        if str(ace.trustee) == security.SID_BUILTIN_PREW2K:
+            continue
+
+        ace.flags = ace.flags | security.SEC_ACE_FLAG_OBJECT_INHERIT | security.SEC_ACE_FLAG_CONTAINER_INHERIT
+        if str(ace.trustee) == security.SID_CREATOR_OWNER:
+            # For Creator/Owner the IO flag is set as this ACE has only a sense for child objects
+            ace.flags = ace.flags | security.SEC_ACE_FLAG_INHERIT_ONLY
+
+        ace.access_mask = ldapmask2filemask(ace.access_mask)
+        fdescr.dacl_add(ace)
 
     if not as_sddl:
         return fdescr

diff --git a/selftest/knownfail.d/python-ntacls b/selftest/knownfail.d/python-ntacls
deleted file mode 100644 (file)
index 6a15e23..0000000
--- a/selftest/knownfail.d/python-ntacls
+++ /dev/null
@@ -1 +0,0 @@
-samba.tests.ntacls.samba.tests.ntacls.NtaclsTests.test_dsacl2fsacl