From 4fc59089c81b251b4fab17f170e96bd6dac02490 Mon Sep 17 00:00:00 2001 From: Nadezhda Ivanova Date: Tue, 20 Apr 2010 00:23:42 +0300 Subject: Removed more excess looping and fixed problem with incorrect IO flag handling. --- source4/lib/ldb/tests/python/sec_descriptor.py | 33 ++++ source4/libcli/security/create_descriptor.c | 207 ++++++++++--------------- 2 files changed, 114 insertions(+), 126 deletions(-) diff --git a/source4/lib/ldb/tests/python/sec_descriptor.py b/source4/lib/ldb/tests/python/sec_descriptor.py index 609fca86ab..f26df07df1 100755 --- 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 f4849cf9c6..d64de2fe22 100644 --- 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; -- cgit