libsepol/0083-libsepol-cil-Allow-duplicate-optional-blocks-in-most.patch

From 67a8dc8117e0c3887c39f7add8932e4ad23c1d9c Mon Sep 17 00:00:00 2001
From: James Carter <jwcart2@gmail.com>
Date: Wed, 16 Jun 2021 17:04:00 -0400
Subject: [PATCH] libsepol/cil: Allow duplicate optional blocks in most cases

The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) fixed a bug
that allowed duplicate blocks, optionals, and macros with the same
name in the same namespace. For blocks and macros, a duplicate
is always a problem, but optional block names are only used for
in-statement resolution. If no in-statement refers to an optional
block, then it does not matter if more than one with same name
exists.

One easy way to generate multiple optional blocks with the same
name declaration is to call a macro with an optional block multiple
times in the same namespace.

As an example, here is a portion of CIL policy
  (macro m1 ((type t))
    (optional op1
      (allow t self (CLASS (PERM)))
    )
  )
  (type t1)
  (call m1 (t1))
  (type t2)
  (call m1 (t2))
This will result in two optional blocks with the name op1.

There are three parts to allowing multiple optional blocks with
the same name declaration.

1) Track an optional block's enabled status in a different way.

   One hinderance to allowing multiple optional blocks with the same
   name declaration is that they cannot share the same datum. This is
   because the datum is used to get the struct cil_optional which has
   the enabled field and each block's enabled status is independent of
   the others.

   Remove the enabled field from struct cil_optional, so it only contains
   the datum. Use a stack to track which optional blocks are being
   disabled, so they can be deleted in the right order.

2) Allow multiple declarations of optional blocks.

   Update cil_allow_multiple_decls() so that a flavor of CIL_OPTIONAL
   will return CIL_TRUE. Also remove the check in cil_copy_optional().

3) Check if an in-statement refers to an optional with multiple
   declarations and exit with an error if it does.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/cil/src/cil.c             |  1 -
 libsepol/cil/src/cil_build_ast.c   |  3 ++
 libsepol/cil/src/cil_copy_ast.c    | 11 +-----
 libsepol/cil/src/cil_internal.h    |  1 -
 libsepol/cil/src/cil_resolve_ast.c | 55 ++++++++++++++++++------------
 5 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 0d351b491c2c..671b5ec6e183 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -2752,7 +2752,6 @@ void cil_call_init(struct cil_call **call)
 void cil_optional_init(struct cil_optional **optional)
 {
 	*optional = cil_malloc(sizeof(**optional));
-	(*optional)->enabled = CIL_TRUE;
 	cil_symtab_datum_init(&(*optional)->datum);
 }
 
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 71ddada226df..ea665a323f78 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -96,6 +96,9 @@ static int cil_allow_multiple_decls(struct cil_db *db, enum cil_flavor f_new, en
 			return CIL_TRUE;
 		}
 		break;
+	case CIL_OPTIONAL:
+		return CIL_TRUE;
+		break;
 	default:
 		break;
 	}
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 954eab330340..9c0231f294e3 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -1529,19 +1529,10 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
 	return SEPOL_OK;
 }
 
