Removed more excess looping and fixed problem with incorrect IO flag handling.

diff --git a/source4/lib/ldb/tests/python/sec_descriptor.py b/source4/lib/ldb/tests/python/sec_descriptor.py
index 609fca86ab9fbd48603e5d05c2e955d435ddf38e..f26df07df13394ab341a607ce9278b51519431d2 100755 (executable)
--- a/source4/lib/ldb/tests/python/sec_descriptor.py
+++ b/source4/lib/ldb/tests/python/sec_descriptor.py
@@ -1725,6 +1725,39 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.get_desc_sddl(group_dn)
         self.assertTrue("(D;;WP;;;DA)(D;CIIO;WP;;;CO)" in desc_sddl)
 
+    def test_212(self):
+        """ Provide ACE with IO flag, should be ignored
+        """
+        ou_dn = "OU=test_inherit_ou," + self.base_dn
+        group_dn = "CN=test_inherit_group," + ou_dn
+        # Create inheritable-free OU
+        self.create_clean_ou(ou_dn)
+        # Add some custom 'CI' ACE
+        mod = "D:(D;CIIO;WP;;;CO)"
+        self.create_domain_group(self.ldb_admin, group_dn, mod)
+        # Make sure created group object contains only the above inherited ACE(s)
+        # that we've added manually
+        desc_sddl = self.get_desc_sddl(group_dn)
+        print desc_sddl
+        self.assertTrue("(D;CIIO;WP;;;CO)" in desc_sddl)
+        self.assertFalse("(D;;WP;;;DA)" in desc_sddl)
+        self.assertFalse("(D;CIIO;WP;;;CO)(D;CIIO;WP;;;CO)" in desc_sddl)
+
+    def test_213(self):
+        """ Provide ACE with IO flag, should be ignored
+        """
+        ou_dn = "OU=test_inherit_ou," + self.base_dn
+        group_dn = "CN=test_inherit_group," + ou_dn
+        # Create inheritable-free OU
+        self.create_clean_ou(ou_dn)
+        mod = "D:(D;IO;WP;;;DA)"
+        self.create_domain_group(self.ldb_admin, group_dn, mod)
+        # Make sure created group object contains only the above inherited ACE(s)
+        # that we've added manually
+        desc_sddl = self.get_desc_sddl(group_dn)
+        print desc_sddl
+        self.assertFalse("(D;IO;WP;;;DA)" in desc_sddl)
+
     ########################################################################################
 
 

