From: Nadezhda Ivanova <nivanova@samba.org>
Date: Tue, 11 Jan 2011 10:20:43 +0000 (+0200)
Subject: security: Fixed bugs in expansion of generic information ACEs
X-Git-Url: http://git.samba.org/?a=commitdiff_plain;h=980f68a6f26070270313a4c7a4c0430f2bb3f078;p=metze%2Fsamba%2Fwip.git

security: Fixed bugs in expansion of generic information ACEs

When an ACE gontaining GA, GE, GR, GW, CO or CG is provided by a user or inherited
the final SD actually has to have 2 ACEs, one is an effective expanded one, and the
original one with IO flag added.
---

diff --git a/libcli/security/create_descriptor.c b/libcli/security/create_descriptor.c
index d5bb21bb544d..2228e48783ae 100644
--- a/libcli/security/create_descriptor.c
+++ b/libcli/security/create_descriptor.c
@@ -55,12 +55,12 @@ uint32_t map_generic_rights_ds(uint32_t access_mask)
 {
 	if (access_mask & SEC_GENERIC_ALL) {
 		access_mask |= SEC_ADS_GENERIC_ALL;
-		access_mask = ~SEC_GENERIC_ALL;
+		access_mask &= ~SEC_GENERIC_ALL;
 	}
 
 	if (access_mask & SEC_GENERIC_EXECUTE) {
 		access_mask |= SEC_ADS_GENERIC_EXECUTE;
-		access_mask = ~SEC_GENERIC_EXECUTE;
+		access_mask &= ~SEC_GENERIC_EXECUTE;
 	}
 
 	if (access_mask & SEC_GENERIC_WRITE) {
@@ -82,6 +82,45 @@ static bool object_in_list(struct GUID *object_list, struct GUID *object)
 {
 	return true;
 }
+ 
+/* returns true if the ACE gontains generic information
+ * that needs to be processed additionally */
+ 
+static bool desc_ace_has_generic(TALLOC_CTX *mem_ctx,
+			     struct security_ace *ace)
+{
+	struct dom_sid *co, *cg;
+	co = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_OWNER);
+	cg = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_GROUP);
+	if (ace->access_mask & SEC_GENERIC_ALL || ace->access_mask & SEC_GENERIC_READ ||
+	    ace->access_mask & SEC_GENERIC_WRITE || ace->access_mask & SEC_GENERIC_EXECUTE) {
+		return true;
+	}
+	if (dom_sid_equal(&ace->trustee, co) || dom_sid_equal(&ace->trustee, cg)) {
+		return true;
+	}
+	return false;
+}
+
+/* creates an ace in which the generic information is expanded */
+
+static void desc_expand_generic(TALLOC_CTX *mem_ctx,
+				struct security_ace *new_ace,
+				struct dom_sid *owner,
+				struct dom_sid *group)
+{
+	struct dom_sid *co, *cg;
+	co = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_OWNER);
+	cg = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_GROUP);
+	new_ace->access_mask = map_generic_rights_ds(new_ace->access_mask);
+	if (dom_sid_equal(&new_ace->trustee, co)) {
+		new_ace->trustee = *owner;
+	}
+	if (dom_sid_equal(&new_ace->trustee, cg)) {
+		new_ace->trustee = *group;
+	}
+	new_ace->flags = 0x0;
+}
 
 static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 							    struct security_acl *acl,
@@ -108,7 +147,8 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 		struct security_ace *ace = &acl->aces[i];
 		if ((ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT) ||
 		    (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
-			tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces, struct security_ace,
+			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);
@@ -128,30 +168,24 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 				}
 
 			}
-			tmp_acl->aces[tmp_acl->num_aces].access_mask =
-						    map_generic_rights_ds(ace->access_mask);
 			tmp_acl->num_aces++;
 			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,
+				    (desc_ace_has_generic(tmp_ctx, ace))) {
+					    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);
+					    desc_expand_generic(tmp_ctx,
+								&tmp_acl->aces[tmp_acl->num_aces],
+								owner,
+								group);
+					    tmp_acl->aces[tmp_acl->num_aces].flags = SEC_ACE_FLAG_INHERITED_ACE;
 					    tmp_acl->num_aces++;
 				}
 			}
@@ -200,29 +234,39 @@ static struct security_acl *process_user_acl(TALLOC_CTX *mem_ctx,
 		      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))
-			continue;
-
-		tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces, struct security_ace,
+		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;
-		tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
 		tmp_acl->num_aces++;
+		if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
+			continue;
+		}
+		/* if the ACE contains CO, CG, GA, GE, GR or GW, and is inheritable
+		 * it has to be expanded to two aces, the original as IO,
+		 * and another one where these are translated */
+		if (desc_ace_has_generic(tmp_ctx, ace)) {
+			if (!(ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT)) {
+				desc_expand_generic(tmp_ctx,
+						    &tmp_acl->aces[tmp_acl->num_aces-1],
+						    owner,
+						    group);
+			} else {
+			/*The original ACE becomes read only */
+			tmp_acl->aces[tmp_acl->num_aces-1].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
+			tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces,
+						       struct security_ace,
+						       tmp_acl->num_aces+1);
+			/* add a new ACE with expanded generic info */
+			tmp_acl->aces[tmp_acl->num_aces] = *ace;
+			desc_expand_generic(tmp_ctx,
+					    &tmp_acl->aces[tmp_acl->num_aces],
+					    owner,
+					    group);
+			tmp_acl->num_aces++;
+			}
+		}
 	}
 	new_acl = security_acl_dup(mem_ctx,tmp_acl);