-int cil_copy_optional(__attribute__((unused)) struct cil_db *db, void *data, void **copy, symtab_t *symtab)
+int cil_copy_optional(__attribute__((unused)) struct cil_db *db, __attribute__((unused)) void *data, void **copy, __attribute__((unused)) symtab_t *symtab)
 {
-	struct cil_optional *orig = data;
-	char *key = orig->datum.name;
-	struct cil_symtab_datum *datum = NULL;
 	struct cil_optional *new;
 
-	cil_symtab_get_datum(symtab, key, &datum);
-	if (datum != NULL) {
-		cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
-		return SEPOL_ERR;
-	}
-
 	cil_optional_init(&new);
 	*copy = new;
 
diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
index a77c95201fb7..24be09aa33d8 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -358,7 +358,6 @@ struct cil_in {
 
 struct cil_optional {
 	struct cil_symtab_datum datum;
-	int enabled;
 };
 
 struct cil_perm {
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index b5199bad35e2..6d13544c80f7 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -46,12 +46,13 @@
 #include "cil_verify.h"
 #include "cil_strpool.h"
 #include "cil_symtab.h"
+#include "cil_stack.h"
 
 struct cil_args_resolve {
 	struct cil_db *db;
 	enum cil_pass pass;
 	uint32_t *changed;
-	struct cil_list *disabled_optionals;
+	struct cil_list *to_destroy;
 	struct cil_tree_node *block;
 	struct cil_tree_node *macro;
 	struct cil_tree_node *optional;
@@ -62,6 +63,7 @@ struct cil_args_resolve {
 	struct cil_list *catorder_lists;
 	struct cil_list *sensitivityorder_lists;
 	struct cil_list *in_list;
+	struct cil_stack *disabled_optionals;
 };
 
 static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key, struct cil_tree_node *ast_node)
@@ -2552,6 +2554,15 @@ int cil_resolve_in(struct cil_tree_node *current, void *extra_args)
 
 	block_node = NODE(block_datum);
 
+	if (block_node->flavor == CIL_OPTIONAL) {
+		if (block_datum->nodes && block_datum->nodes->head != block_datum->nodes->tail) {
+			cil_tree_log(current, CIL_ERR, "Multiple optional blocks referred to by in-statement");
+			cil_tree_log(block_node, CIL_ERR, "First optional block");
+			rc = SEPOL_ERR;
+			goto exit;
+		}
+	}
+
 	rc = cil_copy_ast(db, current, block_node);
 	if (rc != SEPOL_OK) {
 		cil_tree_log(current, CIL_ERR, "Failed to copy in-statement");
@@ -3874,6 +3885,7 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished
 	struct cil_tree_node *macro = args->macro;
 	struct cil_tree_node *optional = args->optional;
 	struct cil_tree_node *boolif = args->boolif;
+	struct cil_stack *disabled_optionals = args->disabled_optionals;
 
 	if (node == NULL) {
 		goto exit;
@@ -3953,22 +3965,14 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished
 
 	rc = __cil_resolve_ast_node(node, extra_args);
 	if (rc == SEPOL_ENOENT) {
-		enum cil_log_level lvl = CIL_ERR;
-
-		if (optional != NULL) {
-			struct cil_optional *opt = (struct cil_optional *)optional->data;
-			struct cil_tree_node *opt_node = NODE(opt);
-
-			lvl = CIL_INFO;
-			/* disable an optional if something failed to resolve */
-			opt->enabled = CIL_FALSE;
-			cil_tree_log(node, lvl, "Failed to resolve %s statement", cil_node_to_string(node));
-			cil_tree_log(opt_node, lvl, "Disabling optional '%s'", opt->datum.name);
+		if (optional == NULL) {
+			cil_tree_log(node, CIL_ERR, "Failed to resolve %s statement", cil_node_to_string(node));
+		} else {
+			cil_stack_push(disabled_optionals, CIL_NODE, optional);
+			cil_tree_log(node, CIL_INFO, "Failed to resolve %s statement", cil_node_to_string(node));
+			cil_tree_log(optional, CIL_INFO, "Disabling optional '%s'", DATUM(optional->data)->name);
 			rc = SEPOL_OK;
-			goto exit;
 		}
-
-		cil_tree_log(node, lvl, "Failed to resolve %s statement", cil_node_to_string(node));
 		goto exit;
 	}
 
@@ -4011,6 +4015,7 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext
 {
 	int rc = SEPOL_ERR;
 	struct cil_args_resolve *args = extra_args;
+	struct cil_stack *disabled_optionals = args->disabled_optionals;
 	struct cil_tree_node *parent = NULL;
 
 	if (current == NULL ||  extra_args == NULL) {
@@ -4033,9 +4038,11 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext
 		args->macro = NULL;
 	} else if (parent->flavor == CIL_OPTIONAL) {
 		struct cil_tree_node *n = parent->parent;
-		if (((struct cil_optional *)parent->data)->enabled == CIL_FALSE) {
+		struct cil_stack_item *item = cil_stack_peek(disabled_optionals);
+		if (item && item->data == parent) {
+			cil_stack_pop(disabled_optionals);
 			*(args->changed) = CIL_TRUE;
-			cil_list_append(args->disabled_optionals, CIL_NODE, parent);
+			cil_list_append(args->to_destroy, CIL_NODE, parent);
 		}
 		args->optional = NULL;
 		while (n && n->flavor != CIL_ROOT) {
@@ -4079,14 +4086,17 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 	extra_args.catorder_lists = NULL;
 	extra_args.sensitivityorder_lists = NULL;
 	extra_args.in_list = NULL;
+	extra_args.disabled_optionals = NULL;
 
-	cil_list_init(&extra_args.disabled_optionals, CIL_NODE);
+	cil_list_init(&extra_args.to_destroy, CIL_NODE);
 	cil_list_init(&extra_args.sidorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.classorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.unordered_classorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.catorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.sensitivityorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.in_list, CIL_IN);
+	cil_stack_init(&extra_args.disabled_optionals);
+
 	for (pass = CIL_PASS_TIF; pass < CIL_PASS_NUM; pass++) {
 		extra_args.pass = pass;
 		rc = cil_tree_walk(current, __cil_resolve_ast_node_helper, __cil_resolve_ast_first_child_helper, __cil_resolve_ast_last_child_helper, &extra_args);
@@ -4179,11 +4189,11 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 					goto exit;
 				}
 			}
-			cil_list_for_each(item, extra_args.disabled_optionals) {
+			cil_list_for_each(item, extra_args.to_destroy) {
 				cil_tree_children_destroy(item->data);
 			}
-			cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE);
-			cil_list_init(&extra_args.disabled_optionals, CIL_NODE);
+			cil_list_destroy(&extra_args.to_destroy, CIL_FALSE);
+			cil_list_init(&extra_args.to_destroy, CIL_NODE);
 			changed = 0;
 		}
 	}
@@ -4200,8 +4210,9 @@ exit:
 	__cil_ordered_lists_destroy(&extra_args.catorder_lists);
 	__cil_ordered_lists_destroy(&extra_args.sensitivityorder_lists);
 	__cil_ordered_lists_destroy(&extra_args.unordered_classorder_lists);
-	cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE);
+	cil_list_destroy(&extra_args.to_destroy, CIL_FALSE);
 	cil_list_destroy(&extra_args.in_list, CIL_FALSE);
+	cil_stack_destroy(&extra_args.disabled_optionals);
 
 	return rc;
 }
-- 
2.32.0