diff --git a/source4/libcli/security/create_descriptor.c b/source4/libcli/security/create_descriptor.c
index f4849cf9c6205a6b71f72acd161ee6f3eaa6a1a2..d64de2fe226ad9dc56e1b5bc6cf5a0510921820f 100644 (file)
--- a/source4/libcli/security/create_descriptor.c
+++ b/source4/libcli/security/create_descriptor.c
@@ -53,22 +53,22 @@
 
 uint32_t map_generic_rights_ds(uint32_t access_mask)
 {
-       if (access_mask & SEC_GENERIC_ALL){
+       if (access_mask & SEC_GENERIC_ALL) {
                access_mask |= SEC_ADS_GENERIC_ALL;
                access_mask = ~SEC_GENERIC_ALL;
        }
 
-       if (access_mask & SEC_GENERIC_EXECUTE){
+       if (access_mask & SEC_GENERIC_EXECUTE) {
                access_mask |= SEC_ADS_GENERIC_EXECUTE;
                access_mask = ~SEC_GENERIC_EXECUTE;
        }
 
-       if (access_mask & SEC_GENERIC_WRITE){
+       if (access_mask & SEC_GENERIC_WRITE) {
                access_mask |= SEC_ADS_GENERIC_WRITE;
                access_mask &= ~SEC_GENERIC_WRITE;
        }
 
-       if (access_mask & SEC_GENERIC_READ){
+       if (access_mask & SEC_GENERIC_READ) {
                access_mask |= SEC_ADS_GENERIC_READ;
                access_mask &= ~SEC_GENERIC_READ;
        }
@@ -83,85 +83,20 @@ static bool object_in_list(struct GUID *object_list, struct GUID *object)
        return true;
 }
 
- /* remove any ACEs with inherited flag up  - TODO test this! */
-static struct security_acl *clean_user_acl(TALLOC_CTX *mem, struct security_acl *acl)
-{
-       int i;
-       struct security_acl *new_acl; 
-       if (!acl) {
-               return NULL;
-       }
-       
-       new_acl = talloc_zero(mem, struct security_acl);
-
-       for (i=0; i < acl->num_aces; i++) {
-               struct security_ace *ace = &acl->aces[i];
-               if (!(ace->flags & SEC_ACE_FLAG_INHERITED_ACE)){
-                       new_acl->aces = talloc_realloc(new_acl, new_acl->aces, struct security_ace,
-                                          new_acl->num_aces+1);
-                       if (new_acl->aces == NULL) {
-                               talloc_free(new_acl);
-                               return NULL;
-                       }
-                       new_acl->aces[new_acl->num_aces] = *ace;
-                       new_acl->num_aces++;
-               }
-       }
-       if (new_acl)
-               new_acl->revision = acl->revision;
-
-       return new_acl;
-}
-
-/* sort according to rules,
- * replace generic flags with the mapping
- * replace CO and CG with the appropriate owner/group */
-
-static bool postprocess_acl(struct security_acl *acl,
-                           struct dom_sid *owner,
-                           struct dom_sid *group,
-                           uint32_t (*generic_map)(uint32_t access_mask))
-{
-       int i;
-       struct dom_sid *co, *cg;
-       TALLOC_CTX *tmp_ctx = talloc_new(acl);
-       if (!generic_map){
-               return false;
-       }
-       co = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_OWNER);
-       cg = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_GROUP);
-       for (i=0; i < acl->num_aces; i++) {
-               struct security_ace *ace = &acl->aces[i];
-               if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY)
-                       continue;
-               if (dom_sid_equal(&ace->trustee, co)){
-                       ace->trustee = *owner;
-                       ace->flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
-               }
-               if (dom_sid_equal(&ace->trustee, cg)){
-                       ace->trustee = *group;
-                       ace->flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
-                       }
-               ace->access_mask = generic_map(ace->access_mask);
-       }
-
-       talloc_free(tmp_ctx);
-       return true;
-}
-
 static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
-                                               struct security_acl *acl,
-                                               bool is_container,
-                                               struct GUID *object_list)
+                                                           struct security_acl *acl,
+                                                           bool is_container,
+                                                           struct dom_sid *owner,
+                                                           struct dom_sid *group,
+                                                           struct GUID *object_list)
 {
        int i;
        TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
-       struct security_acl *tmp_acl = talloc_zero(tmp_ctx, struct security_acl);
-       struct security_acl *inh_acl = talloc_zero(tmp_ctx, struct security_acl);
-       struct security_acl *new_acl;
+       struct security_acl *tmp_acl = talloc_zero(mem_ctx, struct security_acl);
        struct dom_sid *co, *cg;
-       if (!tmp_acl || !inh_acl)
+       if (!tmp_acl) {
                return NULL;
+       }
 
        if (!acl) {
                return NULL;
@@ -169,7 +104,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
        co = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_OWNER);
        cg = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_GROUP);
 
