Attachment 17861 Details for Bug 15338 – Patch for v4-17-test (without tests)

[patch] Patch for v4-17-test (without tests)

bfixes-tmp417.txt (text/plain), 9.59 KB, created by Stefan Metzmacher on 2023-04-12 12:40:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2023-04-12 12:40:27 UTC

Size: 9.59 KB

patch

obsolete

>From fc422ed60c2bb557dedf87ff1911df28aa23480d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Sat, 18 Mar 2023 01:17:04 +0100
>Subject: [PATCH] libcli/security: rewrite calculate_inherited_from_parent()
>
>This allows us to pass the new tests we just added.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15338
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit bb09c06d6d58a04e1d270a9f99d1179cfa9acbda)
>(cherry picked from commit 08a0b11392587d68ad9ff8467fb6ca08d490e98d)
>---
> libcli/security/create_descriptor.c | 247 +++++++++++++++++++++-------
> 1 file changed, 192 insertions(+), 55 deletions(-)
>
>diff --git a/libcli/security/create_descriptor.c b/libcli/security/create_descriptor.c
>index ef60d847033f..947d6c19d588 100644
>--- a/libcli/security/create_descriptor.c
>+++ b/libcli/security/create_descriptor.c
>@@ -78,7 +78,7 @@ uint32_t map_generic_rights_ds(uint32_t access_mask)
> 
> /* Not sure what this has to be,
> * and it does not seem to have any influence */
>-static bool object_in_list(struct GUID *object_list, struct GUID *object)
>+static bool object_in_list(const struct GUID *object_list, const struct GUID *object)
> {
> 	size_t i;
> 
>@@ -107,7 +107,7 @@ static bool object_in_list(struct GUID *object_list, struct GUID *object)
> /* returns true if the ACE gontains generic information
>  * that needs to be processed additionally */
>  
>-static bool desc_ace_has_generic(struct security_ace *ace)
>+static bool desc_ace_has_generic(const struct security_ace *ace)
> {
> 	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) {
>@@ -155,12 +155,114 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
> 	}
> 
> 	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)) {
>-			struct GUID inherited_object = GUID_zero();
>+		const struct security_ace *ace = &acl->aces[i];
>+		const struct GUID *inherited_object = NULL;
>+		const struct GUID *inherited_property = NULL;
>+		struct security_ace *tmp_ace = NULL;
>+		bool applies = false;
>+		bool inherited_only = false;
>+		bool expand_ace = false;
>+		bool expand_only = false;
>+
>+		if (is_container && (ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT)) {
>+			applies = true;
>+		} else if (!is_container && (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
>+			applies = true;
>+		}
>+
>+		if (!applies) {
>+			/*
>+			 * If the ace doesn't apply to the
>+			 * current node, we should only keep
>+			 * it as SEC_ACE_FLAG_OBJECT_INHERIT
>+			 * on a container. We'll add
>+			 * SEC_ACE_FLAG_INHERITED_ACE
>+			 * and SEC_ACE_FLAG_INHERIT_ONLY below.
>+			 *
>+			 * Otherwise we should completely ignore it.
>+			 */
>+			if (!(ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
>+				continue;
>+			}
>+		}
>+
>+		switch (ace->type) {
>+		case SEC_ACE_TYPE_ACCESS_ALLOWED:
>+		case SEC_ACE_TYPE_ACCESS_DENIED:
>+		case SEC_ACE_TYPE_SYSTEM_AUDIT:
>+		case SEC_ACE_TYPE_SYSTEM_ALARM:
>+		case SEC_ACE_TYPE_ALLOWED_COMPOUND:
>+			break;
>+
>+		case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
>+		case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
>+		case SEC_ACE_TYPE_SYSTEM_ALARM_OBJECT:
>+		case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT:
>+			if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) {
>+				inherited_property = &ace->object.object.type.type;
>+			}
>+			if (ace->object.object.flags & SEC_ACE_INHERITED_OBJECT_TYPE_PRESENT) {
>+				inherited_object = &ace->object.object.inherited_type.inherited_type;
>+			}
>+
>+			if (inherited_object != NULL && !object_in_list(object_list, inherited_object)) {
>+				/*
>+				 * An explicit object class schemaId is given,
>+				 * but doesn't belong to the current object.
>+				 */
>+				applies = false;
>+			}
> 
>-			tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces,
>+			break;
>+		}
>+
>+		if (ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) {
>+			if (!applies) {
>+				/*
>+				 * If the ACE doesn't apply to
>+				 * the current object, we should
>+				 * ignore it as it should not be
>+				 * inherited any further
>+				 */
>+				continue;
>+			}
>+			/*
>+			 * We should only keep the expanded version
>+			 * of the ACE on the current object.
>+			 */
>+			expand_ace = true;
>+			expand_only = true;
>+		} else if (applies) {
>+			/*
>+			 * We check if should also add
>+			 * the expanded version of the ACE
>+			 * in addition, in case we should
>+			 * expand generic access bits or
>+			 * special sids.
>+			 *
>+			 * In that case we need to
>+			 * keep the original ACE with
>+			 * SEC_ACE_FLAG_INHERIT_ONLY.
>+			 */
>+			expand_ace = desc_ace_has_generic(ace);
>+			if (expand_ace) {
>+				inherited_only = true;
>+			}
>+		} else {
>+			/*
>+			 * If the ACE doesn't apply
>+			 * to the current object,
>+			 * we need to keep it with
>+			 * SEC_ACE_FLAG_INHERIT_ONLY
>+			 * in order to apply them to
>+			 * grandchildren
>+			 */
>+			inherited_only = true;
>+		}
>+
>+		if (expand_ace) {
>+			tmp_acl->aces = talloc_realloc(tmp_acl,
>+						       tmp_acl->aces,
> 						       struct security_ace,
> 						       tmp_acl->num_aces+1);
> 			if (tmp_acl->aces == NULL) {
>@@ -168,61 +270,96 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
> 				return NULL;
> 			}
> 
>-			tmp_acl->aces[tmp_acl->num_aces] = *ace;
>-			tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERITED_ACE;
>-			/* remove IO flag from the child's ace */
>-			if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY &&
>-			    !desc_ace_has_generic(ace)) {
>-				tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
>-			}
>+			tmp_ace = &tmp_acl->aces[tmp_acl->num_aces];
>+			tmp_acl->num_aces++;
> 
>-			if (is_container && (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT))
>-			    tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
>-
>-			switch (ace->type) {
>-			case SEC_ACE_TYPE_ACCESS_ALLOWED:
>-			case SEC_ACE_TYPE_ACCESS_DENIED:
>-			case SEC_ACE_TYPE_SYSTEM_AUDIT:
>-			case SEC_ACE_TYPE_SYSTEM_ALARM:
>-			case SEC_ACE_TYPE_ALLOWED_COMPOUND:
>-				break;
>-
>-			case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
>-			case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
>-			case SEC_ACE_TYPE_SYSTEM_ALARM_OBJECT:
>-			case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT:
>-				if (ace->object.object.flags & SEC_ACE_INHERITED_OBJECT_TYPE_PRESENT) {
>-					inherited_object = ace->object.object.inherited_type.inherited_type;
>-				}
>+			*tmp_ace = *ace;
>+
>+			/*
>+			 * Expand generic access bits as well as special
>+			 * sids.
>+			 */
>+			desc_expand_generic(tmp_ace, owner, group);
>+
>+			/*
>+			 * Expanded ACEs are marked as inherited,
>+			 * but never inherited any further to
>+			 * grandchildren.
>+			 */
>+			tmp_ace->flags |= SEC_ACE_FLAG_INHERITED_ACE;
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_OBJECT_INHERIT;
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_NO_PROPAGATE_INHERIT;
>+
>+			/*
>+			 * Expanded ACEs never have an explicit
>+			 * object class schemaId, so clear it
>+			 * if present.
>+			 */
>+			if (inherited_object != NULL) {
>+				tmp_ace->object.object.flags &= ~SEC_ACE_INHERITED_OBJECT_TYPE_PRESENT;
>+			}
> 
>-				if (!object_in_list(object_list, &inherited_object)) {
>-					tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
>+			/*
>+			 * If the ACE had an explicit object class
>+			 * schemaId, but no attribute/propertySet
>+			 * we need to downgrate the _OBJECT variants
>+			 * to the normal ones.
>+			 */
>+			if (inherited_property == NULL) {
>+				switch (tmp_ace->type) {
>+				case SEC_ACE_TYPE_ACCESS_ALLOWED:
>+				case SEC_ACE_TYPE_ACCESS_DENIED:
>+				case SEC_ACE_TYPE_SYSTEM_AUDIT:
>+				case SEC_ACE_TYPE_SYSTEM_ALARM:
>+				case SEC_ACE_TYPE_ALLOWED_COMPOUND:
>+					break;
>+				case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
>+					tmp_ace->type = SEC_ACE_TYPE_ACCESS_ALLOWED;
>+					break;
>+				case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
>+					tmp_ace->type = SEC_ACE_TYPE_ACCESS_DENIED;
>+					break;
>+				case SEC_ACE_TYPE_SYSTEM_ALARM_OBJECT:
>+					tmp_ace->type = SEC_ACE_TYPE_SYSTEM_ALARM;
>+					break;
>+				case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT:
>+					tmp_ace->type = SEC_ACE_TYPE_SYSTEM_AUDIT;
>+					break;
> 				}
>-
>-				break;
> 			}
> 
>-			tmp_acl->num_aces++;
>-			if (is_container) {
>-				if (!(ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) &&
>-				    (desc_ace_has_generic(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;
>-					    desc_expand_generic(&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++;
>-				}
>+			if (expand_only) {
>+				continue;
> 			}
> 		}
>+
>+		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_ace = &tmp_acl->aces[tmp_acl->num_aces];
>+		tmp_acl->num_aces++;
>+
>+		*tmp_ace = *ace;
>+		tmp_ace->flags |= SEC_ACE_FLAG_INHERITED_ACE;
>+
>+		if (inherited_only) {
>+			tmp_ace->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
>+		} else {
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
>+		}
>+
>+		if (ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) {
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_CONTAINER_INHERIT;
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_OBJECT_INHERIT;
>+			tmp_ace->flags &= ~SEC_ACE_FLAG_NO_PROPAGATE_INHERIT;
>+		}
> 	}
> 	if (tmp_acl->num_aces == 0) {
> 		return NULL;
>-- 
>2.34.1
>

Flags:

jsutton: review+

Actions: View

Attachments on bug 15338: 17860 | 17861 | 17874 | 17875