-       for (i=0; i < acl->num_aces; i++){
+       for (i=0; i < acl->num_aces; i++) {
                struct security_ace *ace = &acl->aces[i];
                if ((ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT) ||
                    (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
@@ -188,53 +123,55 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 
                        if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT ||
                            ace->type == SEC_ACE_TYPE_ACCESS_DENIED_OBJECT) {
-                               if (!object_in_list(object_list, &ace->object.object.type.type)){
+                               if (!object_in_list(object_list, &ace->object.object.type.type)) {
                                        tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
                                }
 
                        }
+                       tmp_acl->aces[tmp_acl->num_aces].access_mask =
+                                                   map_generic_rights_ds(ace->access_mask);
                        tmp_acl->num_aces++;
-               }
-       }
-
-       if (is_container) {
-               for (i=0; i < acl->num_aces; i++){
-                       struct security_ace *ace = &acl->aces[i];
-
-                       if (ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT)
-                               continue;
-                       if (!dom_sid_equal(&ace->trustee, co) && !dom_sid_equal(&ace->trustee, cg))
-                               continue;
-
-                       if ((ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT) ||
-                           (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)){
-                               inh_acl->aces = talloc_realloc(inh_acl, inh_acl->aces, struct security_ace,
-                                                              inh_acl->num_aces+1);
-                               if (inh_acl->aces == NULL){
-                                       talloc_free(tmp_ctx);
-                                       return NULL;
+                       if (is_container) {
+                               if (!(ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) &&
+                                   ((dom_sid_equal(&ace->trustee, co) || dom_sid_equal(&ace->trustee, cg)))) {
+                                           tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces, struct security_ace,
+                                                                          tmp_acl->num_aces+1);
+                                           if (tmp_acl->aces == NULL) {
+                                                   talloc_free(tmp_ctx);
+                                                   return NULL;
+                                           }
+                                           tmp_acl->aces[tmp_acl->num_aces] = *ace;
+                                           tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
+                                           tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERITED_ACE;
+                                           if (dom_sid_equal(&tmp_acl->aces[tmp_acl->num_aces].trustee, co)) {
+                                                   tmp_acl->aces[tmp_acl->num_aces].trustee = *owner;
+                                           }
+                                           if (dom_sid_equal(&tmp_acl->aces[tmp_acl->num_aces].trustee, cg)) {
+                                                   tmp_acl->aces[tmp_acl->num_aces].trustee = *group;
+                                           }
+                                           tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
+                                           tmp_acl->aces[tmp_acl->num_aces].access_mask =
+                                                   map_generic_rights_ds(ace->access_mask);
+                                           tmp_acl->num_aces++;
                                }
-                               inh_acl->aces[inh_acl->num_aces] = *ace;
-                               inh_acl->aces[inh_acl->num_aces].flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
-                               inh_acl->aces[inh_acl->num_aces].flags |= SEC_ACE_FLAG_INHERITED_ACE;
-                               inh_acl->num_aces++;
                        }
                }
-               }
-       new_acl = security_acl_concatenate(mem_ctx, inh_acl, tmp_acl);
-       if (new_acl->num_aces == 0) {
+       }
+       if (tmp_acl->num_aces == 0) {
                return NULL;
        }
-       if (new_acl)
-               new_acl->revision = acl->revision;
-       talloc_free(tmp_ctx);
-       return new_acl;
+       if (acl) {
+               tmp_acl->revision = acl->revision;
+       }
+       return tmp_acl;
 }
 
-static struct security_acl *calculate_inherited_from_creator(TALLOC_CTX *mem_ctx,
-                                                            struct security_acl *acl,
-                                                            bool is_container,
-                                                            struct GUID *object_list)
+static struct security_acl *process_user_acl(TALLOC_CTX *mem_ctx,
+                                            struct security_acl *acl,
+                                            bool is_container,
+                                            struct dom_sid *owner,
+                                            struct dom_sid *group,
+                                            struct GUID *object_list)
 {
        int i;
        TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
@@ -258,10 +195,24 @@ static struct security_acl *calculate_inherited_from_creator(TALLOC_CTX *mem_ctx
                struct security_ace *ace = &acl->aces[i];
                if (ace->flags & SEC_ACE_FLAG_INHERITED_ACE)
                        continue;
+               if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY &&
+                   !(ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT ||
+                     ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT))
+                       continue;
 
                tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces, struct security_ace,
                                              tmp_acl->num_aces+1);
                tmp_acl->aces[tmp_acl->num_aces] = *ace;
+               if (dom_sid_equal(&(tmp_acl->aces[tmp_acl->num_aces].trustee), co)) {
+                       tmp_acl->aces[tmp_acl->num_aces].trustee = *owner;
+                       tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
+               }
+               if (dom_sid_equal(&(tmp_acl->aces[tmp_acl->num_aces].trustee), cg)) {
+                       tmp_acl->aces[tmp_acl->num_aces].trustee = *group;
+                       tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
+                       }
+               tmp_acl->aces[tmp_acl->num_aces].access_mask =
+                       map_generic_rights_ds(tmp_acl->aces[tmp_acl->num_aces].access_mask);
                tmp_acl->num_aces++;
 
                if (!dom_sid_equal(&ace->trustee, co) && !dom_sid_equal(&ace->trustee, cg))
@@ -319,7 +270,7 @@ static bool compute_acl(struct security_descriptor *parent_sd,
                        struct security_token *token,
                        struct security_descriptor *new_sd) /* INOUT argument */
 {
-       struct security_acl *user_dacl, *tmp_dacl, *tmp_sacl, *user_sacl, *inherited_dacl, *inherited_sacl;
+       struct security_acl *user_dacl, *user_sacl, *inherited_dacl, *inherited_sacl;
        int level = 10;
 
        if (!parent_sd || !(inherit_flags & SEC_DACL_AUTO_INHERIT)) {
@@ -330,6 +281,8 @@ static bool compute_acl(struct security_descriptor *parent_sd,
                inherited_dacl = calculate_inherited_from_parent(new_sd,
                                                                 parent_sd->dacl,
                                                                 is_container,
+                                                                new_sd->owner_sid,
+                                                                new_sd->group_sid,
                                                                 object_list);
        }
 
@@ -342,6 +295,8 @@ static bool compute_acl(struct security_descriptor *parent_sd,
                inherited_sacl = calculate_inherited_from_parent(new_sd,
                                                                 parent_sd->sacl,
                                                                 is_container,
+                                                                new_sd->owner_sid,
+                                                                new_sd->group_sid,
                                                                 object_list);
        }
 
@@ -349,16 +304,18 @@ static bool compute_acl(struct security_descriptor *parent_sd,
                user_dacl = NULL;
                user_sacl = NULL;
        } else {
-               tmp_dacl = clean_user_acl(new_sd, creator_sd->dacl);
-               user_dacl = calculate_inherited_from_creator(new_sd,
-                                                            tmp_dacl,
-                                                            is_container,
-                                                            object_list);
-               tmp_sacl = clean_user_acl(new_sd, creator_sd->sacl);
-               user_sacl = calculate_inherited_from_creator(new_sd,
-                                                            tmp_sacl,
-                                                            is_container,
-                                                            object_list);
+               user_dacl = process_user_acl(new_sd,
+                                            creator_sd->dacl,
+                                            is_container,
+                                            new_sd->owner_sid,
+                                            new_sd->group_sid,
+                                            object_list);
+               user_sacl = process_user_acl(new_sd,
+                                            creator_sd->sacl,
+                                            is_container,
+                                            new_sd->owner_sid,
+                                            new_sd->group_sid,
+                                            object_list);
        }
        cr_descr_log_descriptor(parent_sd, __location__"parent_sd", level);
        cr_descr_log_descriptor(creator_sd,__location__ "creator_sd", level);
@@ -366,7 +323,6 @@ static bool compute_acl(struct security_descriptor *parent_sd,
        new_sd->dacl = security_acl_concatenate(new_sd, user_dacl, inherited_dacl);
        if (new_sd->dacl) {
                new_sd->type |= SEC_DESC_DACL_PRESENT;
-               postprocess_acl(new_sd->dacl,new_sd->owner_sid,new_sd->group_sid,generic_map);
        }
        if (inherited_dacl) {
                new_sd->type |= SEC_DESC_DACL_AUTO_INHERITED;
@@ -375,7 +331,6 @@ static bool compute_acl(struct security_descriptor *parent_sd,
        new_sd->sacl = security_acl_concatenate(new_sd, user_sacl, inherited_sacl);
        if (new_sd->sacl) {
                new_sd->type |= SEC_DESC_SACL_PRESENT;
-               postprocess_acl(new_sd->sacl,new_sd->owner_sid,new_sd->group_sid,generic_map);
        }
        if (inherited_sacl) {
                new_sd->type |= SEC_DESC_SACL_AUTO_INHERITED;