All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/9] Add CIL Deny Rule
@ 2022-12-15 21:34 James Carter
  2022-12-15 21:34 ` [RFC PATCH 1/9] libsepol/cil: Parse and add deny rule to AST, but do not process James Carter
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

I don't expect this to be part of the upcoming userspace release,
but I did want to see if this is going to be what Cascade needs.

This series of patches implements a deny rule in CIL. A deny rule will remove
the stated permissions in it from the policy. CIL does this by searching for
allow rules that match the deny rule and then writing new allow rules that
correspond to the matched allow rule with the permissions from the deny rule
removed. The rule uses the same syntax as an allow rule, but with "deny"
instead of "allow".

  (deny SRC TGT (CLASS (PERMS)))

Deny rules are processed during post processing (after the AST is resolved,
but before the binary policy is written). This means that neverallow checking
is done after deny rules are resolved. Deny rules are complimentary to
neverallow checking. When an allow rule is found that matches, a deny rule
removes permissions while a neverallow rule reports an error.

Patch 4 is biggest and most complex since it is the one doing the processing.

James Carter (9):
  libsepol/cil: Parse and add deny rule to AST, but do not process
  libsepol/cil: Add cil_list_is_empty macro
  libsepol/cil: Add cil_tree_remove_node function
  libsepol/cil: Process deny rules
  libsepol/cil: Add cil_write_post_ast function
  libsepol: Export the cil_write_post_ast function
  secilc/secil2tree: Add option to write CIL AST after post processing
  secilc/test: Add a deny rule test
  secilc/docs: Add deny rule to CIL documentation

 libsepol/cil/include/cil/cil.h         |   1 +
 libsepol/cil/src/cil.c                 |  68 ++
 libsepol/cil/src/cil_build_ast.c       |  56 ++
 libsepol/cil/src/cil_build_ast.h       |   2 +
 libsepol/cil/src/cil_copy_ast.c        |  19 +
 libsepol/cil/src/cil_copy_ast.h        |   1 +
 libsepol/cil/src/cil_deny.c            | 957 +++++++++++++++++++++++++
 libsepol/cil/src/cil_deny.h            |  34 +
 libsepol/cil/src/cil_flavor.h          |   1 +
 libsepol/cil/src/cil_internal.h        |  10 +
 libsepol/cil/src/cil_list.h            |   3 +
 libsepol/cil/src/cil_post.c            |   7 +
 libsepol/cil/src/cil_reset_ast.c       |   8 +
 libsepol/cil/src/cil_resolve_ast.c     |  44 ++
 libsepol/cil/src/cil_resolve_ast.h     |   1 +
 libsepol/cil/src/cil_tree.c            |  27 +
 libsepol/cil/src/cil_tree.h            |   1 +
 libsepol/cil/src/cil_verify.c          |   9 +
 libsepol/cil/src/cil_write_ast.c       |  10 +
 libsepol/cil/src/cil_write_ast.h       |   1 +
 libsepol/src/libsepol.map.in           |   5 +
 secilc/docs/cil_access_vector_rules.md |  68 ++
 secilc/secil2tree.c                    |   8 +-
 secilc/test/deny_rule_test.cil         | 384 ++++++++++
 24 files changed, 1724 insertions(+), 1 deletion(-)
 create mode 100644 libsepol/cil/src/cil_deny.c
 create mode 100644 libsepol/cil/src/cil_deny.h
 create mode 100644 secilc/test/deny_rule_test.cil

-- 
2.38.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [RFC PATCH 1/9] libsepol/cil: Parse and add deny rule to AST, but do not process
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
@ 2022-12-15 21:34 ` James Carter
  2022-12-15 21:34 ` [RFC PATCH 2/9] libsepol/cil: Add cil_list_is_empty macro James Carter
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Adds the ability to parse a deny rule, add it to the AST, and
write it out when writing the AST, but the deny rule is otherwise
ignored and does nothing.

When it is fully supported, the deny rule will work like a neverallow
except that it will remove permissions rather than give an error.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil.c             | 18 ++++++++++
 libsepol/cil/src/cil_build_ast.c   | 56 ++++++++++++++++++++++++++++++
 libsepol/cil/src/cil_build_ast.h   |  2 ++
 libsepol/cil/src/cil_copy_ast.c    | 19 ++++++++++
 libsepol/cil/src/cil_copy_ast.h    |  1 +
 libsepol/cil/src/cil_flavor.h      |  1 +
 libsepol/cil/src/cil_internal.h    | 10 ++++++
 libsepol/cil/src/cil_reset_ast.c   |  8 +++++
 libsepol/cil/src/cil_resolve_ast.c | 44 +++++++++++++++++++++++
 libsepol/cil/src/cil_resolve_ast.h |  1 +
 libsepol/cil/src/cil_verify.c      |  9 +++++
 libsepol/cil/src/cil_write_ast.c   | 10 ++++++
 12 files changed, 179 insertions(+)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 38edcf8e..0b35ad35 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -225,6 +225,7 @@ char *CIL_KEY_SRC_CIL;
 char *CIL_KEY_SRC_HLL_LMS;
 char *CIL_KEY_SRC_HLL_LMX;
 char *CIL_KEY_SRC_HLL_LME;
+char *CIL_KEY_DENY_RULE;
 
 static void cil_init_keys(void)
 {
@@ -394,6 +395,7 @@ static void cil_init_keys(void)
 	CIL_KEY_SRC_HLL_LMS = cil_strpool_add("lms");
 	CIL_KEY_SRC_HLL_LMX = cil_strpool_add("lmx");
 	CIL_KEY_SRC_HLL_LME = cil_strpool_add("lme");
+	CIL_KEY_DENY_RULE = cil_strpool_add("deny");
 }
 
 void cil_db_init(struct cil_db **db)
@@ -915,6 +917,9 @@ void cil_destroy_data(void **data, enum cil_flavor flavor)
 	case CIL_PERMISSIONX:
 		cil_destroy_permissionx(*data);
 		break;
+	case CIL_DENY_RULE:
+		cil_destroy_deny_rule(*data);
+		break;
 	case CIL_ROLETRANSITION:
 		cil_destroy_roletransition(*data);
 		break;
@@ -1291,6 +1296,8 @@ const char * cil_node_to_string(struct cil_tree_node *node)
 		break;
 	case CIL_PERMISSIONX:
 		return CIL_KEY_PERMISSIONX;
+	case CIL_DENY_RULE:
+		return CIL_KEY_DENY_RULE;
 	case CIL_ROLETRANSITION:
 		return CIL_KEY_ROLETRANSITION;
 	case CIL_TYPE_RULE:
@@ -2470,6 +2477,17 @@ void cil_permissionx_init(struct cil_permissionx **permx)
 	(*permx)->perms = NULL;
 }
 
+void cil_deny_rule_init(struct cil_deny_rule **rule)
+{
+	*rule = cil_malloc(sizeof(**rule));
+
+	(*rule)->src_str = NULL;
+	(*rule)->src = NULL;
+	(*rule)->tgt_str = NULL;
+	(*rule)->tgt = NULL;
+	(*rule)->classperms = NULL;
+}
+
 void cil_type_rule_init(struct cil_type_rule **type_rule)
 {
 	*type_rule = cil_malloc(sizeof(**type_rule));
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 4177c9f6..1afc274f 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -2289,6 +2289,60 @@ exit:
 	return rc;
 }
 
+int cil_gen_deny_rule(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node)
+{
+	enum cil_syntax syntax[] = {
+		CIL_SYN_STRING,
+		CIL_SYN_STRING,
+		CIL_SYN_STRING,
+		CIL_SYN_STRING | CIL_SYN_LIST,
+		CIL_SYN_END
+	};
+	size_t syntax_len = sizeof(syntax)/sizeof(*syntax);
+	struct cil_deny_rule *rule = NULL;
+	int rc = SEPOL_ERR;
+
+	if (parse_current == NULL || ast_node == NULL) {
+		goto exit;
+	}
+
+	rc = __cil_verify_syntax(parse_current, syntax, syntax_len);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	cil_deny_rule_init(&rule);
+
+	rule->src_str = parse_current->next->data;
+	rule->tgt_str = parse_current->next->next->data;
+
+	rc = cil_fill_classperms_list(parse_current->next->next->next, &rule->classperms);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	ast_node->data = rule;
+	ast_node->flavor = CIL_DENY_RULE;
+
+	return SEPOL_OK;
+
+exit:
+	cil_tree_log(parse_current, CIL_ERR, "Bad deny rule");
+	cil_destroy_deny_rule(rule);
+	return rc;
+}
+
+void cil_destroy_deny_rule(struct cil_deny_rule *rule)
+{
+	if (rule == NULL) {
+		return;
+	}
+
+	cil_destroy_classperms_list(&rule->classperms);
+
+	free(rule);
+}
+
 int cil_gen_type_rule(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node, uint32_t rule_kind)
 {
 	enum cil_syntax syntax[] = {
@@ -6365,6 +6419,8 @@ static struct cil_tree_node * parse_statement(struct cil_db *db, struct cil_tree
 		rc = cil_gen_avrulex(parse_current, new_ast_node, CIL_AVRULE_NEVERALLOW);
 	} else if (parse_current->data == CIL_KEY_PERMISSIONX) {
 		rc = cil_gen_permissionx(db, parse_current, new_ast_node);
+	} else if (parse_current->data == CIL_KEY_DENY_RULE) {
+		rc = cil_gen_deny_rule(parse_current, new_ast_node);
 	} else if (parse_current->data == CIL_KEY_TYPETRANSITION) {
 		rc = cil_gen_typetransition(db, parse_current, new_ast_node);
 	} else if (parse_current->data == CIL_KEY_TYPECHANGE) {
diff --git a/libsepol/cil/src/cil_build_ast.h b/libsepol/cil/src/cil_build_ast.h
index fd9053ce..aca84b24 100644
--- a/libsepol/cil/src/cil_build_ast.h
+++ b/libsepol/cil/src/cil_build_ast.h
@@ -116,6 +116,8 @@ void cil_destroy_avrule(struct cil_avrule *rule);
 int cil_gen_avrulex(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node, uint32_t rule_kind);
 int cil_gen_permissionx(struct cil_db *db, struct cil_tree_node *parse_current, struct cil_tree_node *ast_node);
 void cil_destroy_permissionx(struct cil_permissionx *permx);
+int cil_gen_deny_rule(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node);
+void cil_destroy_deny_rule(struct cil_deny_rule *rule);
 int cil_gen_type_rule(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node, uint32_t rule_kind);
 void cil_destroy_type_rule(struct cil_type_rule *rule);
 int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct cil_tree_node *ast_node);
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 17f05021..bc972f03 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -854,6 +854,22 @@ static int cil_copy_permissionx(struct cil_db *db, void *data, void **copy, symt
 	return SEPOL_OK;
 }
 
+int cil_copy_deny_rule(__attribute__((unused)) struct cil_db *db, void *data, void **copy, __attribute__((unused)) symtab_t *symtab)
+{
+	struct cil_deny_rule *orig = data;
+	struct cil_deny_rule *new = NULL;
+
+	cil_deny_rule_init(&new);
+
+	new->src_str = orig->src_str;
+	new->tgt_str = orig->tgt_str;
+	cil_copy_classperms_list(orig->classperms, &new->classperms);
+
+	*copy = new;
+
+	return SEPOL_OK;
+}
+
 int cil_copy_type_rule(__attribute__((unused)) struct cil_db *db, void *data, void **copy, __attribute__((unused)) symtab_t *symtab)
 {
 	struct cil_type_rule  *orig = data;
@@ -1860,6 +1876,9 @@ static int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished
 	case CIL_PERMISSIONX:
 		copy_func = &cil_copy_permissionx;
 		break;
+	case CIL_DENY_RULE:
+		copy_func = &cil_copy_deny_rule;
+		break;
 	case CIL_TYPE_RULE:
 		copy_func = &cil_copy_type_rule;
 		break;
diff --git a/libsepol/cil/src/cil_copy_ast.h b/libsepol/cil/src/cil_copy_ast.h
index a50c3708..9f695ec5 100644
--- a/libsepol/cil/src/cil_copy_ast.h
+++ b/libsepol/cil/src/cil_copy_ast.h
@@ -80,6 +80,7 @@ int cil_copy_nametypetransition(struct cil_db *db, void *data, void **copy, symt
 int cil_copy_rangetransition(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
 int cil_copy_bool(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
 int cil_copy_avrule(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
+int cil_copy_deny_rule(__attribute__((unused)) struct cil_db *db, void *data, void **copy, __attribute__((unused)) symtab_t *symtab);
 int cil_copy_type_rule(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
 int cil_copy_sens(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
 int cil_copy_sensalias(struct cil_db *db, void *data, void **copy, symtab_t *symtab);
diff --git a/libsepol/cil/src/cil_flavor.h b/libsepol/cil/src/cil_flavor.h
index c2f0cee7..89ab7875 100644
--- a/libsepol/cil/src/cil_flavor.h
+++ b/libsepol/cil/src/cil_flavor.h
@@ -86,6 +86,7 @@ enum cil_flavor {
 	CIL_ROLEALLOW,
 	CIL_AVRULE,
 	CIL_AVRULEX,
+	CIL_DENY_RULE,
 	CIL_ROLETRANSITION,
 	CIL_TYPE_RULE,
 	CIL_NAMETYPETRANSITION,
diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
index a7604762..b15cbf64 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -242,6 +242,7 @@ extern char *CIL_KEY_SRC_CIL;
 extern char *CIL_KEY_SRC_HLL_LMS;
 extern char *CIL_KEY_SRC_HLL_LMX;
 extern char *CIL_KEY_SRC_HLL_LME;
+extern char *CIL_KEY_DENY_RULE;
 
 /*
 	Symbol Table Array Indices
@@ -632,6 +633,14 @@ struct cil_permissionx {
 	ebitmap_t *perms;
 };
 
+struct cil_deny_rule {
+	char *src_str;
+	void *src; /* type, alias, or attribute */
+	char *tgt_str;
+	void *tgt; /* type, alias, or attribute */
+	struct cil_list *classperms;
+};
+
 #define CIL_TYPE_TRANSITION 16
 #define CIL_TYPE_MEMBER     32
 #define CIL_TYPE_CHANGE     64
@@ -1037,6 +1046,7 @@ void cil_tunable_init(struct cil_tunable **ciltun);
 void cil_tunif_init(struct cil_tunableif **tif);
 void cil_avrule_init(struct cil_avrule **avrule);
 void cil_permissionx_init(struct cil_permissionx **permx);
+void cil_deny_rule_init(struct cil_deny_rule **rule);
 void cil_type_rule_init(struct cil_type_rule **type_rule);
 void cil_roletransition_init(struct cil_roletransition **roletrans);
 void cil_roleallow_init(struct cil_roleallow **role_allow);
diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c
index 0864d7ef..9069317e 100644
--- a/libsepol/cil/src/cil_reset_ast.c
+++ b/libsepol/cil/src/cil_reset_ast.c
@@ -218,6 +218,11 @@ static void cil_reset_avrule(struct cil_avrule *rule)
 	cil_reset_classperms_list(rule->perms.classperms);
 }
 
+static void cil_reset_deny_rule(struct cil_deny_rule *rule)
+{
+	cil_reset_classperms_list(rule->classperms);
+}
+
 static void cil_reset_rangetransition(struct cil_rangetransition *rangetrans)
 {
 	if (rangetrans->range_str == NULL) {
@@ -545,6 +550,9 @@ static int __cil_reset_node(struct cil_tree_node *node,  __attribute__((unused))
 	case CIL_AVRULE:
 		cil_reset_avrule(node->data);
 		break;
+	case CIL_DENY_RULE:
+		cil_reset_deny_rule(node->data);
+		break;
 	case CIL_SENS:
 		cil_reset_sens(node->data);
 		break;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index f5e22c97..941bdde4 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -371,6 +371,47 @@ exit:
 	return rc;
 }
 
+int cil_resolve_deny_rule(struct cil_tree_node *current, void *extra_args)
+{
+	struct cil_args_resolve *args = extra_args;
+	struct cil_db *db = NULL;
+
+	struct cil_deny_rule *rule = current->data;
+	struct cil_symtab_datum *src_datum = NULL;
+	struct cil_symtab_datum *tgt_datum = NULL;
+	int rc = SEPOL_ERR;
+
+	if (args != NULL) {
+		db = args->db;
+	}
+
+	rc = cil_resolve_name(current, rule->src_str, CIL_SYM_TYPES, args, &src_datum);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+	rule->src = src_datum;
+
+	if (rule->tgt_str == CIL_KEY_SELF) {
+		rule->tgt = db->selftype;
+	} else {
+		rc = cil_resolve_name(current, rule->tgt_str, CIL_SYM_TYPES, args, &tgt_datum);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+		rule->tgt = tgt_datum;
+	}
+
+	rc = cil_resolve_classperms_list(current, rule->classperms, extra_args);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	return SEPOL_OK;
+
+exit:
+	return rc;
+}
+
 int cil_resolve_type_rule(struct cil_tree_node *current, void *extra_args)
 {
 	struct cil_args_resolve *args = extra_args;
@@ -3779,6 +3820,9 @@ static int __cil_resolve_ast_node(struct cil_tree_node *node, void *extra_args)
 		case CIL_PERMISSIONX:
 			rc = cil_resolve_permissionx(node, (struct cil_permissionx*)node->data, args);
 			break;
+		case CIL_DENY_RULE:
+			rc = cil_resolve_deny_rule(node, args);
+			break;
 		case CIL_TYPE_RULE:
 			rc = cil_resolve_type_rule(node, args);
 			break;
diff --git a/libsepol/cil/src/cil_resolve_ast.h b/libsepol/cil/src/cil_resolve_ast.h
index 1d971fd6..78357993 100644
--- a/libsepol/cil/src/cil_resolve_ast.h
+++ b/libsepol/cil/src/cil_resolve_ast.h
@@ -40,6 +40,7 @@ int cil_resolve_classperms(struct cil_tree_node *current, struct cil_classperms
 int cil_resolve_classpermissionset(struct cil_tree_node *current, struct cil_classpermissionset *cps, void *extra_args);
 int cil_resolve_classperms_list(struct cil_tree_node *current, struct cil_list *cp_list, void *extra_args);
 int cil_resolve_avrule(struct cil_tree_node *current, void *extra_args);
+int cil_resolve_deny_rule(struct cil_tree_node *current, void *extra_args);
 int cil_resolve_type_rule(struct cil_tree_node *current, void *extra_args);
 int cil_resolve_typeattributeset(struct cil_tree_node *current, void *extra_args);
 int cil_resolve_typealias(struct cil_tree_node *current, void *extra_args);
diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 4640dc59..70cebc16 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -1040,6 +1040,15 @@ static int __cil_verify_booleanif_helper(struct cil_tree_node *node, __attribute
 		}
 		break;
 	}
+	case CIL_DENY_RULE:
+		if (bif->preserved_tunable) {
+			cil_tree_log(node, CIL_ERR, "Not allowed to have a deny rule in a tunableif block (treated as a booleanif due to preserve-tunables)");
+		} else {
+			cil_tree_log(node, CIL_ERR, "Not allowed to have deny rule in a booleanif block");
+		}
+		rc = SEPOL_ERR;
+		goto exit;
+		break;
 	case CIL_TYPE_RULE: /*
 	struct cil_type_rule *typerule = NULL;
 	struct cil_tree_node *temp_node = NULL;
diff --git a/libsepol/cil/src/cil_write_ast.c b/libsepol/cil/src/cil_write_ast.c
index b75784ef..4da7a77c 100644
--- a/libsepol/cil/src/cil_write_ast.c
+++ b/libsepol/cil/src/cil_write_ast.c
@@ -1144,6 +1144,16 @@ void cil_write_ast_node(FILE *out, struct cil_tree_node *node)
 		fprintf(out, ")\n");
 		break;
 	}
+	case CIL_DENY_RULE: {
+		struct cil_deny_rule *rule = node->data;
+		fprintf(out, "(deny ");
+
+		fprintf(out, "%s ", datum_or_str(DATUM(rule->src), rule->src_str));
+		fprintf(out, "%s ", datum_or_str(DATUM(rule->tgt), rule->tgt_str));
+		write_classperms_list(out, rule->classperms);
+		fprintf(out, ")\n");
+		break;
+	}
 	case CIL_TYPE_RULE: {
 		struct cil_type_rule *rule = node->data;
 		if (rule->rule_kind == AVRULE_TRANSITION)
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 2/9] libsepol/cil: Add cil_list_is_empty macro
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
  2022-12-15 21:34 ` [RFC PATCH 1/9] libsepol/cil: Parse and add deny rule to AST, but do not process James Carter
@ 2022-12-15 21:34 ` James Carter
  2022-12-15 21:34 ` [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function James Carter
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Add a macro, called cil_list_is_empty, that returns true if the
list pointer or list head is NULL.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_list.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libsepol/cil/src/cil_list.h b/libsepol/cil/src/cil_list.h
index 6b4708a0..f974fddc 100644
--- a/libsepol/cil/src/cil_list.h
+++ b/libsepol/cil/src/cil_list.h
@@ -44,6 +44,9 @@ struct cil_list_item {
 	void *data;
 };
 
+#define cil_list_is_empty(list) \
+	((list) == NULL || (list)->head == NULL)
+
 #define cil_list_for_each(item, list) \
 	for (item = (list)->head; item != NULL; item = item->next)
 
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
  2022-12-15 21:34 ` [RFC PATCH 1/9] libsepol/cil: Parse and add deny rule to AST, but do not process James Carter
  2022-12-15 21:34 ` [RFC PATCH 2/9] libsepol/cil: Add cil_list_is_empty macro James Carter
@ 2022-12-15 21:34 ` James Carter
  2023-02-03 22:54   ` Daniel Burgener
  2022-12-15 21:34 ` [RFC PATCH 4/9] libsepol/cil: Process deny rules James Carter
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Add the function cil_tree_remove_node() which takes a node pointer
as an input, finds the parent, walks the list of nodes to the node
prior to the given node, and then updates that nodes next pointer
to remove the given node from the tree.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++
 libsepol/cil/src/cil_tree.h |  1 +
 2 files changed, 28 insertions(+)

diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
index 6376c208..73b4e135 100644
--- a/libsepol/cil/src/cil_tree.c
+++ b/libsepol/cil/src/cil_tree.c
@@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node)
 	*node = NULL;
 }
 
+void cil_tree_remove_node(struct cil_tree_node *node)
+{
+	struct cil_tree_node *parent, *curr;
+
+	if (node == NULL || node->parent == NULL) {
+		return;
+	}
+
+	parent = node->parent;
+
+	if (parent->cl_head == node) {
+		parent->cl_head = node->next;
+		return;
+	}
+
+	curr = parent->cl_head;
+	while (curr && curr->next != node) {
+		curr = curr->next;
+	}
+
+	if (curr == NULL) {
+		return;
+	}
+
+	curr->next = node->next;
+}
+
 /* Perform depth-first walk of the tree
    Parameters:
    start_node:          root node to start walking from
diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
index 5a98da55..e4b3fd04 100644
--- a/libsepol/cil/src/cil_tree.h
+++ b/libsepol/cil/src/cil_tree.h
@@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node);
 
 void cil_tree_node_init(struct cil_tree_node **node);
 void cil_tree_node_destroy(struct cil_tree_node **node);
+void cil_tree_remove_node(struct cil_tree_node *node);
 
 //finished values
 #define CIL_TREE_SKIP_NOTHING	0
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 4/9] libsepol/cil: Process deny rules
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (2 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function James Carter
@ 2022-12-15 21:34 ` James Carter
  2023-02-03 22:54   ` Daniel Burgener
  2022-12-15 21:34 ` [RFC PATCH 5/9] libsepol/cil: Add cil_write_post_ast function James Carter
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

A deny rule is like a neverallow rule, except that permissions are
removed rather than an error reported.

(allow S1 T1 P1)
(deny  S2 T2 P2)

In general what is needed is:
1) Allow permissions that do not match the deny rule
  P3 = P1 and not P2
  (allow S1 T1 P3)
2) Allow source types that do not match the deny rule
  S3 = S1 and not S2
  (allow S3 T1 P1)
3) Allow target types that do not match the deny rule
  T3 = T1 and not T2
  (allow S1 T3 P1)

If a self rule is involved then
  If S1 is an attribute then
    If T1 is self and T2 is not self then
      S3 = S1 and not (S2 and T2)
    else if T1 is not self and T2 is self then
      S3 = S1 and not (T1 and S2)
    else if T1 is self and T2 is self then
      S3 = S1 and not S2
    (allow S3 T1 P1)
  If T1 is an attribute then
    If T2 is self then
      T3 = T1 and not (S1 and S2)
    (allow S1 T3 P1)
    If S1 is attribute and S2 is attribute and T2 is self then
      # In addition to any rules involving S3 and T3 above
      SX = S1 and S2 and T1
      For S in SX do
        T = SX and not S
        (allow S T P1)

In all cases, do not create a rule that has an empty set

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_deny.c | 957 ++++++++++++++++++++++++++++++++++++
 libsepol/cil/src/cil_deny.h |  34 ++
 libsepol/cil/src/cil_post.c |   7 +
 3 files changed, 998 insertions(+)
 create mode 100644 libsepol/cil/src/cil_deny.c
 create mode 100644 libsepol/cil/src/cil_deny.h

diff --git a/libsepol/cil/src/cil_deny.c b/libsepol/cil/src/cil_deny.c
new file mode 100644
index 00000000..43043c37
--- /dev/null
+++ b/libsepol/cil/src/cil_deny.c
@@ -0,0 +1,957 @@
+/*
+ * This file is public domain software, i.e. not copyrighted.
+ *
+ * Warranty Exclusion
+ * ------------------
+ * You agree that this software is a non-commercially developed program
+ * that may contain "bugs" (as that term is used in the industry) and
+ * that it may not function as intended. The software is licensed
+ * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
+ * express, implied, statutory, or otherwise with respect to the software,
+ * including noninfringement and the implied warranties of merchantability
+ * and fitness for a particular purpose.
+ *
+ * Limitation of Liability
+ *-----------------------
+ * In no event will NSA be liable for any damages, including loss of data,
+ * lost profits, cost of cover, or other special, incidental, consequential,
+ * direct or indirect damages arising from the software or the use thereof,
+ * however caused and on any theory of liability. This limitation will apply
+ * even if NSA has been advised of the possibility of such damage. You
+ * acknowledge that this is a reasonable allocation of risk.
+ *
+ * Original author: James Carter
+ */
+
+#include <sepol/policydb/ebitmap.h>
+
+#include "cil_internal.h"
+#include "cil_find.h"
+#include "cil_flavor.h"
+#include "cil_list.h"
+#include "cil_strpool.h"
+#include "cil_log.h"
+#include "cil_symtab.h"
+#include "cil_build_ast.h"
+#include "cil_copy_ast.h"
+#include "cil_deny.h"
+
+#define CIL_DENY_ATTR_PREFIX "deny_rule_attr"
+
+/*
+ * A deny rule will remove the specified permissions from the CIL AST.
+ *
+ * (allow S1 T1 P1)
+ * (deny  S2 T2 P2)
+ *
+ * In general what is needed is:
+ * 1) Allow permissions that do not match the deny rule
+ *   P3 = P1 and not P2
+ *   (allow S1 T1 P3)
+ * 2) Allow source types that do not match the deny rule
+ *   S3 = S1 and not S2
+ *   (allow S3 T1 P1)
+ * 3) Allow target types that do not match the deny rule
+ *   T3 = T1 and not T2
+ *   (allow S1 T3 P1)
+ *
+ * If a self rule is involved then
+ *   If S1 is an attribute then
+ *     If T1 is self and T2 is not self then
+ *       S3 = S1 and not (S2 and T2)
+ *     else if T1 is not self and T2 is self then
+ *       S3 = S1 and not (T1 and S2)
+ *     else if T1 is self and T2 is self then
+ *       S3 = S1 and not S2
+ *     (allow S3 T1 P1)
+ *   If T1 is an attribute then
+ *     If T2 is self then
+ *       T3 = T1 and not (S1 and S2)
+ *     (allow S1 T3 P1)
+ *     If S1 is attribute and S2 is attribute and T2 is self then
+ *       # In addition to any rules involving S3 and T3 above
+ *       SX = S1 and S2 and T1
+ *       For S in SX do
+ *         T = SX and not S
+ *         (allow S T P1)
+ *
+ * Do not create a rule that has an empty set
+ */
+
+static int cil_perm_match(const struct cil_perm *p1, const struct cil_list *pl2)
+{
+	struct cil_list_item *curr;
+
+	cil_list_for_each(curr, pl2) {
+		struct cil_perm *p = curr->data;
+		if (p == p1) {
+			return CIL_TRUE;
+		}
+	}
+	return CIL_FALSE;
+}
+
+static int cil_class_perm_match(const struct cil_class *c1, const struct cil_perm *p1, const struct cil_list *cpl2)
+{
+	struct cil_list_item *curr;
+
+	cil_list_for_each(curr, cpl2) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = curr->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				if (cp->class == c1) {
+					if (cil_perm_match(p1, cp->perms)) {
+						return CIL_TRUE;
+					}
+				}
+			} else { /* MAP */
+				struct cil_list_item *p;
+				cil_list_for_each(p, cp->perms) {
+					struct cil_perm *cmp = p->data;
+					if (cil_class_perm_match(c1, p1, cmp->classperms)) {
+						return CIL_TRUE;
+					}
+				}
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cp_set = curr->data;
+			struct cil_classpermission *cp = cp_set->set;
+			if (cil_class_perm_match(c1, p1, cp->classperms)) {
+				return CIL_TRUE;
+			}
+		}
+	}
+	return CIL_FALSE;
+}
+
+static int cil_classperms_match(const struct cil_classperms *cp1, const struct cil_list *cpl2)
+{
+	struct cil_list_item *curr;
+
+	cil_list_for_each(curr, cp1->perms) {
+		struct cil_perm *perm = curr->data;
+		if (cil_class_perm_match(cp1->class, perm, cpl2)) {
+			return CIL_TRUE;
+		}
+	}
+	return CIL_FALSE;
+}
+
+int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2)
+{
+	struct cil_list_item *curr;
+
+	if (!cpl1 || !cpl2) {
+		return (!cpl1 && !cpl2) ? CIL_TRUE : CIL_FALSE;
+	}
+
+	cil_list_for_each(curr, cpl1) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = curr->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				if (cil_classperms_match(cp, cpl2)) {
+					return CIL_TRUE;
+				}
+			} else { /* MAP */
+				struct cil_list_item *p;
+				cil_list_for_each(p, cp->perms) {
+					struct cil_perm *cmp = p->data;
+					if (cil_classperms_list_match(cmp->classperms, cpl2)) {
+						return CIL_TRUE;
+					}
+				}
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cp_set = curr->data;
+			struct cil_classpermission *cp = cp_set->set;
+			if (cil_classperms_list_match(cp->classperms, cpl2)) {
+				return CIL_TRUE;
+			}
+		}
+	}
+	return CIL_FALSE;
+}
+
+static void cil_classperms_copy(struct cil_classperms **new, const struct cil_classperms *old)
+{
+	cil_classperms_init(new);
+	(*new)->class_str = old->class_str;
+	(*new)->class = old->class;
+	cil_copy_list(old->perm_strs, &(*new)->perm_strs);
+	cil_copy_list(old->perms, &(*new)->perms);
+}
+
+static void cil_classperms_set_copy(struct cil_classperms_set **new, const struct cil_classperms_set *old)
+{
+	cil_classperms_set_init(new);
+	(*new)->set_str = old->set_str;
+	(*new)->set = old->set;
+}
+
+void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old)
+{
+	struct cil_list_item *curr;
+
+	if (!new) {
+		return;
+	}
+
+	if (!old) {
+		*new = NULL;
+		return;
+	}
+
+	cil_list_init(new, CIL_LIST);
+
+	cil_list_for_each(curr, old) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *new_cp;
+			cil_classperms_copy(&new_cp, curr->data);
+			cil_list_append(*new, CIL_CLASSPERMS, new_cp);
+		} else { /* SET */
+			struct cil_classperms_set *new_cps;
+			cil_classperms_set_copy(&new_cps, curr->data);
+			cil_list_append(*new, CIL_CLASSPERMS_SET, new_cps);
+		}
+	}
+
+	if (cil_list_is_empty(*new)) {
+		cil_list_destroy(new, CIL_FALSE);
+	}
+}
+
+/* Append cp1 and not cpl2 to result */
+static void cil_classperms_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
+{
+	struct cil_classperms *new_cp = NULL;
+	struct cil_list_item *curr;
+
+	if (!cil_classperms_match(cp1, cpl2)) {
+		cil_classperms_copy(&new_cp, cp1);
+		cil_list_append(*result, CIL_CLASSPERMS, new_cp);
+		return;
+	}
+
+	cil_list_for_each(curr, cp1->perms) {
+		struct cil_perm *perm = curr->data;
+		if (!cil_class_perm_match(cp1->class, perm, cpl2)) {
+			if (new_cp == NULL) {
+				cil_classperms_init(&new_cp);
+				new_cp->class_str = cp1->class_str;
+				new_cp->class = cp1->class;
+				cil_list_init(&new_cp->perm_strs, CIL_PERM);
+				cil_list_init(&new_cp->perms, CIL_PERM);
+				cil_list_append(*result, CIL_CLASSPERMS, new_cp);
+			}
+			cil_list_append(new_cp->perm_strs, CIL_STRING, perm->datum.fqn);
+			cil_list_append(new_cp->perms, CIL_DATUM, perm);
+		}
+	}
+}
+
+/* Append cp1 and not cpl2 to result */
+static void cil_classperms_map_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
+{
+	struct cil_classperms *new_cp = NULL;
+	struct cil_list_item *p;
+
+	cil_list_for_each(p, cp1->perms) {
+		struct cil_perm *map_perm = p->data;
+		if (!cil_classperms_list_match(map_perm->classperms, cpl2)) {
+			if (new_cp == NULL) {
+				cil_classperms_init(&new_cp);
+				new_cp->class_str = cp1->class_str;
+				new_cp->class = cp1->class;
+				cil_list_init(&new_cp->perm_strs, CIL_PERM);
+				cil_list_init(&new_cp->perms, CIL_PERM);
+				cil_list_append(*result, CIL_CLASSPERMS, new_cp);
+			}
+			cil_list_append(new_cp->perm_strs, CIL_STRING, map_perm->datum.fqn);
+			cil_list_append(new_cp->perms, CIL_DATUM, map_perm);
+		} else {
+			struct cil_list *new_cpl = NULL;
+			cil_classperms_list_andnot(&new_cpl, map_perm->classperms, cpl2);
+			if (new_cpl) {
+				struct cil_list_item *i;
+				cil_list_for_each(i, new_cpl) {
+					cil_list_append(*result, i->flavor, i->data);
+				}
+				cil_list_destroy(&new_cpl, CIL_FALSE);
+			}
+		}
+	}
+}
+
+/* Append cps1 and not cpl2 to result */
+static void cil_classperms_set_andnot(struct cil_list **result, const struct cil_classperms_set *cps1, const struct cil_list *cpl2)
+{
+	struct cil_classpermission *cp = cps1->set;
+
+	if (!cil_classperms_list_match(cp->classperms, cpl2)) {
+		struct cil_classperms_set *new_cps;
+		cil_classperms_set_copy(&new_cps, cps1);
+		cil_list_append(*result, CIL_CLASSPERMS_SET, new_cps);
+	} else {
+		struct cil_list *new_cpl;
+		cil_classperms_list_andnot(&new_cpl, cp->classperms, cpl2);
+		if (new_cpl) {
+			struct cil_list_item *i;
+			cil_list_for_each(i, new_cpl) {
+				cil_list_append(*result, i->flavor, i->data);
+			}
+			cil_list_destroy(&new_cpl, CIL_FALSE);
+		}
+	}
+}
+
+/* result = cpl1 and not cpl2 */
+void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2)
+{
+	struct cil_list_item *curr;
+
+	if (!result) {
+		return;
+	}
+
+	if (!cpl1) {
+		*result = NULL;
+		return;
+	}
+
+	cil_list_init(result, CIL_LIST);
+
+	if (!cpl2 || !cil_classperms_list_match(cpl1, cpl2)) {
+		cil_classperms_list_copy(result, cpl1);
+		return;
+	}
+
+	cil_list_for_each(curr, cpl1) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = curr->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				cil_classperms_andnot(result, cp, cpl2);
+			} else { /* MAP */
+				cil_classperms_map_andnot(result, cp, cpl2);
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cps = curr->data;
+			cil_classperms_set_andnot(result, cps, cpl2);
+		}
+	}
+
+	if (cil_list_is_empty(*result)) {
+		cil_list_destroy(result, CIL_FALSE);
+	}
+}
+
+/* result = d1 and d2 */
+static int cil_datums_and(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
+{
+	int rc = SEPOL_OK;
+	enum cil_flavor f1 = FLAVOR(d1);
+	enum cil_flavor f2 = FLAVOR(d2);
+
+	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		ebitmap_init(result);
+		if (t1->value == t2->value) {
+			rc = ebitmap_set_bit(result, t1->value, 1);
+			if (rc != SEPOL_OK) {
+				ebitmap_destroy(result);
+				goto exit;
+			}
+		}
+	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		ebitmap_init(result);
+		if (ebitmap_get_bit(a1->types, t2->value)) {
+			rc = ebitmap_set_bit(result, t2->value, 1);
+			if (rc != SEPOL_OK) {
+				ebitmap_destroy(result);
+				goto exit;
+			}
+		}
+	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		ebitmap_init(result);
+		if (ebitmap_get_bit(a2->types, t1->value)) {
+			rc = ebitmap_set_bit(result, t1->value, 1);
+			if (rc != SEPOL_OK) {
+				ebitmap_destroy(result);
+				goto exit;
+			}
+		}
+	} else {
+		/* Both are attributes */
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		rc = ebitmap_and(result, a1->types, a2->types);
+		if (rc != SEPOL_OK) {
+			ebitmap_destroy(result);
+			goto exit;
+		}
+	}
+exit:
+	return rc;
+}
+
+/* result = d1 and not d2 */
+static int cil_datums_andnot(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
+{
+	int rc = SEPOL_OK;
+	enum cil_flavor f1 = FLAVOR(d1);
+	enum cil_flavor f2 = FLAVOR(d2);
+
+	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		ebitmap_init(result);
+		if (t1->value != t2->value) {
+			rc = ebitmap_set_bit(result, t1->value, 1);
+			if (rc != SEPOL_OK) {
+				ebitmap_destroy(result);
+				goto exit;
+			}
+		}
+	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		rc = ebitmap_cpy(result, a1->types);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+		rc = ebitmap_set_bit(result, t2->value, 0);
+		if (rc != SEPOL_OK) {
+			ebitmap_destroy(result);
+			goto exit;
+		}
+	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		ebitmap_init(result);
+		if (!ebitmap_get_bit(a2->types, t1->value)) {
+			rc = ebitmap_set_bit(result, t1->value, 1);
+			if (rc != SEPOL_OK) {
+				ebitmap_destroy(result);
+				goto exit;
+			}
+		}
+	} else {
+		/* Both are attributes */
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		rc = ebitmap_andnot(result, a1->types, a2->types, a1->types->highbit);
+		if (rc != SEPOL_OK) {
+			ebitmap_destroy(result);
+			goto exit;
+		}
+	}
+exit:
+	return rc;
+}
+
+static size_t num_digits(unsigned n)
+{
+	size_t num = 1;
+	while (n >= 10) {
+		n /= 10;
+		num++;
+	}
+	return num;
+}
+
+static char *cil_create_new_attribute_name(unsigned num)
+{
+	char *s1 = NULL;
+	char *s2 = NULL;
+	size_t len_num = num_digits(num);
+	size_t len = strlen(CIL_DENY_ATTR_PREFIX) + 1 + len_num + 1;
+	int rc;
+
+	if (len >= CIL_MAX_NAME_LENGTH) {
+		cil_log(CIL_ERR, "Name length greater than max name length of %d",
+				CIL_MAX_NAME_LENGTH);
+		goto exit;
+	}
+
+	s1 = cil_malloc(len);
+	rc = snprintf(s1, len, "%s_%u", CIL_DENY_ATTR_PREFIX, num);
+	if (rc < 0 || (size_t)rc >= len) {
+		cil_log(CIL_ERR, "Error creating new attribute name");
+		free(s1);
+		goto exit;
+	}
+
+	s2 = cil_strpool_add(s1);
+	free(s1);
+
+exit:
+	return s2;
+}
+
+static struct cil_list *cil_create_and_expr_list(enum cil_flavor flavor, void *v1, void *v2)
+{
+	struct cil_list *expr;
+
+	cil_list_init(&expr, CIL_TYPE);
+	cil_list_append(expr, CIL_OP, (void *)CIL_AND);
+	cil_list_append(expr, flavor, v1);
+	cil_list_append(expr, flavor, v2);
+
+	return expr;
+}
+
+static struct cil_list *cil_create_andnot_expr_list(enum cil_flavor flavor, void *v1, void *v2)
+{
+	struct cil_list *expr, *sub_expr;
+
+	cil_list_init(&expr, CIL_TYPE);
+	cil_list_append(expr, CIL_OP, (void *)CIL_AND);
+	cil_list_append(expr, flavor, v1);
+	cil_list_init(&sub_expr, CIL_TYPE);
+	cil_list_append(sub_expr, CIL_OP, (void *)CIL_NOT);
+	cil_list_append(sub_expr, flavor, v2);
+	cil_list_append(expr, CIL_LIST, sub_expr);
+
+	return expr;
+}
+
+static struct cil_tree_node *cil_create_and_insert_node(struct cil_tree_node *prev, enum cil_flavor flavor, void *data)
+{
+	struct cil_tree_node *new;
+
+	cil_tree_node_init(&new);
+	new->parent = prev->parent;
+	new->line = prev->line;
+	new->hll_offset = prev->hll_offset;
+	new->flavor = flavor;
+	new->data = data;
+	new->next = prev->next;
+	prev->next = new;
+
+	return new;
+}
+
+static int cil_create_and_insert_attribute_and_set(struct cil_db *db, struct cil_tree_node *prev, struct cil_list *str_expr, struct cil_list *datum_expr, ebitmap_t *types, struct cil_symtab_datum **d)
+{
+	struct cil_tree_node *attr_node = NULL;
+	char *name;
+	struct cil_typeattribute *attr = NULL;
+	struct cil_tree_node *attrset_node = NULL;
+	struct cil_typeattributeset *attrset = NULL;
+	symtab_t *symtab = NULL;
+	int rc = SEPOL_ERR;
+
+	name = cil_create_new_attribute_name(db->num_types_and_attrs);
+	if (!name) {
+		goto exit;
+	}
+
+	cil_typeattributeset_init(&attrset);
+	attrset->attr_str = name;
+	attrset->str_expr = str_expr;
+	attrset->datum_expr = datum_expr;
+
+	cil_typeattribute_init(&attr);
+	cil_list_init(&attr->expr_list, CIL_TYPE);
+	cil_list_append(attr->expr_list, CIL_LIST, datum_expr);
+	attr->types = types;
+	attr->used = CIL_ATTR_AVRULE;
+	attr->keep = (ebitmap_cardinality(types) < db->attrs_expand_size) ? CIL_FALSE : CIL_TRUE;
+
+	attr_node = cil_create_and_insert_node(prev, CIL_TYPEATTRIBUTE, attr);
+	attrset_node = cil_create_and_insert_node(attr_node, CIL_TYPEATTRIBUTESET, attrset);
+
+	rc = cil_get_symtab(prev->parent, &symtab, CIL_SYM_TYPES);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	rc = cil_symtab_insert(symtab, name, &attr->datum, attr_node);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	db->num_types_and_attrs++;
+
+	*d = &attr->datum;
+
+	return SEPOL_OK;
+
+exit:
+	if (attrset_node) {
+		prev->next = attrset_node->next;
+		cil_destroy_typeattribute(attr_node->data);
+		free(attr_node);
+		cil_destroy_typeattributeset(attrset_node->data);
+		free(attrset_node);
+	}
+	return rc;
+}
+
+static int cil_create_attribute_d1_and_not_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
+{
+	struct cil_list *str_expr;
+	struct cil_list *datum_expr;
+	ebitmap_t *types;
+	int rc;
+
+	*d3 = NULL;
+
+	if (d1 == d2) {
+		return SEPOL_OK;
+	}
+
+	str_expr = cil_create_andnot_expr_list(CIL_STRING, d1->fqn, d2->fqn);
+	datum_expr = cil_create_andnot_expr_list(CIL_DATUM, d1, d2);
+
+	types = cil_malloc(sizeof(*types));
+	rc = cil_datums_andnot(types, d1, d2);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+	if (ebitmap_is_empty(types)) {
+		rc = SEPOL_OK;
+		goto exit;
+	}
+
+	if (ebitmap_cardinality(types) == 1) {
+		unsigned i = ebitmap_highest_set_bit(types);
+		*d3 = DATUM(db->val_to_type[i]);
+		ebitmap_destroy(types);
+		rc = SEPOL_OK;
+		goto exit;
+	}
+
+	return cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
+
+exit:
+	cil_list_destroy(&str_expr, CIL_FALSE);
+	cil_list_destroy(&datum_expr, CIL_FALSE);
+	free(types);
+	return rc;
+}
+
+static int cil_create_attribute_d1_and_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
+{
+	struct cil_list *str_expr;
+	struct cil_list *datum_expr;
+	ebitmap_t *types;
+	int rc;
+
+	if (d1 == d2) {
+		*d3 = d1;
+		return SEPOL_OK;
+	}
+
+	*d3 = NULL;
+
+	str_expr = cil_create_and_expr_list(CIL_STRING, d1->fqn, d2->fqn);
+	datum_expr = cil_create_and_expr_list(CIL_DATUM, d1, d2);
+
+	types = cil_malloc(sizeof(*types));
+	rc = cil_datums_and(types, d1, d2);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+	if (ebitmap_is_empty(types)) {
+		rc = SEPOL_OK;
+		goto exit;
+	}
+
+	if (ebitmap_cardinality(types) == 1) {
+		unsigned i = ebitmap_highest_set_bit(types);
+		*d3 = DATUM(db->val_to_type[i]);
+		ebitmap_destroy(types);
+		rc = SEPOL_OK;
+		goto exit;
+	}
+
+	rc = cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	return SEPOL_OK;
+
+exit:
+	cil_list_destroy(&str_expr, CIL_FALSE);
+	cil_list_destroy(&datum_expr, CIL_FALSE);
+	free(types);
+	return rc;
+}
+
+static struct cil_avrule *cil_create_avrule(struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
+{
+	struct cil_avrule *new;
+
+	cil_avrule_init(&new);
+	new->is_extended = CIL_FALSE;
+	new->rule_kind = CIL_AVRULE_ALLOWED;
+	new->src_str = src->name;
+	new->src = src;
+	new->tgt_str = tgt->name;
+	new->tgt = tgt;
+	new->perms.classperms = classperms;
+
+	return new;
+}
+
+static struct cil_tree_node *cil_create_and_add_avrule(struct cil_tree_node *curr, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
+{
+	struct cil_avrule *new_avrule;
+
+	new_avrule = cil_create_avrule(src, tgt, classperms);
+	return cil_create_and_insert_node(curr, CIL_AVRULE, new_avrule);
+}
+
+static int cil_remove_permissions_from_rule(struct cil_db *db, struct cil_tree_node *allow_node, const struct cil_tree_node *deny_node)
+{
+	struct cil_avrule *allow_rule = allow_node->data;
+	struct cil_deny_rule *deny_rule = deny_node->data;
+	struct cil_symtab_datum *s1 = allow_rule->src;
+	struct cil_symtab_datum *t1 = allow_rule->tgt;
+	struct cil_list *p1 = allow_rule->perms.classperms;
+	struct cil_symtab_datum *s2 = deny_rule->src;
+	struct cil_symtab_datum *t2 = deny_rule->tgt;
+	struct cil_list *p2 = deny_rule->classperms;
+	struct cil_list *p3 = NULL;
+	struct cil_tree_node *curr = allow_node;
+	int rc;
+
+	cil_classperms_list_andnot(&p3, p1, p2);
+
+	if (!cil_list_is_empty(p3)) {;
+		curr = cil_create_and_add_avrule(curr, s1, t1, p3);
+	} else {
+		cil_destroy_classperms_list(&p3);
+		p3 = NULL;
+	}
+
+	if (FLAVOR(s1) == CIL_TYPEATTRIBUTE) {
+		struct cil_symtab_datum *s3 = NULL;
+		struct cil_symtab_datum *d = NULL;
+		struct cil_list *cp = NULL;
+		if (t1->fqn == CIL_KEY_SELF && t2->fqn != CIL_KEY_SELF) {
+			/* S3 = S1 and not (S2 and T2) */
+			rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t2, &d);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+			if (d) {
+				rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
+				if (rc != SEPOL_OK) {
+					goto exit;
+				}
+			}
+		} else if (t1->fqn != CIL_KEY_SELF && t2->fqn == CIL_KEY_SELF) {
+			/* S3 = S1 and not (T1 and S2) */
+			rc = cil_create_attribute_d1_and_d2(db, NODE(t1), t1, s2, &d);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+			if (d) {
+				rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
+				if (rc != SEPOL_OK) {
+					goto exit;
+				}
+			}
+		} else {
+			/* S3 = S1 and not S2 */
+			rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, s2, &s3);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+		}
+		if (s3) {
+			cil_classperms_list_copy(&cp, p1);
+			curr = cil_create_and_add_avrule(curr, s3, t1, cp);
+		}
+	}
+
+	if (t1->fqn != CIL_KEY_SELF && FLAVOR(t1) == CIL_TYPEATTRIBUTE) {
+		struct cil_symtab_datum *t3 = NULL;
+		struct cil_symtab_datum *d = NULL;
+		struct cil_list *cp = NULL;
+		if (t2->fqn == CIL_KEY_SELF) {
+			/* T3 = T1 and not (S1 AND S2) */
+			rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, s2, &d);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+			if (d) {
+				rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, d, &t3);
+				if (rc != SEPOL_OK) {
+					goto exit;
+				}
+			}
+		} else {
+			/* S3 = T1 and not T2 */
+			rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, t2, &t3);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+		}
+		if (t3) {
+			cil_classperms_list_copy(&cp, p1);
+			curr = cil_create_and_add_avrule(curr, s1, t3, cp);
+		}
+
+		if (FLAVOR(s1) == CIL_TYPEATTRIBUTE && FLAVOR(s2) == CIL_TYPEATTRIBUTE &&
+			t2->fqn == CIL_KEY_SELF) {
+			/* Really special case */
+			/* FLAVOR(t1) == CIL_TYPEATTRIBUTE */
+			/* SX = S1 and S2 and T1 */
+			struct cil_symtab_datum *sx = NULL;
+			struct cil_typeattribute *sx_attr = NULL;
+			
+			d = NULL;
+			cp = NULL;
+
+			rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t1, &d);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+			if (d) {
+				rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, d, &sx);
+				if (rc != SEPOL_OK) {
+					goto exit;
+				}
+				sx_attr = (struct cil_typeattribute *)sx;
+			}
+			if (sx && ebitmap_cardinality(sx_attr->types) > 1) {
+				struct cil_symtab_datum *s = NULL;
+				struct cil_symtab_datum *t = NULL;
+				ebitmap_node_t *n;
+				unsigned i;
+				ebitmap_for_each_positive_bit(sx_attr->types, n, i) {
+					s = DATUM(db->val_to_type[i]);
+					rc = cil_create_attribute_d1_and_not_d2(db, NODE(sx), sx, s, &t);
+					if (rc != SEPOL_OK) {
+						goto exit;
+					}
+					if (t) {
+						cil_classperms_list_copy(&cp, p1);
+						curr = cil_create_and_add_avrule(curr, s, t, cp);
+					}
+				}
+			}
+		}
+	}
+
+exit:
+	return rc;
+}
+
+static int cil_find_matching_allow_rules(struct cil_list *matching, struct cil_tree_node *start, struct cil_tree_node *deny_node)
+{
+	struct cil_deny_rule *deny_rule = deny_node->data;
+	struct cil_avrule target;
+
+	target.rule_kind = CIL_AVRULE_ALLOWED;
+	target.is_extended = CIL_FALSE;
+	target.src = deny_rule->src;
+	target.tgt = deny_rule->tgt;
+	target.perms.classperms = deny_rule->classperms;
+
+	return cil_find_matching_avrule_in_ast(start, CIL_AVRULE, &target, matching, CIL_FALSE);
+}
+
+static int cil_process_deny_rule(struct cil_db *db, struct cil_tree_node *start, struct cil_tree_node *deny_node)
+{
+	struct cil_list *matching;
+	struct cil_list_item *item;
+	int rc;
+
+	cil_list_init(&matching, CIL_NODE);
+
+	rc = cil_find_matching_allow_rules(matching, start, deny_node);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	cil_list_for_each(item, matching) {
+		struct cil_tree_node *allow_node = item->data;
+		rc = cil_remove_permissions_from_rule(db, allow_node, deny_node);
+		cil_tree_remove_node(allow_node);
+		cil_tree_node_destroy(&allow_node);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+
+	}
+
+exit:
+	cil_list_destroy(&matching, CIL_FALSE);
+	return rc;
+}
+
+static int cil_process_deny_rules(struct cil_db *db, struct cil_tree_node *start, struct cil_list *deny_rules)
+{
+	struct cil_list_item *item;
+	int rc;
+
+	cil_list_for_each(item, deny_rules) {
+		struct cil_tree_node *deny_node = item->data;
+		rc = cil_process_deny_rule(db, start, deny_node);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+		cil_tree_remove_node(deny_node);
+		cil_tree_node_destroy(&deny_node);
+	}
+
+exit:
+	return rc;
+}
+
+static int __cil_find_deny_rules(struct cil_tree_node *node,  uint32_t *finished, void *extra_args)
+{
+	struct cil_list *deny_rules = extra_args;
+
+	if (node->flavor == CIL_BLOCK) {
+		struct cil_block *block = node->data;
+		if (block->is_abstract == CIL_TRUE) {
+			*finished = CIL_TREE_SKIP_HEAD;
+		}
+	} else if (node->flavor == CIL_MACRO) {
+		*finished = CIL_TREE_SKIP_HEAD;
+	} else if (node->flavor == CIL_DENY_RULE) {
+		cil_list_append(deny_rules, CIL_DENY_RULE, node);
+	}
+	return SEPOL_OK;
+}
+
+int cil_process_deny_rules_in_ast(struct cil_db *db)
+{
+	struct cil_tree_node *start;
+	struct cil_list *deny_rules;
+	int rc = SEPOL_ERR;
+
+	cil_list_init(&deny_rules, CIL_DENY_RULE);
+
+	if (!db) {
+		cil_log(CIL_ERR, "No CIL db provided to process deny rules\n");
+		goto exit;
+	}
+
+	start = db->ast->root;
+	rc = cil_tree_walk(start, __cil_find_deny_rules, NULL, NULL, deny_rules);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "An error occurred while getting deny rules\n");
+		goto exit;
+	}
+
+	rc = cil_process_deny_rules(db, start, deny_rules);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "An error occurred while processing deny rules\n");
+		goto exit;
+	}
+
+exit:
+	cil_list_destroy(&deny_rules, CIL_FALSE);
+	return rc;
+}
diff --git a/libsepol/cil/src/cil_deny.h b/libsepol/cil/src/cil_deny.h
new file mode 100644
index 00000000..4a9e92d1
--- /dev/null
+++ b/libsepol/cil/src/cil_deny.h
@@ -0,0 +1,34 @@
+/*
+ * This file is public domain software, i.e. not copyrighted.
+ *
+ * Warranty Exclusion
+ * ------------------
+ * You agree that this software is a non-commercially developed program
+ * that may contain "bugs" (as that term is used in the industry) and
+ * that it may not function as intended. The software is licensed
+ * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
+ * express, implied, statutory, or otherwise with respect to the software,
+ * including noninfringement and the implied warranties of merchantability
+ * and fitness for a particular purpose.
+ *
+ * Limitation of Liability
+ *-----------------------
+ * In no event will NSA be liable for any damages, including loss of data,
+ * lost profits, cost of cover, or other special, incidental, consequential,
+ * direct or indirect damages arising from the software or the use thereof,
+ * however caused and on any theory of liability. This limitation will apply
+ * even if NSA has been advised of the possibility of such damage. You
+ * acknowledge that this is a reasonable allocation of risk.
+ *
+ * Original author: James Carter
+ */
+
+#ifndef CIL_DENY_H_
+#define CIL_DENY_H_
+
+int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2);
+void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old);
+void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2);
+int cil_process_deny_rules_in_ast(struct cil_db *db);
+
+#endif /* CIL_DENY_H_ */
diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index 11e572e2..fb7eec2d 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -47,6 +47,7 @@
 #include "cil_policy.h"
 #include "cil_verify.h"
 #include "cil_symtab.h"
+#include "cil_deny.h"
 
 #define GEN_REQUIRE_ATTR "cil_gen_require" /* Also in libsepol/src/module_to_cil.c */
 #define TYPEATTR_INFIX "_typeattr_"        /* Also in libsepol/src/module_to_cil.c */
@@ -2551,6 +2552,12 @@ int cil_post_process(struct cil_db *db)
 		goto exit;
 	}
 
+	rc = cil_process_deny_rules_in_ast(db);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "Failed to process deny rules\n");
+		goto exit;
+	}
+
 	rc = cil_post_verify(db);
 	if (rc != SEPOL_OK) {
 		cil_log(CIL_ERR, "Failed to verify cil database\n");
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 5/9] libsepol/cil: Add cil_write_post_ast function
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (3 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 4/9] libsepol/cil: Process deny rules James Carter
@ 2022-12-15 21:34 ` James Carter
  2022-12-15 21:34 ` [RFC PATCH 6/9] libsepol: Export the " James Carter
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

The function cil_write_post_ast() will write the CIL AST after
post processing is done. Most post processing does not change the
CIL AST, this is where deny rules are processed (because to process
them, type attributes have to have been evaluated.)

When processed, deny rules may add new rules and attributes and the
deny rule itself will be removed from the AST, so using this new
function will show the results of the deny rule processing.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/include/cil/cil.h   |  1 +
 libsepol/cil/src/cil.c           | 50 ++++++++++++++++++++++++++++++++
 libsepol/cil/src/cil_write_ast.h |  1 +
 3 files changed, 52 insertions(+)

diff --git a/libsepol/cil/include/cil/cil.h b/libsepol/cil/include/cil/cil.h
index 482ca522..88e47e79 100644
--- a/libsepol/cil/include/cil/cil.h
+++ b/libsepol/cil/include/cil/cil.h
@@ -64,6 +64,7 @@ extern void cil_write_policy_conf(FILE *out, struct cil_db *db);
 extern int cil_write_parse_ast(FILE *out, cil_db_t *db);
 extern int cil_write_build_ast(FILE *out, cil_db_t *db);
 extern int cil_write_resolve_ast(FILE *out, cil_db_t *db);
+extern int cil_write_post_ast(FILE *out, cil_db_t *db);
 
 enum cil_log_level {
 	CIL_ERR = 1,
diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 0b35ad35..bfeccb0e 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -675,6 +675,56 @@ exit:
 	return rc;
 }
 
+int cil_write_post_ast(FILE *out, cil_db_t *db)
+{
+	int rc = SEPOL_ERR;
+
+	if (db == NULL) {
+		goto exit;
+	}
+
+	cil_log(CIL_INFO, "Building AST from Parse Tree\n");
+	rc = cil_build_ast(db, db->parse->root, db->ast->root);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "Failed to build ast\n");
+		goto exit;
+	}
+
+	cil_log(CIL_INFO, "Destroying Parse Tree\n");
+	cil_tree_destroy(&db->parse);
+
+	cil_log(CIL_INFO, "Resolving AST\n");
+	rc = cil_resolve_ast(db, db->ast->root);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "Failed to resolve ast\n");
+		goto exit;
+	}
+
+	cil_log(CIL_INFO, "Qualifying Names\n");
+	rc = cil_fqn_qualify(db->ast->root);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "Failed to qualify names\n");
+		goto exit;
+	}
+
+	cil_log(CIL_INFO, "Compile post process\n");
+	rc = cil_post_process(db);
+	if (rc != SEPOL_OK ) {
+		cil_log(CIL_ERR, "Post process failed\n");
+		goto exit;
+	}
+
+	cil_log(CIL_INFO, "Writing Post AST\n");
+	rc = cil_write_ast(out, CIL_WRITE_AST_PHASE_POST, db->ast->root);
+	if (rc != SEPOL_OK) {
+		cil_log(CIL_ERR, "Failed to write post ast\n");
+		goto exit;
+	}
+
+exit:
+	return rc;
+}
+
 int cil_build_policydb(cil_db_t *db, sepol_policydb_t **sepol_db)
 {
 	int rc;
diff --git a/libsepol/cil/src/cil_write_ast.h b/libsepol/cil/src/cil_write_ast.h
index 3f4b9d95..6b3274a8 100644
--- a/libsepol/cil/src/cil_write_ast.h
+++ b/libsepol/cil/src/cil_write_ast.h
@@ -38,6 +38,7 @@ enum cil_write_ast_phase {
 	CIL_WRITE_AST_PHASE_PARSE = 0,
 	CIL_WRITE_AST_PHASE_BUILD,
 	CIL_WRITE_AST_PHASE_RESOLVE,
+	CIL_WRITE_AST_PHASE_POST,
 };
 
 void cil_write_ast_node(FILE *out, struct cil_tree_node *node);
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 6/9] libsepol: Export the cil_write_post_ast function
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (4 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 5/9] libsepol/cil: Add cil_write_post_ast function James Carter
@ 2022-12-15 21:34 ` James Carter
  2022-12-15 21:34 ` [RFC PATCH 7/9] secilc/secil2tree: Add option to write CIL AST after post processing James Carter
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/src/libsepol.map.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libsepol/src/libsepol.map.in b/libsepol/src/libsepol.map.in
index 844924fc..6ad68f0b 100644
--- a/libsepol/src/libsepol.map.in
+++ b/libsepol/src/libsepol.map.in
@@ -289,3 +289,8 @@ LIBSEPOL_3.4 {
 	sepol_string_to_security_class;
 	sepol_validate_transition_reason_buffer;
 } LIBSEPOL_3.0;
+
+LIBSEPOL_3.5 {
+  global:
+	cil_write_post_ast;
+} LIBSEPOL_3.4;
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 7/9] secilc/secil2tree: Add option to write CIL AST after post processing
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (5 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 6/9] libsepol: Export the " James Carter
@ 2022-12-15 21:34 ` James Carter
  2022-12-15 21:34 ` [RFC PATCH 8/9] secilc/test: Add a deny rule test James Carter
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

This will show the resulting CIL AST after deny rules have been
processed.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 secilc/secil2tree.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/secilc/secil2tree.c b/secilc/secil2tree.c
index e5cdf6bd..ff0fc92b 100644
--- a/secilc/secil2tree.c
+++ b/secilc/secil2tree.c
@@ -45,6 +45,7 @@ enum write_ast_phase {
 	WRITE_AST_PHASE_PARSE = 0,
 	WRITE_AST_PHASE_BUILD,
 	WRITE_AST_PHASE_RESOLVE,
+	WRITE_AST_PHASE_POST,
 };
 
 static __attribute__((__noreturn__)) void usage(const char *prog)
@@ -58,7 +59,7 @@ static __attribute__((__noreturn__)) void usage(const char *prog)
 	printf("                           Blocks, blockinherits, blockabstracts, and\n");
 	printf("                           in-statements will not be allowed.\n");
 	printf("  -A, --ast-phase=<phase>  write AST of phase <phase>. Phase must be parse, \n");
-	printf("                           build, or resolve. (default: resolve)\n");
+	printf("                           build, resolve, or post. (default: resolve)\n");
 	printf("  -v, --verbose            increment verbosity level\n");
 	printf("  -h, --help               display usage information\n");
 	exit(1);
@@ -115,6 +116,8 @@ int main(int argc, char *argv[])
 					write_ast = WRITE_AST_PHASE_BUILD;
 				} else if (!strcasecmp(optarg, "resolve")) {
 					write_ast = WRITE_AST_PHASE_RESOLVE;
+				} else if (!strcasecmp(optarg, "post")) {
+					write_ast = WRITE_AST_PHASE_POST;
 				} else {
 					fprintf(stderr, "Invalid AST phase: %s\n", optarg);
 					usage(argv[0]);
@@ -197,6 +200,9 @@ int main(int argc, char *argv[])
 	case WRITE_AST_PHASE_RESOLVE:
 		rc = cil_write_resolve_ast(file, db);
 		break;
+	case WRITE_AST_PHASE_POST:
+		rc = cil_write_post_ast(file, db);
+		break;
 	}
 
 	if (rc != SEPOL_OK) {
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 8/9] secilc/test: Add a deny rule test
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (6 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 7/9] secilc/secil2tree: Add option to write CIL AST after post processing James Carter
@ 2022-12-15 21:34 ` James Carter
  2023-02-03 22:54   ` Daniel Burgener
  2022-12-15 21:34 ` [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation James Carter
  2022-12-16 18:51 ` [RFC PATCH 0/9] Add CIL Deny Rule Daniel Burgener
  9 siblings, 1 reply; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 secilc/test/deny_rule_test.cil | 384 +++++++++++++++++++++++++++++++++
 1 file changed, 384 insertions(+)
 create mode 100644 secilc/test/deny_rule_test.cil

diff --git a/secilc/test/deny_rule_test.cil b/secilc/test/deny_rule_test.cil
new file mode 100644
index 00000000..3ef4ac98
--- /dev/null
+++ b/secilc/test/deny_rule_test.cil
@@ -0,0 +1,384 @@
+(class CLASS (PERM))
+(class C1 (p1a p1b p1c p1d))
+(class C2 (p2a p2b p2c p2d))
+(class C3 (p3a p3b p3c p3d))
+(class C4 (p4a p4b p4c p4d))
+(classorder (CLASS C1 C2 C3 C4))
+(sid SID)
+(sidorder (SID))
+(user USER)
+(role ROLE)
+(type TYPE)
+(category CAT)
+(categoryorder (CAT))
+(sensitivity SENS)
+(sensitivityorder (SENS))
+(sensitivitycategory SENS (CAT))
+(allow TYPE self (CLASS (PERM)))
+(roletype ROLE TYPE)
+(userrole USER ROLE)
+(userlevel USER (SENS))
+(userrange USER ((SENS)(SENS (CAT))))
+(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
+
+(classmap cm5 (mp5a mp5b))
+(classmapping cm5 mp5a
+	      (C2 (p2a p2b)))
+(classmapping cm5 mp5b
+	      (C2 (p2c p2d)))
+
+(classpermission cp6)
+(classpermissionset cp6 (C3 (p3a p3b)))
+(classpermissionset cp6 (C4 (p4a p4b)))
+
+(classpermission cp7)
+(classpermissionset cp7 (C2 (p2a p2b)))
+(classpermissionset cp7 (C2 (p2c p2d)))
+
+; Test 1
+(type t01a)
+(type t01b)
+(allow t01a t01b (C1 (p1a)))
+(deny t01a t01b (C1 (p1a)))
+(neverallow t01a t01b (C1 (p1a)))
+
+; Test 2
+(type t02a)
+(type t02b)
+(allow t02a t02b (C1 (p1a p1b)))
+(deny t02a t02b (C1 (p1a)))
+(neverallow t02a t02b (C1 (p1a)))
+; (neverallow t02a t02b (C1 (p1b))) ; This check should fail
+
+; Test 3
+(type t03a)
+(type t03b)
+(allow t03a t03b (C1 (p1a)))
+(deny t03a t03b (C1 (p1a p1b)))
+(neverallow t03a t03b (C1 (p1a p1b)))
+
+
+; Test 11
+(type t11a)
+(type t11b)
+(type t11c)
+(type t11d)
+(typeattribute a11a)
+(typeattribute a11b)
+(typeattributeset a11a (t11a t11b))
+(typeattributeset a11b (t11c t11d))
+(allow a11a a11b (C1 (p1a)))
+(deny a11a a11b (C1 (p1a)))
+(neverallow a11a a11b (C1 (p1a)))
+
+; Test 12
+(type t12a)
+(type t12b)
+(type t12c)
+(type t12d)
+(typeattribute a12a)
+(typeattribute a12b)
+(typeattributeset a12a (t12a t12b))
+(typeattributeset a12b (t12c t12d))
+(allow t12a t12c (C1 (p1a)))
+(deny a12a a12b (C1 (p1a)))
+(neverallow a12a a12b (C1 (p1a)))
+
+; Test 13
+(type t13a)
+(type t13b)
+(type t13c)
+(type t13d)
+(typeattribute a13a)
+(typeattribute a13b)
+(typeattributeset a13a (t13a t13b))
+(typeattributeset a13b (t13c t13d))
+(allow a13a a13b (C1 (p1a)))
+(deny t13a t13c (C1 (p1a)))
+(neverallow t13a t13c (C1 (p1a)))
+; (neverallow t13b t13d (C1 (p1a))) ; This check should fail
+
+; Test 21
+(type t21a)
+(type t21b)
+(allow t21a t21b (cm5 (mp5a)))
+(deny t21a t21b (cm5 (mp5a)))
+(neverallow t21a t21b (cm5 (mp5a)))
+
+; Test 22
+(type t22a)
+(type t22b)
+(allow t22a t22b (cm5 (mp5a mp5b)))
+(deny t22a t22b (cm5 (mp5a)))
+(neverallow t22a t22b (cm5 (mp5a)))
+; (neverallow t22a t22b (cm5 (mp5b))) ; This check should fail
+
+; Test 23
+(type t23a)
+(type t23b)
+(allow t23a t23b (cm5 (mp5a)))
+(deny t23a t23b (cm5 (mp5a mp5b)))
+(neverallow t23a t23b (cm5 (mp5a mp5b)))
+
+; Test 24
+(type t24a)
+(type t24b)
+(allow t24a t24b (C2 (p2a)))
+(deny t24a t24b (cm5 (mp5a)))
+(neverallow t24a t24b (cm5 (mp5a)))
+
+; Test 25
+(type t25a)
+(type t25b)
+(allow t25a t25b (cm5 (mp5a)))
+(deny t25a t25b (C2 (p2a)))
+(neverallow t25a t25b (C2 (p2a)))
+; (neverallow t25a t25b (C2 (p2b))) ; This check should fail
+
+; Test 31
+(type t31a)
+(type t31b)
+(allow t31a t31b cp6)
+(deny t31a t31b cp6)
+(neverallow t31a t31b cp6)
+
+; Test 32
+(type t32a)
+(type t32b)
+(allow t32a t32b cp6)
+(deny t32a t32b (C3 (p3a p3b)))
+(neverallow t32a t32b (C3 (p3a p3b)))
+; (neverallow t32a t32b (C4 (p4a p4b))) ; This check should fail
+
+; Test 33
+(type t33a)
+(type t33b)
+(allow t33a t33b (C3 (p3a)))
+(deny t33a t33b cp6)
+(neverallow t33a t33b cp6)
+
+; Test 34
+(type t34a)
+(type t34b)
+(allow t34a t34b cp7)
+(deny t34a t34b (cm5 (mp5a mp5b)))
+(neverallow t34a t34b (cm5 (mp5a mp5b)))
+
+; Test 35
+(type t35a)
+(type t35b)
+(allow t35a t35b (cm5 (mp5a mp5b)))
+(deny t35a t35b cp7)
+(neverallow t35a t35b cp7)
+
+; Test 41
+(type t41a)
+(allow t41a self (C1 (p1a)))
+(deny t41a self (C1 (p1a)))
+(neverallow t41a self (C1 (p1a)))
+
+; Test 42
+(type t42a)
+(allow t42a self (C1 (p1a)))
+(deny t42a t42a (C1 (p1a)))
+(neverallow t42a t42a (C1 (p1a)))
+
+; Test 43
+(type t43a)
+(allow t43a t43a (C1 (p1a)))
+(deny t43a self (C1 (p1a)))
+(neverallow t43a self (C1 (p1a)))
+
+; Test 51
+(type t51a)
+(type t51b)
+(typeattribute a51a)
+(typeattributeset a51a (t51a t51b))
+(allow a51a self (C1 (p1a)))
+(deny a51a self (C1 (p1a)))
+(neverallow a51a self (C1 (p1a)))
+
+; Test 52
+(type t52a)
+(type t52b)
+(typeattribute a52a)
+(typeattributeset a52a (t52a t52b))
+(allow t52a self (C1 (p1a)))
+(deny a52a self (C1 (p1a)))
+(neverallow a52a self (C1 (p1a)))
+
+; Test 53
+(type t53a)
+(type t53b)
+(typeattribute a53a)
+(typeattributeset a53a (t53a t53b))
+(allow a53a self (C1 (p1a)))
+(deny t53a self (C1 (p1a)))
+(neverallow t53a self (C1 (p1a)))
+; (neverallow t53b self (C1 (p1a))) ; This check should fail
+
+; Test 54
+(type t54a)
+(type t54b)
+(typeattribute a54a)
+(typeattributeset a54a (t54a t54b))
+(allow a54a self (C1 (p1a)))
+(deny a54a a54a (C1 (p1a)))
+(neverallow a54a a54a (C1 (p1a)))
+
+; Test 55
+(type t55a)
+(type t55b)
+(typeattribute a55a)
+(typeattributeset a55a (t55a t55b))
+(allow a55a a55a (C1 (p1a)))
+(deny a55a self (C1 (p1a)))
+(neverallow a55a self (C1 (p1a)))
+; (neverallow t55a t55b (C1 (p1a))) ; This check should fail
+; (neverallow t55b t55a (C1 (p1a))) ; This check should fail
+
+; Test 61
+(type t61a)
+(type t61b)
+(type t61c)
+(type t61d)
+(type t61e)
+(type t61f)
+(type t61g)
+(type t61h)
+(type t61i)
+(type t61j)
+(type t61k)
+(type t61l)
+(type t61m)
+(type t61n)
+(type t61o)
+(type t61p)
+(type t61q)
+(type t61r)
+(type t61s)
+(type t61t)
+(typeattribute a61a)
+(typeattribute a61b)
+(typeattribute a61c)
+(typeattribute a61x)
+(typeattribute a61s)
+(typeattribute a61t)
+(typeattributeset a61a (t61a t61b t61c t61d t61e t61f t61g t61h t61o t61p))
+(typeattributeset a61b (t61a t61b t61c t61d t61i t61j t61k t61l t61q t61r))
+(typeattributeset a61c (t61a t61b t61e t61f t61i t61j t61m t61n t61s t61t))
+(typeattributeset a61x (t61a t61b))
+(typeattributeset a61s (t61c t61d t61e t61f t61g t61h t61o t61p))
+(typeattributeset a61t (t61c t61d t61i t61j t61k t61l t61q t61r))
+(allow a61a a61b (C1 (p1a)))
+(deny a61c self (C1 (p1a)))
+(neverallow a61c self (C1 (p1a)))
+; (neverallow a61s a61b (C1 (p1a))) ; This check should fail
+; (neverallow a61a a61t (C1 (p1a))) ; This check should fail
+; (neverallow t61a t61b (C1 (p1a))) ; This check should fail
+; (neverallow t61b t61a (C1 (p1a))) ; This check should fail
+
+; Test 62
+(type t62a)
+(type t62b)
+(type t62c)
+(type t62d)
+(type t62e)
+(type t62f)
+(type t62g)
+(type t62h)
+(type t62i)
+(type t62j)
+(type t62k)
+(type t62l)
+(type t62m)
+(type t62n)
+(type t62o)
+(type t62p)
+(type t62s)
+(type t62t)
+(type t62u)
+(type t62v)
+(typeattribute a62a)
+(typeattribute a62c)
+(typeattribute a62d)
+(typeattribute a62s)
+(typeattributeset a62a (t62a t62b t62c t62d t62e t62f t62g t62h t62o t62p))
+(typeattributeset a62c (t62a t62b t62e t62f t62i t62j t62m t62n t62s t62t))
+(typeattributeset a62d (t62a t62b t62g t62h t62k t62l t62m t62n t62u t62v))
+(typeattributeset a62s (t62c t62d t62e t62f t62g t62h t62o t62p))
+(allow a62a self (C1 (p1a)))
+(deny a62c a62d (C1 (p1a)))
+(neverallow a62c a62d (C1 (p1a)))
+; (neverallow a62s self (C1 (p1a))) ; This check should fail
+
+; Test 63
+(type t63a)
+(type t63b)
+(type t63c)
+(type t63d)
+(type t63e)
+(type t63f)
+(type t63g)
+(type t63h)
+(type t63i)
+(type t63j)
+(type t63k)
+(type t63l)
+(type t63m)
+(type t63n)
+(type t63o)
+(type t63p)
+(type t63s)
+(type t63t)
+(typeattribute a63a)
+(typeattribute a63c)
+(typeattribute a63s)
+(typeattributeset a63a (t63a t63b t63c t63d t63e t63f t63g t63h t63o t63p))
+(typeattributeset a63c (t63a t63b t63e t63f t63i t63j t63m t63n t63s t63t))
+(typeattributeset a63s (t62c t62d t62e t62f t62g t62h t62o t62p))
+(allow a63a self (C1 (p1a)))
+(deny a63c self (C1 (p1a)))
+(neverallow a63c self (C1 (p1a)))
+; (neverallow a63s self (C1 (p1a))) ; This check should fail
+
+; Test 64
+(type t64a)
+(type t64b)
+(type t64c)
+(type t64d)
+(type t64e)
+(type t64f)
+(type t64g)
+(type t64h)
+(type t64i)
+(type t64j)
+(type t64k)
+(type t64l)
+(type t64m)
+(type t64n)
+(type t64o)
+(type t64p)
+(type t64q)
+(type t64r)
+(type t64s)
+(type t64t)
+(type t64u)
+(type t64v)
+(typeattribute a64a)
+(typeattribute a64b)
+(typeattribute a64c)
+(typeattribute a64d)
+(typeattribute a64s)
+(typeattribute a64t)
+(typeattributeset a64a (t64a t64b t64c t64d t64e t64f t64g t64h t64o t64p))
+(typeattributeset a64b (t64a t64b t64c t64d t64i t64j t64k t64l t64q t64r))
+(typeattributeset a64c (t64a t64b t64e t64f t64i t64j t64m t64n t64s t64t))
+(typeattributeset a64d (t64a t64b t64g t64h t64k t64l t64m t64n t64u t64v))
+(typeattributeset a64s (t64c t64d t64g t64h t64o t64p))
+(typeattributeset a64t (t64c t64d t64i t64j t64q t64r))
+(allow a64a a64b (C1 (p1a)))
+(deny a64c a64d (C1 (p1a)))
+(neverallow a64c a64d (C1 (p1a)))
+; (neverallow a64s a64b (C1 (p1a))) ; This check should fail
+; (neverallow a64a a64t (C1 (p1a))) ; This check should fail
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (7 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 8/9] secilc/test: Add a deny rule test James Carter
@ 2022-12-15 21:34 ` James Carter
  2023-02-03 22:55   ` Daniel Burgener
  2022-12-16 18:51 ` [RFC PATCH 0/9] Add CIL Deny Rule Daniel Burgener
  9 siblings, 1 reply; 20+ messages in thread
From: James Carter @ 2022-12-15 21:34 UTC (permalink / raw)
  To: selinux; +Cc: dburgener, James Carter

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 secilc/docs/cil_access_vector_rules.md | 68 ++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
index f0ba4a90..35825283 100644
--- a/secilc/docs/cil_access_vector_rules.md
+++ b/secilc/docs/cil_access_vector_rules.md
@@ -247,6 +247,74 @@ This example will not compile as `type_3` is not allowed to be a source type for
         (allow type_3 self (property_service (set)))
     )
 ```
+deny
+----------
+
+Remove the access rights defined from any matching allow rules. These rules are processed before [`neverallow`](cil_access_vector_rules.md#neverallow) checking.
+
+**Rule definition:**
+
+```secil
+    (deny source_id target_id|self classpermissionset_id ...)
+```
+
+**Where:**
+
+<table>
+<colgroup>
+<col width="27%" />
+<col width="72%" />
+</colgroup>
+<tbody>
+<tr class="odd">
+<td align="left"><p><code>deny</code></p></td>
+<td align="left"><p>The <code>deny</code> keyword.</p></td>
+</tr>
+<tr class="even">
+<td align="left"><p><code>source_id</code></p></td>
+<td align="left"><p>A single previously defined source <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p></td>
+</tr>
+<tr class="odd">
+<td align="left"><p><code>target_id</code></p></td>
+<td align="left"><p>A single previously defined target <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p>
+<p>The <code>self</code> keyword may be used instead to signify that source and target are the same.</p></td>
+</tr>
+<tr class="even">
+<td align="left"><p><code>classpermissionset_id</code></p></td>
+<td align="left"><p>A single named or anonymous <code>classpermissionset</code> or a single set of <code>classmap</code>/<code>classmapping</code> identifiers.</p></td>
+</tr>
+</tbody>
+</table>
+
+**Example:**
+
+```secil
+    (class class1 (perm1 perm2))
+
+	(type type_1)
+    (type type_2)
+	(allow type_1 type_2 (class1 (perm1))) ; Allow_1
+	(deny type_1 type_2 (class1 (perm1)))  ; Deny_1
+  	; Allow_1 will be complete removed by Deny_1.
+
+    (type type_3)
+	(type type_4)
+	(allow type_3 type_4 (class1 (perm1 perm2))) ; Allow_2
+	(deny type_3 type_4 (class1 (perm1)))        ; Deny_2
+	; Allow_2 will be removed and replaced with the following when Deny_2 is evaluated
+    ; (allow type_3 type_4 (class1 (perm2)))
+
+	(type type_5)
+	(type type_6)
+	(typeattribute attr_1)
+	(typeattributeset attr_1 (type_5 type_6))
+	(allow attr_1 attr_1 (class1 (perm1))) ; Allow_3
+	(deny type_5 type_6 (class1 (perm1)))  ; Deny_3
+	; Allow_3 will be removed and replaced with the following when Deny_3 is evaluated
+	; (allow type_6 attr_1 (class1 (perm1)))
+	; (allow attr_1 type_5 (class1 (perm1)))
+    )
+```
 
 allowx
 ------
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/9] Add CIL Deny Rule
  2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
                   ` (8 preceding siblings ...)
  2022-12-15 21:34 ` [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation James Carter
@ 2022-12-16 18:51 ` Daniel Burgener
  2022-12-16 20:23   ` James Carter
  9 siblings, 1 reply; 20+ messages in thread
From: Daniel Burgener @ 2022-12-16 18:51 UTC (permalink / raw)
  To: James Carter, selinux

On 12/15/2022 4:34 PM, James Carter wrote:
> I don't expect this to be part of the upcoming userspace release,
> but I did want to see if this is going to be what Cascade needs.

Thank you!  I've been playing with this series locally all morning and 
so far it mostly works as expected.  The only minor surprise so far is 
if I do something like:

(type my_type1)
(type my_type2)
(type my_type3)
(typeattributeset my_attr (my_type1 my_type2 my_type3))
(allow my_attr my_attr (process (signal signull)))
(deny my_type1 my_attr (process (signal)))

I get rules like this:

$ sesearch -A -s my_attr policy.33
allow deny_rule_attr_11 my_attr:process { signal signull };
allow my_attr my_attr:process signull;

Since deny_rule_attr_11 is a subset of my_attr (specifically, my_type2 
and my_type3), both of those types get the "signull" permission from the 
second rule on the attribute, and only strictly need the "signal" 
permission (which my_type1 doesn't get)

It's obviously not a real problem, since it's just redundant policy and 
both versions are functionally equivalent.  It's just a little weird 
browsing the rules using sesearch, particularly with large permission 
sets (My first attribute test involved using "all" on the file class and 
denying one permission from a type, and it was mildly challenging to 
manually verify the results looking at the allow rules in a large list.)

Anyways, besides that minor issue everything I've tried so far has 
worked well and the overall concept seems sensible and helpful. 
Although, I still need to get my head around the specifics of the 
attribute handling, and I haven't looked thoroughly at the code yet.

I intend to do more thorough testing and code review, but I don't expect 
I'll have time before the holidays, so that will likely come in January. 
  Since I don't have time for anything more thorough now, hopefully some 
first impressions are helpful in the interim.

-Daniel

> 
> This series of patches implements a deny rule in CIL. A deny rule will remove
> the stated permissions in it from the policy. CIL does this by searching for
> allow rules that match the deny rule and then writing new allow rules that
> correspond to the matched allow rule with the permissions from the deny rule
> removed. The rule uses the same syntax as an allow rule, but with "deny"
> instead of "allow".
> 
>    (deny SRC TGT (CLASS (PERMS)))
> 
> Deny rules are processed during post processing (after the AST is resolved,
> but before the binary policy is written). This means that neverallow checking
> is done after deny rules are resolved. Deny rules are complimentary to
> neverallow checking. When an allow rule is found that matches, a deny rule
> removes permissions while a neverallow rule reports an error.
> 
> Patch 4 is biggest and most complex since it is the one doing the processing.
> 
> James Carter (9):
>    libsepol/cil: Parse and add deny rule to AST, but do not process
>    libsepol/cil: Add cil_list_is_empty macro
>    libsepol/cil: Add cil_tree_remove_node function
>    libsepol/cil: Process deny rules
>    libsepol/cil: Add cil_write_post_ast function
>    libsepol: Export the cil_write_post_ast function
>    secilc/secil2tree: Add option to write CIL AST after post processing
>    secilc/test: Add a deny rule test
>    secilc/docs: Add deny rule to CIL documentation
> 
>   libsepol/cil/include/cil/cil.h         |   1 +
>   libsepol/cil/src/cil.c                 |  68 ++
>   libsepol/cil/src/cil_build_ast.c       |  56 ++
>   libsepol/cil/src/cil_build_ast.h       |   2 +
>   libsepol/cil/src/cil_copy_ast.c        |  19 +
>   libsepol/cil/src/cil_copy_ast.h        |   1 +
>   libsepol/cil/src/cil_deny.c            | 957 +++++++++++++++++++++++++
>   libsepol/cil/src/cil_deny.h            |  34 +
>   libsepol/cil/src/cil_flavor.h          |   1 +
>   libsepol/cil/src/cil_internal.h        |  10 +
>   libsepol/cil/src/cil_list.h            |   3 +
>   libsepol/cil/src/cil_post.c            |   7 +
>   libsepol/cil/src/cil_reset_ast.c       |   8 +
>   libsepol/cil/src/cil_resolve_ast.c     |  44 ++
>   libsepol/cil/src/cil_resolve_ast.h     |   1 +
>   libsepol/cil/src/cil_tree.c            |  27 +
>   libsepol/cil/src/cil_tree.h            |   1 +
>   libsepol/cil/src/cil_verify.c          |   9 +
>   libsepol/cil/src/cil_write_ast.c       |  10 +
>   libsepol/cil/src/cil_write_ast.h       |   1 +
>   libsepol/src/libsepol.map.in           |   5 +
>   secilc/docs/cil_access_vector_rules.md |  68 ++
>   secilc/secil2tree.c                    |   8 +-
>   secilc/test/deny_rule_test.cil         | 384 ++++++++++
>   24 files changed, 1724 insertions(+), 1 deletion(-)
>   create mode 100644 libsepol/cil/src/cil_deny.c
>   create mode 100644 libsepol/cil/src/cil_deny.h
>   create mode 100644 secilc/test/deny_rule_test.cil
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/9] Add CIL Deny Rule
  2022-12-16 18:51 ` [RFC PATCH 0/9] Add CIL Deny Rule Daniel Burgener
@ 2022-12-16 20:23   ` James Carter
  0 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2022-12-16 20:23 UTC (permalink / raw)
  To: Daniel Burgener; +Cc: selinux

On Fri, Dec 16, 2022 at 1:52 PM Daniel Burgener
<dburgener@linux.microsoft.com> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > I don't expect this to be part of the upcoming userspace release,
> > but I did want to see if this is going to be what Cascade needs.
>
> Thank you!  I've been playing with this series locally all morning and
> so far it mostly works as expected.  The only minor surprise so far is
> if I do something like:
>
> (type my_type1)
> (type my_type2)
> (type my_type3)
> (typeattributeset my_attr (my_type1 my_type2 my_type3))
> (allow my_attr my_attr (process (signal signull)))
> (deny my_type1 my_attr (process (signal)))
>
> I get rules like this:
>
> $ sesearch -A -s my_attr policy.33
> allow deny_rule_attr_11 my_attr:process { signal signull };
> allow my_attr my_attr:process signull;
>
> Since deny_rule_attr_11 is a subset of my_attr (specifically, my_type2
> and my_type3), both of those types get the "signull" permission from the
> second rule on the attribute, and only strictly need the "signal"
> permission (which my_type1 doesn't get)
>
> It's obviously not a real problem, since it's just redundant policy and
> both versions are functionally equivalent.  It's just a little weird
> browsing the rules using sesearch, particularly with large permission
> sets (My first attribute test involved using "all" on the file class and
> denying one permission from a type, and it was mildly challenging to
> manually verify the results looking at the allow rules in a large list.)
>

I knew that I was creating some redundancy in the rules, but I was
trying to eliminate the special cases. Now that I have something that
appears to be working, it will probably be worth it to go back and see
how messy it would be to remove some of the redundancy.

> Anyways, besides that minor issue everything I've tried so far has
> worked well and the overall concept seems sensible and helpful.
> Although, I still need to get my head around the specifics of the
> attribute handling, and I haven't looked thoroughly at the code yet.
>
> I intend to do more thorough testing and code review, but I don't expect
> I'll have time before the holidays, so that will likely come in January.
>   Since I don't have time for anything more thorough now, hopefully some
> first impressions are helpful in the interim.
>

I am going to be on vacation for the last two weeks of December, so I
won't be getting back to this until January anyway.

Thanks for the quick look and comments!
Jim


> -Daniel
>
> >
> > This series of patches implements a deny rule in CIL. A deny rule will remove
> > the stated permissions in it from the policy. CIL does this by searching for
> > allow rules that match the deny rule and then writing new allow rules that
> > correspond to the matched allow rule with the permissions from the deny rule
> > removed. The rule uses the same syntax as an allow rule, but with "deny"
> > instead of "allow".
> >
> >    (deny SRC TGT (CLASS (PERMS)))
> >
> > Deny rules are processed during post processing (after the AST is resolved,
> > but before the binary policy is written). This means that neverallow checking
> > is done after deny rules are resolved. Deny rules are complimentary to
> > neverallow checking. When an allow rule is found that matches, a deny rule
> > removes permissions while a neverallow rule reports an error.
> >
> > Patch 4 is biggest and most complex since it is the one doing the processing.
> >
> > James Carter (9):
> >    libsepol/cil: Parse and add deny rule to AST, but do not process
> >    libsepol/cil: Add cil_list_is_empty macro
> >    libsepol/cil: Add cil_tree_remove_node function
> >    libsepol/cil: Process deny rules
> >    libsepol/cil: Add cil_write_post_ast function
> >    libsepol: Export the cil_write_post_ast function
> >    secilc/secil2tree: Add option to write CIL AST after post processing
> >    secilc/test: Add a deny rule test
> >    secilc/docs: Add deny rule to CIL documentation
> >
> >   libsepol/cil/include/cil/cil.h         |   1 +
> >   libsepol/cil/src/cil.c                 |  68 ++
> >   libsepol/cil/src/cil_build_ast.c       |  56 ++
> >   libsepol/cil/src/cil_build_ast.h       |   2 +
> >   libsepol/cil/src/cil_copy_ast.c        |  19 +
> >   libsepol/cil/src/cil_copy_ast.h        |   1 +
> >   libsepol/cil/src/cil_deny.c            | 957 +++++++++++++++++++++++++
> >   libsepol/cil/src/cil_deny.h            |  34 +
> >   libsepol/cil/src/cil_flavor.h          |   1 +
> >   libsepol/cil/src/cil_internal.h        |  10 +
> >   libsepol/cil/src/cil_list.h            |   3 +
> >   libsepol/cil/src/cil_post.c            |   7 +
> >   libsepol/cil/src/cil_reset_ast.c       |   8 +
> >   libsepol/cil/src/cil_resolve_ast.c     |  44 ++
> >   libsepol/cil/src/cil_resolve_ast.h     |   1 +
> >   libsepol/cil/src/cil_tree.c            |  27 +
> >   libsepol/cil/src/cil_tree.h            |   1 +
> >   libsepol/cil/src/cil_verify.c          |   9 +
> >   libsepol/cil/src/cil_write_ast.c       |  10 +
> >   libsepol/cil/src/cil_write_ast.h       |   1 +
> >   libsepol/src/libsepol.map.in           |   5 +
> >   secilc/docs/cil_access_vector_rules.md |  68 ++
> >   secilc/secil2tree.c                    |   8 +-
> >   secilc/test/deny_rule_test.cil         | 384 ++++++++++
> >   24 files changed, 1724 insertions(+), 1 deletion(-)
> >   create mode 100644 libsepol/cil/src/cil_deny.c
> >   create mode 100644 libsepol/cil/src/cil_deny.h
> >   create mode 100644 secilc/test/deny_rule_test.cil
> >
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function
  2022-12-15 21:34 ` [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function James Carter
@ 2023-02-03 22:54   ` Daniel Burgener
  2023-02-08 21:09     ` James Carter
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Burgener @ 2023-02-03 22:54 UTC (permalink / raw)
  To: James Carter, selinux

On 12/15/2022 4:34 PM, James Carter wrote:
> Add the function cil_tree_remove_node() which takes a node pointer
> as an input, finds the parent, walks the list of nodes to the node
> prior to the given node, and then updates that nodes next pointer
> to remove the given node from the tree.
> 
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>   libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++
>   libsepol/cil/src/cil_tree.h |  1 +
>   2 files changed, 28 insertions(+)
> 
> diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> index 6376c208..73b4e135 100644
> --- a/libsepol/cil/src/cil_tree.c
> +++ b/libsepol/cil/src/cil_tree.c
> @@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node)
>   	*node = NULL;
>   }
>   
> +void cil_tree_remove_node(struct cil_tree_node *node)
> +{
> +	struct cil_tree_node *parent, *curr;
> +
> +	if (node == NULL || node->parent == NULL) {
> +		return;
> +	}
> +
> +	parent = node->parent;
> +
> +	if (parent->cl_head == node) {
> +		parent->cl_head = node->next;
> +		return;
> +	}
> +
> +	curr = parent->cl_head;
> +	while (curr && curr->next != node) {
> +		curr = curr->next;
> +	}
> +
> +	if (curr == NULL) {
> +		return;
> +	}
> +
> +	curr->next = node->next;
> +}
> +

Is there a reason this leaves it to the caller to call 
cil_tree_node_destroy()?  It just feels a little weird to leave the node 
unreachable without freeing.  It looks like both callers in the series 
(cil_process_deny_rule(s)) call cil_tree_node_destroy() immediately after.

-Daniel

>   /* Perform depth-first walk of the tree
>      Parameters:
>      start_node:          root node to start walking from
> diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
> index 5a98da55..e4b3fd04 100644
> --- a/libsepol/cil/src/cil_tree.h
> +++ b/libsepol/cil/src/cil_tree.h
> @@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node);
>   
>   void cil_tree_node_init(struct cil_tree_node **node);
>   void cil_tree_node_destroy(struct cil_tree_node **node);
> +void cil_tree_remove_node(struct cil_tree_node *node);
>   
>   //finished values
>   #define CIL_TREE_SKIP_NOTHING	0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 4/9] libsepol/cil: Process deny rules
  2022-12-15 21:34 ` [RFC PATCH 4/9] libsepol/cil: Process deny rules James Carter
@ 2023-02-03 22:54   ` Daniel Burgener
  2023-02-08 21:57     ` James Carter
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Burgener @ 2023-02-03 22:54 UTC (permalink / raw)
  To: James Carter, selinux

On 12/15/2022 4:34 PM, James Carter wrote:
> A deny rule is like a neverallow rule, except that permissions are
> removed rather than an error reported.
> 
> (allow S1 T1 P1)
> (deny  S2 T2 P2)
> 
> In general what is needed is:
> 1) Allow permissions that do not match the deny rule
>    P3 = P1 and not P2
>    (allow S1 T1 P3)
> 2) Allow source types that do not match the deny rule
>    S3 = S1 and not S2
>    (allow S3 T1 P1)
> 3) Allow target types that do not match the deny rule
>    T3 = T1 and not T2
>    (allow S1 T3 P1)
> 
> If a self rule is involved then
>    If S1 is an attribute then
>      If T1 is self and T2 is not self then
>        S3 = S1 and not (S2 and T2)
>      else if T1 is not self and T2 is self then
>        S3 = S1 and not (T1 and S2)
>      else if T1 is self and T2 is self then
>        S3 = S1 and not S2
>      (allow S3 T1 P1)
>    If T1 is an attribute then
>      If T2 is self then
>        T3 = T1 and not (S1 and S2)
>      (allow S1 T3 P1)
>      If S1 is attribute and S2 is attribute and T2 is self then
>        # In addition to any rules involving S3 and T3 above
>        SX = S1 and S2 and T1
>        For S in SX do
>          T = SX and not S
>          (allow S T P1)
> 
> In all cases, do not create a rule that has an empty set
> 
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>   libsepol/cil/src/cil_deny.c | 957 ++++++++++++++++++++++++++++++++++++
>   libsepol/cil/src/cil_deny.h |  34 ++
>   libsepol/cil/src/cil_post.c |   7 +
>   3 files changed, 998 insertions(+)
>   create mode 100644 libsepol/cil/src/cil_deny.c
>   create mode 100644 libsepol/cil/src/cil_deny.h
> 
> diff --git a/libsepol/cil/src/cil_deny.c b/libsepol/cil/src/cil_deny.c
> new file mode 100644
> index 00000000..43043c37
> --- /dev/null
> +++ b/libsepol/cil/src/cil_deny.c
> @@ -0,0 +1,957 @@
> +/*
> + * This file is public domain software, i.e. not copyrighted.
> + *
> + * Warranty Exclusion
> + * ------------------
> + * You agree that this software is a non-commercially developed program
> + * that may contain "bugs" (as that term is used in the industry) and
> + * that it may not function as intended. The software is licensed
> + * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
> + * express, implied, statutory, or otherwise with respect to the software,
> + * including noninfringement and the implied warranties of merchantability
> + * and fitness for a particular purpose.
> + *
> + * Limitation of Liability
> + *-----------------------
> + * In no event will NSA be liable for any damages, including loss of data,
> + * lost profits, cost of cover, or other special, incidental, consequential,
> + * direct or indirect damages arising from the software or the use thereof,
> + * however caused and on any theory of liability. This limitation will apply
> + * even if NSA has been advised of the possibility of such damage. You
> + * acknowledge that this is a reasonable allocation of risk.
> + *
> + * Original author: James Carter
> + */
> +
> +#include <sepol/policydb/ebitmap.h>
> +
> +#include "cil_internal.h"
> +#include "cil_find.h"
> +#include "cil_flavor.h"
> +#include "cil_list.h"
> +#include "cil_strpool.h"
> +#include "cil_log.h"
> +#include "cil_symtab.h"
> +#include "cil_build_ast.h"
> +#include "cil_copy_ast.h"
> +#include "cil_deny.h"
> +
> +#define CIL_DENY_ATTR_PREFIX "deny_rule_attr"
> +
> +/*
> + * A deny rule will remove the specified permissions from the CIL AST.
> + *
> + * (allow S1 T1 P1)
> + * (deny  S2 T2 P2)
> + *
> + * In general what is needed is:
> + * 1) Allow permissions that do not match the deny rule
> + *   P3 = P1 and not P2
> + *   (allow S1 T1 P3)
> + * 2) Allow source types that do not match the deny rule
> + *   S3 = S1 and not S2
> + *   (allow S3 T1 P1)
> + * 3) Allow target types that do not match the deny rule
> + *   T3 = T1 and not T2
> + *   (allow S1 T3 P1)
> + *
> + * If a self rule is involved then
> + *   If S1 is an attribute then
> + *     If T1 is self and T2 is not self then
> + *       S3 = S1 and not (S2 and T2)
> + *     else if T1 is not self and T2 is self then
> + *       S3 = S1 and not (T1 and S2)
> + *     else if T1 is self and T2 is self then
> + *       S3 = S1 and not S2
> + *     (allow S3 T1 P1)
> + *   If T1 is an attribute then
> + *     If T2 is self then
> + *       T3 = T1 and not (S1 and S2)
> + *     (allow S1 T3 P1)
> + *     If S1 is attribute and S2 is attribute and T2 is self then

nit: The rest of the lines refer to "an" attribute, and this one drops 
the article.

BTW, thanks for this description of the cases.  It's very helpful 
working through the logic.

> + *       # In addition to any rules involving S3 and T3 above
> + *       SX = S1 and S2 and T1
> + *       For S in SX do
> + *         T = SX and not S
> + *         (allow S T P1)
> + *
> + * Do not create a rule that has an empty set
> + */
> +
> +static int cil_perm_match(const struct cil_perm *p1, const struct cil_list *pl2)
> +{
> +	struct cil_list_item *curr;
> +
> +	cil_list_for_each(curr, pl2) {
> +		struct cil_perm *p = curr->data;
> +		if (p == p1) {
> +			return CIL_TRUE;
> +		}

Why are we comparing the pointers rather than the value fields?

> +	}
> +	return CIL_FALSE;
> +}
> +
> +static int cil_class_perm_match(const struct cil_class *c1, const struct cil_perm *p1, const struct cil_list *cpl2)
> +{
> +	struct cil_list_item *curr;
> +
> +	cil_list_for_each(curr, cpl2) {
> +		if (curr->flavor == CIL_CLASSPERMS) {
> +			struct cil_classperms *cp = curr->data;
> +			if (FLAVOR(cp->class) == CIL_CLASS) {
> +				if (cp->class == c1) {
> +					if (cil_perm_match(p1, cp->perms)) {
> +						return CIL_TRUE;
> +					}
> +				}
> +			} else { /* MAP */
> +				struct cil_list_item *p;
> +				cil_list_for_each(p, cp->perms) {
> +					struct cil_perm *cmp = p->data;
> +					if (cil_class_perm_match(c1, p1, cmp->classperms)) {
> +						return CIL_TRUE;
> +					}
> +				}
> +			}
> +		} else { /* SET */
> +			struct cil_classperms_set *cp_set = curr->data;
> +			struct cil_classpermission *cp = cp_set->set;
> +			if (cil_class_perm_match(c1, p1, cp->classperms)) {
> +				return CIL_TRUE;
> +			}
> +		}
> +	}
> +	return CIL_FALSE;
> +}
> +
> +static int cil_classperms_match(const struct cil_classperms *cp1, const struct cil_list *cpl2)
> +{
> +	struct cil_list_item *curr;
> +
> +	cil_list_for_each(curr, cp1->perms) {
> +		struct cil_perm *perm = curr->data;
> +		if (cil_class_perm_match(cp1->class, perm, cpl2)) {
> +			return CIL_TRUE;
> +		}
> +	}
> +	return CIL_FALSE;
> +}
> +
> +int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2)
> +{
> +	struct cil_list_item *curr;
> +
> +	if (!cpl1 || !cpl2) {
> +		return (!cpl1 && !cpl2) ? CIL_TRUE : CIL_FALSE;
> +	}
> +
> +	cil_list_for_each(curr, cpl1) {
> +		if (curr->flavor == CIL_CLASSPERMS) {
> +			struct cil_classperms *cp = curr->data;
> +			if (FLAVOR(cp->class) == CIL_CLASS) {
> +				if (cil_classperms_match(cp, cpl2)) {
> +					return CIL_TRUE;
> +				}
> +			} else { /* MAP */
> +				struct cil_list_item *p;
> +				cil_list_for_each(p, cp->perms) {
> +					struct cil_perm *cmp = p->data;
> +					if (cil_classperms_list_match(cmp->classperms, cpl2)) {
> +						return CIL_TRUE;
> +					}
> +				}
> +			}
> +		} else { /* SET */
> +			struct cil_classperms_set *cp_set = curr->data;
> +			struct cil_classpermission *cp = cp_set->set;
> +			if (cil_classperms_list_match(cp->classperms, cpl2)) {
> +				return CIL_TRUE;
> +			}
> +		}
> +	}
> +	return CIL_FALSE;
> +}
> +
> +static void cil_classperms_copy(struct cil_classperms **new, const struct cil_classperms *old)
> +{
> +	cil_classperms_init(new);
> +	(*new)->class_str = old->class_str;
> +	(*new)->class = old->class;
> +	cil_copy_list(old->perm_strs, &(*new)->perm_strs);
> +	cil_copy_list(old->perms, &(*new)->perms);
> +}
> +
> +static void cil_classperms_set_copy(struct cil_classperms_set **new, const struct cil_classperms_set *old)
> +{
> +	cil_classperms_set_init(new);
> +	(*new)->set_str = old->set_str;
> +	(*new)->set = old->set;
> +}
> +
> +void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old)
> +{
> +	struct cil_list_item *curr;
> +
> +	if (!new) {
> +		return;
> +	}
> +
> +	if (!old) {
> +		*new = NULL;
> +		return;
> +	}
> +
> +	cil_list_init(new, CIL_LIST);
> +
> +	cil_list_for_each(curr, old) {
> +		if (curr->flavor == CIL_CLASSPERMS) {
> +			struct cil_classperms *new_cp;
> +			cil_classperms_copy(&new_cp, curr->data);
> +			cil_list_append(*new, CIL_CLASSPERMS, new_cp);
> +		} else { /* SET */
> +			struct cil_classperms_set *new_cps;
> +			cil_classperms_set_copy(&new_cps, curr->data);
> +			cil_list_append(*new, CIL_CLASSPERMS_SET, new_cps);
> +		}
> +	}
> +
> +	if (cil_list_is_empty(*new)) {
> +		cil_list_destroy(new, CIL_FALSE);
> +	}
> +}
> +
> +/* Append cp1 and not cpl2 to result */
> +static void cil_classperms_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
> +{
> +	struct cil_classperms *new_cp = NULL;
> +	struct cil_list_item *curr;
> +
> +	if (!cil_classperms_match(cp1, cpl2)) {
> +		cil_classperms_copy(&new_cp, cp1);
> +		cil_list_append(*result, CIL_CLASSPERMS, new_cp);
> +		return;
> +	}
> +
> +	cil_list_for_each(curr, cp1->perms) {
> +		struct cil_perm *perm = curr->data;
> +		if (!cil_class_perm_match(cp1->class, perm, cpl2)) {
> +			if (new_cp == NULL) {
> +				cil_classperms_init(&new_cp);
> +				new_cp->class_str = cp1->class_str;
> +				new_cp->class = cp1->class;
> +				cil_list_init(&new_cp->perm_strs, CIL_PERM);
> +				cil_list_init(&new_cp->perms, CIL_PERM);
> +				cil_list_append(*result, CIL_CLASSPERMS, new_cp) > +			}
> +			cil_list_append(new_cp->perm_strs, CIL_STRING, perm->datum.fqn);
> +			cil_list_append(new_cp->perms, CIL_DATUM, perm);

Why can't you just use cil_classperms_copy() here?

> +		}
> +	}
> +}
> +
> +/* Append cp1 and not cpl2 to result */
> +static void cil_classperms_map_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
> +{
> +	struct cil_classperms *new_cp = NULL;
> +	struct cil_list_item *p;
> +
> +	cil_list_for_each(p, cp1->perms) {
> +		struct cil_perm *map_perm = p->data;
> +		if (!cil_classperms_list_match(map_perm->classperms, cpl2)) {
> +			if (new_cp == NULL) {
> +				cil_classperms_init(&new_cp);
> +				new_cp->class_str = cp1->class_str;
> +				new_cp->class = cp1->class;
> +				cil_list_init(&new_cp->perm_strs, CIL_PERM);
> +				cil_list_init(&new_cp->perms, CIL_PERM);
> +				cil_list_append(*result, CIL_CLASSPERMS, new_cp);
> +			}
> +			cil_list_append(new_cp->perm_strs, CIL_STRING, map_perm->datum.fqn);
> +			cil_list_append(new_cp->perms, CIL_DATUM, map_perm);

Same question about cil_classperms_copy().

> +		} else {
> +			struct cil_list *new_cpl = NULL;
> +			cil_classperms_list_andnot(&new_cpl, map_perm->classperms, cpl2);
> +			if (new_cpl) {
> +				struct cil_list_item *i;
> +				cil_list_for_each(i, new_cpl) {
> +					cil_list_append(*result, i->flavor, i->data);
> +				}
> +				cil_list_destroy(&new_cpl, CIL_FALSE);
> +			}
> +		}
> +	}
> +}
> +
> +/* Append cps1 and not cpl2 to result */
> +static void cil_classperms_set_andnot(struct cil_list **result, const struct cil_classperms_set *cps1, const struct cil_list *cpl2)
> +{
> +	struct cil_classpermission *cp = cps1->set;
> +
> +	if (!cil_classperms_list_match(cp->classperms, cpl2)) {
> +		struct cil_classperms_set *new_cps;
> +		cil_classperms_set_copy(&new_cps, cps1);
> +		cil_list_append(*result, CIL_CLASSPERMS_SET, new_cps);
> +	} else {
> +		struct cil_list *new_cpl;
> +		cil_classperms_list_andnot(&new_cpl, cp->classperms, cpl2);
> +		if (new_cpl) {
> +			struct cil_list_item *i;
> +			cil_list_for_each(i, new_cpl) {
> +				cil_list_append(*result, i->flavor, i->data);
> +			}
> +			cil_list_destroy(&new_cpl, CIL_FALSE);
> +		}
> +	}
> +}
> +
> +/* result = cpl1 and not cpl2 */
> +void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2)
> +{
> +	struct cil_list_item *curr;
> +
> +	if (!result) {
> +		return;
> +	}
> +
> +	if (!cpl1) {
> +		*result = NULL;
> +		return;
> +	}
> +
> +	cil_list_init(result, CIL_LIST);
> +
> +	if (!cpl2 || !cil_classperms_list_match(cpl1, cpl2)) {
> +		cil_classperms_list_copy(result, cpl1);
> +		return;
> +	}
> +
> +	cil_list_for_each(curr, cpl1) {
> +		if (curr->flavor == CIL_CLASSPERMS) {
> +			struct cil_classperms *cp = curr->data;
> +			if (FLAVOR(cp->class) == CIL_CLASS) {
> +				cil_classperms_andnot(result, cp, cpl2);
> +			} else { /* MAP */
> +				cil_classperms_map_andnot(result, cp, cpl2);
> +			}
> +		} else { /* SET */
> +			struct cil_classperms_set *cps = curr->data;
> +			cil_classperms_set_andnot(result, cps, cpl2);
> +		}
> +	}
> +
> +	if (cil_list_is_empty(*result)) {
> +		cil_list_destroy(result, CIL_FALSE);
> +	}
> +}
> +
> +/* result = d1 and d2 */
> +static int cil_datums_and(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
> +{
> +	int rc = SEPOL_OK;
> +	enum cil_flavor f1 = FLAVOR(d1);
> +	enum cil_flavor f2 = FLAVOR(d2);
> +
> +	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> +		struct cil_type *t1 = (struct cil_type *)d1;
> +		struct cil_type *t2 = (struct cil_type *)d2;
> +		ebitmap_init(result);
> +		if (t1->value == t2->value) {
> +			rc = ebitmap_set_bit(result, t1->value, 1);
> +			if (rc != SEPOL_OK) {
> +				ebitmap_destroy(result);
> +				goto exit;
> +			}
> +		}
> +	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> +		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> +		struct cil_type *t2 = (struct cil_type *)d2;
> +		ebitmap_init(result);
> +		if (ebitmap_get_bit(a1->types, t2->value)) {
> +			rc = ebitmap_set_bit(result, t2->value, 1);
> +			if (rc != SEPOL_OK) {
> +				ebitmap_destroy(result);
> +				goto exit;
> +			}
> +		}
> +	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
> +		struct cil_type *t1 = (struct cil_type *)d1;
> +		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> +		ebitmap_init(result);
> +		if (ebitmap_get_bit(a2->types, t1->value)) {
> +			rc = ebitmap_set_bit(result, t1->value, 1);
> +			if (rc != SEPOL_OK) {
> +				ebitmap_destroy(result);
> +				goto exit;
> +			}
> +		}
> +	} else {
> +		/* Both are attributes */
> +		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> +		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> +		rc = ebitmap_and(result, a1->types, a2->types);
> +		if (rc != SEPOL_OK) {
> +			ebitmap_destroy(result);
> +			goto exit;
> +		}
> +	}
> +exit:
> +	return rc;
> +}
> +
> +/* result = d1 and not d2 */
> +static int cil_datums_andnot(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
> +{
> +	int rc = SEPOL_OK;
> +	enum cil_flavor f1 = FLAVOR(d1);
> +	enum cil_flavor f2 = FLAVOR(d2);
> +
> +	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> +		struct cil_type *t1 = (struct cil_type *)d1;
> +		struct cil_type *t2 = (struct cil_type *)d2;
> +		ebitmap_init(result);
> +		if (t1->value != t2->value) {
> +			rc = ebitmap_set_bit(result, t1->value, 1);
> +			if (rc != SEPOL_OK) {
> +				ebitmap_destroy(result);
> +				goto exit;
> +			}
> +		}
> +	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> +		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> +		struct cil_type *t2 = (struct cil_type *)d2;
> +		rc = ebitmap_cpy(result, a1->types);
> +		if (rc != SEPOL_OK) {
> +			goto exit;
> +		}
> +		rc = ebitmap_set_bit(result, t2->value, 0);
> +		if (rc != SEPOL_OK) {
> +			ebitmap_destroy(result);
> +			goto exit;
> +		}
> +	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
> +		struct cil_type *t1 = (struct cil_type *)d1;
> +		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> +		ebitmap_init(result);
> +		if (!ebitmap_get_bit(a2->types, t1->value)) {
> +			rc = ebitmap_set_bit(result, t1->value, 1);
> +			if (rc != SEPOL_OK) {
> +				ebitmap_destroy(result);
> +				goto exit;
> +			}
> +		}
> +	} else {
> +		/* Both are attributes */
> +		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> +		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> +		rc = ebitmap_andnot(result, a1->types, a2->types, a1->types->highbit);
> +		if (rc != SEPOL_OK) {
> +			ebitmap_destroy(result);
> +			goto exit;
> +		}
> +	}
> +exit:
> +	return rc;
> +}
> +
> +static size_t num_digits(unsigned n)
> +{
> +	size_t num = 1;
> +	while (n >= 10) {
> +		n /= 10;
> +		num++;
> +	}
> +	return num;
> +}
> +
> +static char *cil_create_new_attribute_name(unsigned num)
> +{
> +	char *s1 = NULL;
> +	char *s2 = NULL;
> +	size_t len_num = num_digits(num);
> +	size_t len = strlen(CIL_DENY_ATTR_PREFIX) + 1 + len_num + 1;
> +	int rc;
> +
> +	if (len >= CIL_MAX_NAME_LENGTH) {
> +		cil_log(CIL_ERR, "Name length greater than max name length of %d",
> +				CIL_MAX_NAME_LENGTH);
> +		goto exit;
> +	}
> +
> +	s1 = cil_malloc(len);
> +	rc = snprintf(s1, len, "%s_%u", CIL_DENY_ATTR_PREFIX, num);
> +	if (rc < 0 || (size_t)rc >= len) {
> +		cil_log(CIL_ERR, "Error creating new attribute name");
> +		free(s1);
> +		goto exit;
> +	}
> +
> +	s2 = cil_strpool_add(s1);
> +	free(s1);
> +
> +exit:
> +	return s2;
> +}
> +
> +static struct cil_list *cil_create_and_expr_list(enum cil_flavor flavor, void *v1, void *v2)
> +{
> +	struct cil_list *expr;
> +
> +	cil_list_init(&expr, CIL_TYPE);
> +	cil_list_append(expr, CIL_OP, (void *)CIL_AND);
> +	cil_list_append(expr, flavor, v1);
> +	cil_list_append(expr, flavor, v2);
> +
> +	return expr;
> +}
> +
> +static struct cil_list *cil_create_andnot_expr_list(enum cil_flavor flavor, void *v1, void *v2)
> +{
> +	struct cil_list *expr, *sub_expr;
> +
> +	cil_list_init(&expr, CIL_TYPE);
> +	cil_list_append(expr, CIL_OP, (void *)CIL_AND);
> +	cil_list_append(expr, flavor, v1);
> +	cil_list_init(&sub_expr, CIL_TYPE);
> +	cil_list_append(sub_expr, CIL_OP, (void *)CIL_NOT);
> +	cil_list_append(sub_expr, flavor, v2);
> +	cil_list_append(expr, CIL_LIST, sub_expr);
> +
> +	return expr;
> +}
> +
> +static struct cil_tree_node *cil_create_and_insert_node(struct cil_tree_node *prev, enum cil_flavor flavor, void *data)
> +{
> +	struct cil_tree_node *new;
> +
> +	cil_tree_node_init(&new);
> +	new->parent = prev->parent;
> +	new->line = prev->line;
> +	new->hll_offset = prev->hll_offset;
> +	new->flavor = flavor;
> +	new->data = data;
> +	new->next = prev->next;
> +	prev->next = new;
> +
> +	return new;
> +}
> +
> +static int cil_create_and_insert_attribute_and_set(struct cil_db *db, struct cil_tree_node *prev, struct cil_list *str_expr, struct cil_list *datum_expr, ebitmap_t *types, struct cil_symtab_datum **d)
> +{
> +	struct cil_tree_node *attr_node = NULL;
> +	char *name;
> +	struct cil_typeattribute *attr = NULL;
> +	struct cil_tree_node *attrset_node = NULL;
> +	struct cil_typeattributeset *attrset = NULL;
> +	symtab_t *symtab = NULL;
> +	int rc = SEPOL_ERR;
> +
> +	name = cil_create_new_attribute_name(db->num_types_and_attrs);
> +	if (!name) {
> +		goto exit;
> +	}
> +
> +	cil_typeattributeset_init(&attrset);
> +	attrset->attr_str = name;
> +	attrset->str_expr = str_expr;
> +	attrset->datum_expr = datum_expr;
> +
> +	cil_typeattribute_init(&attr);
> +	cil_list_init(&attr->expr_list, CIL_TYPE);
> +	cil_list_append(attr->expr_list, CIL_LIST, datum_expr);
> +	attr->types = types;
> +	attr->used = CIL_ATTR_AVRULE;
> +	attr->keep = (ebitmap_cardinality(types) < db->attrs_expand_size) ? CIL_FALSE : CIL_TRUE;
> +
> +	attr_node = cil_create_and_insert_node(prev, CIL_TYPEATTRIBUTE, attr);
> +	attrset_node = cil_create_and_insert_node(attr_node, CIL_TYPEATTRIBUTESET, attrset);
> +
> +	rc = cil_get_symtab(prev->parent, &symtab, CIL_SYM_TYPES);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}
> +
> +	rc = cil_symtab_insert(symtab, name, &attr->datum, attr_node);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}
> +
> +	db->num_types_and_attrs++;
> +
> +	*d = &attr->datum;
> +
> +	return SEPOL_OK;
> +
> +exit:
> +	if (attrset_node) {
> +		prev->next = attrset_node->next;
> +		cil_destroy_typeattribute(attr_node->data);
> +		free(attr_node);
> +		cil_destroy_typeattributeset(attrset_node->data);
> +		free(attrset_node);
> +	}
> +	return rc;
> +}
> +
> +static int cil_create_attribute_d1_and_not_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
> +{
> +	struct cil_list *str_expr;
> +	struct cil_list *datum_expr;
> +	ebitmap_t *types;
> +	int rc;
> +
> +	*d3 = NULL;
> +
> +	if (d1 == d2) {
> +		return SEPOL_OK;
> +	}
> +
> +	str_expr = cil_create_andnot_expr_list(CIL_STRING, d1->fqn, d2->fqn);
> +	datum_expr = cil_create_andnot_expr_list(CIL_DATUM, d1, d2);
> +
> +	types = cil_malloc(sizeof(*types));
> +	rc = cil_datums_andnot(types, d1, d2);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}
> +	if (ebitmap_is_empty(types)) {
> +		rc = SEPOL_OK;
> +		goto exit;
> +	}
> +
> +	if (ebitmap_cardinality(types) == 1) {
> +		unsigned i = ebitmap_highest_set_bit(types);
> +		*d3 = DATUM(db->val_to_type[i]);
> +		ebitmap_destroy(types);
> +		rc = SEPOL_OK;
> +		goto exit;
> +	}
> +
> +	return cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
> +
> +exit:
> +	cil_list_destroy(&str_expr, CIL_FALSE);
> +	cil_list_destroy(&datum_expr, CIL_FALSE);
> +	free(types);
> +	return rc;
> +}
> +
> +static int cil_create_attribute_d1_and_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
> +{
> +	struct cil_list *str_expr;
> +	struct cil_list *datum_expr;
> +	ebitmap_t *types;
> +	int rc;
> +
> +	if (d1 == d2) {
> +		*d3 = d1;
> +		return SEPOL_OK;
> +	}
> +
> +	*d3 = NULL;
> +
> +	str_expr = cil_create_and_expr_list(CIL_STRING, d1->fqn, d2->fqn);
> +	datum_expr = cil_create_and_expr_list(CIL_DATUM, d1, d2);
> +
> +	types = cil_malloc(sizeof(*types));
> +	rc = cil_datums_and(types, d1, d2);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}
> +	if (ebitmap_is_empty(types)) {
> +		rc = SEPOL_OK;
> +		goto exit;
> +	}
> +
> +	if (ebitmap_cardinality(types) == 1) {
> +		unsigned i = ebitmap_highest_set_bit(types);
> +		*d3 = DATUM(db->val_to_type[i]);
> +		ebitmap_destroy(types);
> +		rc = SEPOL_OK;
> +		goto exit;
> +	}
> +
> +	rc = cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}

This and the and_not variant above differ in this error handling.  The 
and_not one trusts cil_create_and_insert_attribute_and_set() to do the 
cleanup, and this one does its own cleanup.

I think they're both potentially wrong, as 
cil_create_and_insert_attribute_and_set() calls 
cil_destroy_typeattributeset() which does the same cleanup only if it 
makes it halfway through the function.  So if 
cil_create_new_attribute_name() fails, then this case works correctly 
and the and_not case doesn't free str_expr and datum_expr.  On the other 
hand, if something later in cil_create_and_insert_attribute_and_set() 
fails, then the and_not case is correct, and this one will double free 
str_expr and datum_expr.

> +
> +	return SEPOL_OK;
> +
> +exit:
> +	cil_list_destroy(&str_expr, CIL_FALSE);
> +	cil_list_destroy(&datum_expr, CIL_FALSE);
> +	free(types);
> +	return rc;
> +}
> +
> +static struct cil_avrule *cil_create_avrule(struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
> +{
> +	struct cil_avrule *new;
> +
> +	cil_avrule_init(&new);
> +	new->is_extended = CIL_FALSE;
> +	new->rule_kind = CIL_AVRULE_ALLOWED;
> +	new->src_str = src->name;
> +	new->src = src;
> +	new->tgt_str = tgt->name;
> +	new->tgt = tgt;
> +	new->perms.classperms = classperms;
> +
> +	return new;
> +}
> +
> +static struct cil_tree_node *cil_create_and_add_avrule(struct cil_tree_node *curr, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
> +{
> +	struct cil_avrule *new_avrule;
> +
> +	new_avrule = cil_create_avrule(src, tgt, classperms);
> +	return cil_create_and_insert_node(curr, CIL_AVRULE, new_avrule);
> +}
> +
> +static int cil_remove_permissions_from_rule(struct cil_db *db, struct cil_tree_node *allow_node, const struct cil_tree_node *deny_node)
> +{
> +	struct cil_avrule *allow_rule = allow_node->data;
> +	struct cil_deny_rule *deny_rule = deny_node->data;
> +	struct cil_symtab_datum *s1 = allow_rule->src;
> +	struct cil_symtab_datum *t1 = allow_rule->tgt;
> +	struct cil_list *p1 = allow_rule->perms.classperms;
> +	struct cil_symtab_datum *s2 = deny_rule->src;
> +	struct cil_symtab_datum *t2 = deny_rule->tgt;
> +	struct cil_list *p2 = deny_rule->classperms;
> +	struct cil_list *p3 = NULL;
> +	struct cil_tree_node *curr = allow_node;
> +	int rc;
> +
> +	cil_classperms_list_andnot(&p3, p1, p2);
> +
> +	if (!cil_list_is_empty(p3)) {;
> +		curr = cil_create_and_add_avrule(curr, s1, t1, p3);
> +	} else {
> +		cil_destroy_classperms_list(&p3);
> +		p3 = NULL;
> +	}
> +
> +	if (FLAVOR(s1) == CIL_TYPEATTRIBUTE) {
> +		struct cil_symtab_datum *s3 = NULL;
> +		struct cil_symtab_datum *d = NULL > +		struct cil_list *cp = NULL;
> +		if (t1->fqn == CIL_KEY_SELF && t2->fqn != CIL_KEY_SELF) {
> +			/* S3 = S1 and not (S2 and T2) */
> +			rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t2, &d);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +			if (d) {
> +				rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
> +				if (rc != SEPOL_OK) {
> +					goto exit;
> +				}
> +			}
> +		} else if (t1->fqn != CIL_KEY_SELF && t2->fqn == CIL_KEY_SELF) {
> +			/* S3 = S1 and not (T1 and S2) */
> +			rc = cil_create_attribute_d1_and_d2(db, NODE(t1), t1, s2, &d);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +			if (d) {
> +				rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
> +				if (rc != SEPOL_OK) {
> +					goto exit;
> +				}
> +			}
> +		} else {
> +			/* S3 = S1 and not S2 */
> +			rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, s2, &s3);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +		}
> +		if (s3) {
> +			cil_classperms_list_copy(&cp, p1);
> +			curr = cil_create_and_add_avrule(curr, s3, t1, cp);
> +		}
> +	}
> +
> +	if (t1->fqn != CIL_KEY_SELF && FLAVOR(t1) == CIL_TYPEATTRIBUTE) {
> +		struct cil_symtab_datum *t3 = NULL;
> +		struct cil_symtab_datum *d = NULL;
> +		struct cil_list *cp = NULL;
> +		if (t2->fqn == CIL_KEY_SELF) {
> +			/* T3 = T1 and not (S1 AND S2) */
> +			rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, s2, &d);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +			if (d) {
> +				rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, d, &t3);
> +				if (rc != SEPOL_OK) {
> +					goto exit;
> +				}
> +			}
> +		} else {
> +			/* S3 = T1 and not T2 */
> +			rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, t2, &t3);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +		}
> +		if (t3) {
> +			cil_classperms_list_copy(&cp, p1);
> +			curr = cil_create_and_add_avrule(curr, s1, t3, cp);
> +		}
> +
> +		if (FLAVOR(s1) == CIL_TYPEATTRIBUTE && FLAVOR(s2) == CIL_TYPEATTRIBUTE &&
> +			t2->fqn == CIL_KEY_SELF) {
> +			/* Really special case */
> +			/* FLAVOR(t1) == CIL_TYPEATTRIBUTE */
> +			/* SX = S1 and S2 and T1 */
> +			struct cil_symtab_datum *sx = NULL;
> +			struct cil_typeattribute *sx_attr = NULL;
> +			
> +			d = NULL;
> +			cp = NULL;
> +
> +			rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t1, &d);
> +			if (rc != SEPOL_OK) {
> +				goto exit;
> +			}
> +			if (d) {
> +				rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, d, &sx);
> +				if (rc != SEPOL_OK) {
> +					goto exit;
> +				}
> +				sx_attr = (struct cil_typeattribute *)sx;
> +			}
> +			if (sx && ebitmap_cardinality(sx_attr->types) > 1) {
> +				struct cil_symtab_datum *s = NULL;
> +				struct cil_symtab_datum *t = NULL;
> +				ebitmap_node_t *n;
> +				unsigned i;
> +				ebitmap_for_each_positive_bit(sx_attr->types, n, i) {
> +					s = DATUM(db->val_to_type[i]);
> +					rc = cil_create_attribute_d1_and_not_d2(db, NODE(sx), sx, s, &t);
> +					if (rc != SEPOL_OK) {
> +						goto exit;
> +					}
> +					if (t) {
> +						cil_classperms_list_copy(&cp, p1);
> +						curr = cil_create_and_add_avrule(curr, s, t, cp);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +exit:
> +	return rc;
> +}

As I mentioned briefly back in December, it might be in the attributes 
case to only grant p2 rather than p3.  I *think* that's safe, since the 
initial allow rule grants everything p3, so the only things needed to be 
added back in the attributes are the denied permissions (p2).

> +
> +static int cil_find_matching_allow_rules(struct cil_list *matching, struct cil_tree_node *start, struct cil_tree_node *deny_node)
> +{
> +	struct cil_deny_rule *deny_rule = deny_node->data;
> +	struct cil_avrule target;
> +
> +	target.rule_kind = CIL_AVRULE_ALLOWED;
> +	target.is_extended = CIL_FALSE;
> +	target.src = deny_rule->src;
> +	target.tgt = deny_rule->tgt;
> +	target.perms.classperms = deny_rule->classperms;
> +
> +	return cil_find_matching_avrule_in_ast(start, CIL_AVRULE, &target, matching, CIL_FALSE);
> +}
> +
> +static int cil_process_deny_rule(struct cil_db *db, struct cil_tree_node *start, struct cil_tree_node *deny_node)
> +{
> +	struct cil_list *matching;
> +	struct cil_list_item *item;
> +	int rc;
> +
> +	cil_list_init(&matching, CIL_NODE);
> +
> +	rc = cil_find_matching_allow_rules(matching, start, deny_node);
> +	if (rc != SEPOL_OK) {
> +		goto exit;
> +	}
> +
> +	cil_list_for_each(item, matching) {
> +		struct cil_tree_node *allow_node = item->data;
> +		rc = cil_remove_permissions_from_rule(db, allow_node, deny_node);
> +		cil_tree_remove_node(allow_node);
> +		cil_tree_node_destroy(&allow_node);
> +		if (rc != SEPOL_OK) {
> +			goto exit;
> +		}
> +
> +	}
> +
> +exit:
> +	cil_list_destroy(&matching, CIL_FALSE);
> +	return rc;
> +}
> +
> +static int cil_process_deny_rules(struct cil_db *db, struct cil_tree_node *start, struct cil_list *deny_rules)
> +{
> +	struct cil_list_item *item;
> +	int rc;
> +
> +	cil_list_for_each(item, deny_rules) {
> +		struct cil_tree_node *deny_node = item->data;
> +		rc = cil_process_deny_rule(db, start, deny_node);
> +		if (rc != SEPOL_OK) {
> +			goto exit;
> +		}
> +		cil_tree_remove_node(deny_node);
> +		cil_tree_node_destroy(&deny_node);
> +	}
> +
> +exit:
> +	return rc;
> +}
> +
> +static int __cil_find_deny_rules(struct cil_tree_node *node,  uint32_t *finished, void *extra_args)
> +{
> +	struct cil_list *deny_rules = extra_args;
> +
> +	if (node->flavor == CIL_BLOCK) {
> +		struct cil_block *block = node->data;
> +		if (block->is_abstract == CIL_TRUE) {
> +			*finished = CIL_TREE_SKIP_HEAD;
> +		}
> +	} else if (node->flavor == CIL_MACRO) {
> +		*finished = CIL_TREE_SKIP_HEAD;
> +	} else if (node->flavor == CIL_DENY_RULE) {
> +		cil_list_append(deny_rules, CIL_DENY_RULE, node);
> +	}
> +	return SEPOL_OK;
> +}
> +
> +int cil_process_deny_rules_in_ast(struct cil_db *db)
> +{
> +	struct cil_tree_node *start;
> +	struct cil_list *deny_rules;
> +	int rc = SEPOL_ERR;
> +
> +	cil_list_init(&deny_rules, CIL_DENY_RULE);
> +
> +	if (!db) {
> +		cil_log(CIL_ERR, "No CIL db provided to process deny rules\n");
> +		goto exit;
> +	}
> +
> +	start = db->ast->root;
> +	rc = cil_tree_walk(start, __cil_find_deny_rules, NULL, NULL, deny_rules);
> +	if (rc != SEPOL_OK) {
> +		cil_log(CIL_ERR, "An error occurred while getting deny rules\n");
> +		goto exit;
> +	}
> +
> +	rc = cil_process_deny_rules(db, start, deny_rules);
> +	if (rc != SEPOL_OK) {
> +		cil_log(CIL_ERR, "An error occurred while processing deny rules\n");
> +		goto exit;
> +	}
> +
> +exit:
> +	cil_list_destroy(&deny_rules, CIL_FALSE);
> +	return rc;
> +}
> diff --git a/libsepol/cil/src/cil_deny.h b/libsepol/cil/src/cil_deny.h
> new file mode 100644
> index 00000000..4a9e92d1
> --- /dev/null
> +++ b/libsepol/cil/src/cil_deny.h
> @@ -0,0 +1,34 @@
> +/*
> + * This file is public domain software, i.e. not copyrighted.
> + *
> + * Warranty Exclusion
> + * ------------------
> + * You agree that this software is a non-commercially developed program
> + * that may contain "bugs" (as that term is used in the industry) and
> + * that it may not function as intended. The software is licensed
> + * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
> + * express, implied, statutory, or otherwise with respect to the software,
> + * including noninfringement and the implied warranties of merchantability
> + * and fitness for a particular purpose.
> + *
> + * Limitation of Liability
> + *-----------------------
> + * In no event will NSA be liable for any damages, including loss of data,
> + * lost profits, cost of cover, or other special, incidental, consequential,
> + * direct or indirect damages arising from the software or the use thereof,
> + * however caused and on any theory of liability. This limitation will apply
> + * even if NSA has been advised of the possibility of such damage. You
> + * acknowledge that this is a reasonable allocation of risk.
> + *
> + * Original author: James Carter
> + */
> +
> +#ifndef CIL_DENY_H_
> +#define CIL_DENY_H_
> +
> +int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2);
> +void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old);
> +void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2);
> +int cil_process_deny_rules_in_ast(struct cil_db *db);
> +
> +#endif /* CIL_DENY_H_ */
> diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
> index 11e572e2..fb7eec2d 100644
> --- a/libsepol/cil/src/cil_post.c
> +++ b/libsepol/cil/src/cil_post.c
> @@ -47,6 +47,7 @@
>   #include "cil_policy.h"
>   #include "cil_verify.h"
>   #include "cil_symtab.h"
> +#include "cil_deny.h"
>   
>   #define GEN_REQUIRE_ATTR "cil_gen_require" /* Also in libsepol/src/module_to_cil.c */
>   #define TYPEATTR_INFIX "_typeattr_"        /* Also in libsepol/src/module_to_cil.c */
> @@ -2551,6 +2552,12 @@ int cil_post_process(struct cil_db *db)
>   		goto exit;
>   	}
>   
> +	rc = cil_process_deny_rules_in_ast(db);
> +	if (rc != SEPOL_OK) {
> +		cil_log(CIL_ERR, "Failed to process deny rules\n");
> +		goto exit;
> +	}
> +
>   	rc = cil_post_verify(db);
>   	if (rc != SEPOL_OK) {
>   		cil_log(CIL_ERR, "Failed to verify cil database\n");


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 8/9] secilc/test: Add a deny rule test
  2022-12-15 21:34 ` [RFC PATCH 8/9] secilc/test: Add a deny rule test James Carter
@ 2023-02-03 22:54   ` Daniel Burgener
  2023-02-09 14:31     ` James Carter
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Burgener @ 2023-02-03 22:54 UTC (permalink / raw)
  To: James Carter, selinux

On 12/15/2022 4:34 PM, James Carter wrote:
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>   secilc/test/deny_rule_test.cil | 384 +++++++++++++++++++++++++++++++++
>   1 file changed, 384 insertions(+)
>   create mode 100644 secilc/test/deny_rule_test.cil
> 
> diff --git a/secilc/test/deny_rule_test.cil b/secilc/test/deny_rule_test.cil
> new file mode 100644
> index 00000000..3ef4ac98
> --- /dev/null
> +++ b/secilc/test/deny_rule_test.cil
> @@ -0,0 +1,384 @@
> +(class CLASS (PERM))
> +(class C1 (p1a p1b p1c p1d))
> +(class C2 (p2a p2b p2c p2d))
> +(class C3 (p3a p3b p3c p3d))
> +(class C4 (p4a p4b p4c p4d))
> +(classorder (CLASS C1 C2 C3 C4))
> +(sid SID)
> +(sidorder (SID))
> +(user USER)
> +(role ROLE)
> +(type TYPE)
> +(category CAT)
> +(categoryorder (CAT))
> +(sensitivity SENS)
> +(sensitivityorder (SENS))
> +(sensitivitycategory SENS (CAT))
> +(allow TYPE self (CLASS (PERM)))
> +(roletype ROLE TYPE)
> +(userrole USER ROLE)
> +(userlevel USER (SENS))
> +(userrange USER ((SENS)(SENS (CAT))))
> +(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
> +
> +(classmap cm5 (mp5a mp5b))
> +(classmapping cm5 mp5a
> +	      (C2 (p2a p2b)))
> +(classmapping cm5 mp5b
> +	      (C2 (p2c p2d)))
> +
> +(classpermission cp6)
> +(classpermissionset cp6 (C3 (p3a p3b)))
> +(classpermissionset cp6 (C4 (p4a p4b)))
> +
> +(classpermission cp7)
> +(classpermissionset cp7 (C2 (p2a p2b)))
> +(classpermissionset cp7 (C2 (p2c p2d)))
> +
> +; Test 1
> +(type t01a)
> +(type t01b)
> +(allow t01a t01b (C1 (p1a)))
> +(deny t01a t01b (C1 (p1a)))
> +(neverallow t01a t01b (C1 (p1a)))
> +
> +; Test 2
> +(type t02a)
> +(type t02b)
> +(allow t02a t02b (C1 (p1a p1b)))
> +(deny t02a t02b (C1 (p1a)))
> +(neverallow t02a t02b (C1 (p1a)))
> +; (neverallow t02a t02b (C1 (p1b))) ; This check should fail
> +
> +; Test 3
> +(type t03a)
> +(type t03b)
> +(allow t03a t03b (C1 (p1a)))
> +(deny t03a t03b (C1 (p1a p1b)))
> +(neverallow t03a t03b (C1 (p1a p1b)))
> +
> +
> +; Test 11
> +(type t11a)
> +(type t11b)
> +(type t11c)
> +(type t11d)
> +(typeattribute a11a)
> +(typeattribute a11b)
> +(typeattributeset a11a (t11a t11b))
> +(typeattributeset a11b (t11c t11d))
> +(allow a11a a11b (C1 (p1a)))
> +(deny a11a a11b (C1 (p1a)))
> +(neverallow a11a a11b (C1 (p1a)))
> +
> +; Test 12
> +(type t12a)
> +(type t12b)
> +(type t12c)
> +(type t12d)
> +(typeattribute a12a)
> +(typeattribute a12b)
> +(typeattributeset a12a (t12a t12b))
> +(typeattributeset a12b (t12c t12d))
> +(allow t12a t12c (C1 (p1a)))
> +(deny a12a a12b (C1 (p1a)))
> +(neverallow a12a a12b (C1 (p1a)))
> +
> +; Test 13
> +(type t13a)
> +(type t13b)
> +(type t13c)
> +(type t13d)
> +(typeattribute a13a)
> +(typeattribute a13b)
> +(typeattributeset a13a (t13a t13b))
> +(typeattributeset a13b (t13c t13d))
> +(allow a13a a13b (C1 (p1a)))
> +(deny t13a t13c (C1 (p1a)))
> +(neverallow t13a t13c (C1 (p1a)))
> +; (neverallow t13b t13d (C1 (p1a))) ; This check should fail
> +
> +; Test 21
> +(type t21a)
> +(type t21b)
> +(allow t21a t21b (cm5 (mp5a)))
> +(deny t21a t21b (cm5 (mp5a)))
> +(neverallow t21a t21b (cm5 (mp5a)))
> +
> +; Test 22
> +(type t22a)
> +(type t22b)
> +(allow t22a t22b (cm5 (mp5a mp5b)))
> +(deny t22a t22b (cm5 (mp5a)))
> +(neverallow t22a t22b (cm5 (mp5a)))
> +; (neverallow t22a t22b (cm5 (mp5b))) ; This check should fail
> +
> +; Test 23
> +(type t23a)
> +(type t23b)
> +(allow t23a t23b (cm5 (mp5a)))
> +(deny t23a t23b (cm5 (mp5a mp5b)))
> +(neverallow t23a t23b (cm5 (mp5a mp5b)))
> +
> +; Test 24
> +(type t24a)
> +(type t24b)
> +(allow t24a t24b (C2 (p2a)))
> +(deny t24a t24b (cm5 (mp5a)))
> +(neverallow t24a t24b (cm5 (mp5a)))
> +
> +; Test 25
> +(type t25a)
> +(type t25b)
> +(allow t25a t25b (cm5 (mp5a)))
> +(deny t25a t25b (C2 (p2a)))
> +(neverallow t25a t25b (C2 (p2a)))
> +; (neverallow t25a t25b (C2 (p2b))) ; This check should fail
> +
> +; Test 31
> +(type t31a)
> +(type t31b)
> +(allow t31a t31b cp6)
> +(deny t31a t31b cp6)
> +(neverallow t31a t31b cp6)
> +
> +; Test 32
> +(type t32a)
> +(type t32b)
> +(allow t32a t32b cp6)
> +(deny t32a t32b (C3 (p3a p3b)))
> +(neverallow t32a t32b (C3 (p3a p3b)))
> +; (neverallow t32a t32b (C4 (p4a p4b))) ; This check should fail
> +
> +; Test 33
> +(type t33a)
> +(type t33b)
> +(allow t33a t33b (C3 (p3a)))
> +(deny t33a t33b cp6)
> +(neverallow t33a t33b cp6)
> +
> +; Test 34
> +(type t34a)
> +(type t34b)
> +(allow t34a t34b cp7)
> +(deny t34a t34b (cm5 (mp5a mp5b)))
> +(neverallow t34a t34b (cm5 (mp5a mp5b)))
> +
> +; Test 35
> +(type t35a)
> +(type t35b)
> +(allow t35a t35b (cm5 (mp5a mp5b)))
> +(deny t35a t35b cp7)
> +(neverallow t35a t35b cp7)
> +
> +; Test 41
> +(type t41a)
> +(allow t41a self (C1 (p1a)))
> +(deny t41a self (C1 (p1a)))
> +(neverallow t41a self (C1 (p1a)))
> +
> +; Test 42
> +(type t42a)
> +(allow t42a self (C1 (p1a)))
> +(deny t42a t42a (C1 (p1a)))
> +(neverallow t42a t42a (C1 (p1a)))
> +
> +; Test 43
> +(type t43a)
> +(allow t43a t43a (C1 (p1a)))
> +(deny t43a self (C1 (p1a)))
> +(neverallow t43a self (C1 (p1a)))
> +
> +; Test 51
> +(type t51a)
> +(type t51b)
> +(typeattribute a51a)
> +(typeattributeset a51a (t51a t51b))
> +(allow a51a self (C1 (p1a)))
> +(deny a51a self (C1 (p1a)))
> +(neverallow a51a self (C1 (p1a)))
> +
> +; Test 52
> +(type t52a)
> +(type t52b)
> +(typeattribute a52a)
> +(typeattributeset a52a (t52a t52b))
> +(allow t52a self (C1 (p1a)))
> +(deny a52a self (C1 (p1a)))
> +(neverallow a52a self (C1 (p1a)))
> +
> +; Test 53
> +(type t53a)
> +(type t53b)
> +(typeattribute a53a)
> +(typeattributeset a53a (t53a t53b))
> +(allow a53a self (C1 (p1a)))
> +(deny t53a self (C1 (p1a)))
> +(neverallow t53a self (C1 (p1a)))
> +; (neverallow t53b self (C1 (p1a))) ; This check should fail
> +
> +; Test 54
> +(type t54a)
> +(type t54b)
> +(typeattribute a54a)
> +(typeattributeset a54a (t54a t54b))
> +(allow a54a self (C1 (p1a)))
> +(deny a54a a54a (C1 (p1a)))
> +(neverallow a54a a54a (C1 (p1a)))
> +
> +; Test 55
> +(type t55a)
> +(type t55b)
> +(typeattribute a55a)
> +(typeattributeset a55a (t55a t55b))
> +(allow a55a a55a (C1 (p1a)))
> +(deny a55a self (C1 (p1a)))
> +(neverallow a55a self (C1 (p1a)))
> +; (neverallow t55a t55b (C1 (p1a))) ; This check should fail
> +; (neverallow t55b t55a (C1 (p1a))) ; This check should fail
> +
> +; Test 61
> +(type t61a)
> +(type t61b)
> +(type t61c)
> +(type t61d)
> +(type t61e)
> +(type t61f)
> +(type t61g)
> +(type t61h)
> +(type t61i)
> +(type t61j)
> +(type t61k)
> +(type t61l)
> +(type t61m)
> +(type t61n)
> +(type t61o)
> +(type t61p)
> +(type t61q)
> +(type t61r)
> +(type t61s)
> +(type t61t)
> +(typeattribute a61a)
> +(typeattribute a61b)
> +(typeattribute a61c)
> +(typeattribute a61x)
> +(typeattribute a61s)
> +(typeattribute a61t)
> +(typeattributeset a61a (t61a t61b t61c t61d t61e t61f t61g t61h t61o t61p))
> +(typeattributeset a61b (t61a t61b t61c t61d t61i t61j t61k t61l t61q t61r))
> +(typeattributeset a61c (t61a t61b t61e t61f t61i t61j t61m t61n t61s t61t))
> +(typeattributeset a61x (t61a t61b))
> +(typeattributeset a61s (t61c t61d t61e t61f t61g t61h t61o t61p))
> +(typeattributeset a61t (t61c t61d t61i t61j t61k t61l t61q t61r))

Was the intention to have a failing check on a61x below?  I don't see 
a61x referenced anywhere except where it was declared and set.

-Daniel

> +(allow a61a a61b (C1 (p1a)))
> +(deny a61c self (C1 (p1a)))
> +(neverallow a61c self (C1 (p1a)))
> +; (neverallow a61s a61b (C1 (p1a))) ; This check should fail
> +; (neverallow a61a a61t (C1 (p1a))) ; This check should fail
> +; (neverallow t61a t61b (C1 (p1a))) ; This check should fail
> +; (neverallow t61b t61a (C1 (p1a))) ; This check should fail
> +
> +; Test 62
> +(type t62a)
> +(type t62b)
> +(type t62c)
> +(type t62d)
> +(type t62e)
> +(type t62f)
> +(type t62g)
> +(type t62h)
> +(type t62i)
> +(type t62j)
> +(type t62k)
> +(type t62l)
> +(type t62m)
> +(type t62n)
> +(type t62o)
> +(type t62p)
> +(type t62s)
> +(type t62t)
> +(type t62u)
> +(type t62v)
> +(typeattribute a62a)
> +(typeattribute a62c)
> +(typeattribute a62d)
> +(typeattribute a62s)
> +(typeattributeset a62a (t62a t62b t62c t62d t62e t62f t62g t62h t62o t62p))
> +(typeattributeset a62c (t62a t62b t62e t62f t62i t62j t62m t62n t62s t62t))
> +(typeattributeset a62d (t62a t62b t62g t62h t62k t62l t62m t62n t62u t62v))
> +(typeattributeset a62s (t62c t62d t62e t62f t62g t62h t62o t62p))
> +(allow a62a self (C1 (p1a)))
> +(deny a62c a62d (C1 (p1a)))
> +(neverallow a62c a62d (C1 (p1a)))
> +; (neverallow a62s self (C1 (p1a))) ; This check should fail
> +
> +; Test 63
> +(type t63a)
> +(type t63b)
> +(type t63c)
> +(type t63d)
> +(type t63e)
> +(type t63f)
> +(type t63g)
> +(type t63h)
> +(type t63i)
> +(type t63j)
> +(type t63k)
> +(type t63l)
> +(type t63m)
> +(type t63n)
> +(type t63o)
> +(type t63p)
> +(type t63s)
> +(type t63t)
> +(typeattribute a63a)
> +(typeattribute a63c)
> +(typeattribute a63s)
> +(typeattributeset a63a (t63a t63b t63c t63d t63e t63f t63g t63h t63o t63p))
> +(typeattributeset a63c (t63a t63b t63e t63f t63i t63j t63m t63n t63s t63t))
> +(typeattributeset a63s (t62c t62d t62e t62f t62g t62h t62o t62p))
> +(allow a63a self (C1 (p1a)))
> +(deny a63c self (C1 (p1a)))
> +(neverallow a63c self (C1 (p1a)))
> +; (neverallow a63s self (C1 (p1a))) ; This check should fail
> +
> +; Test 64
> +(type t64a)
> +(type t64b)
> +(type t64c)
> +(type t64d)
> +(type t64e)
> +(type t64f)
> +(type t64g)
> +(type t64h)
> +(type t64i)
> +(type t64j)
> +(type t64k)
> +(type t64l)
> +(type t64m)
> +(type t64n)
> +(type t64o)
> +(type t64p)
> +(type t64q)
> +(type t64r)
> +(type t64s)
> +(type t64t)
> +(type t64u)
> +(type t64v)
> +(typeattribute a64a)
> +(typeattribute a64b)
> +(typeattribute a64c)
> +(typeattribute a64d)
> +(typeattribute a64s)
> +(typeattribute a64t)
> +(typeattributeset a64a (t64a t64b t64c t64d t64e t64f t64g t64h t64o t64p))
> +(typeattributeset a64b (t64a t64b t64c t64d t64i t64j t64k t64l t64q t64r))
> +(typeattributeset a64c (t64a t64b t64e t64f t64i t64j t64m t64n t64s t64t))
> +(typeattributeset a64d (t64a t64b t64g t64h t64k t64l t64m t64n t64u t64v))
> +(typeattributeset a64s (t64c t64d t64g t64h t64o t64p))
> +(typeattributeset a64t (t64c t64d t64i t64j t64q t64r))
> +(allow a64a a64b (C1 (p1a)))
> +(deny a64c a64d (C1 (p1a)))
> +(neverallow a64c a64d (C1 (p1a)))
> +; (neverallow a64s a64b (C1 (p1a))) ; This check should fail
> +; (neverallow a64a a64t (C1 (p1a))) ; This check should fail


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation
  2022-12-15 21:34 ` [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation James Carter
@ 2023-02-03 22:55   ` Daniel Burgener
  2023-02-09 14:39     ` James Carter
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Burgener @ 2023-02-03 22:55 UTC (permalink / raw)
  To: James Carter, selinux

On 12/15/2022 4:34 PM, James Carter wrote:
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>   secilc/docs/cil_access_vector_rules.md | 68 ++++++++++++++++++++++++++
>   1 file changed, 68 insertions(+)
> 
> diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
> index f0ba4a90..35825283 100644
> --- a/secilc/docs/cil_access_vector_rules.md
> +++ b/secilc/docs/cil_access_vector_rules.md
> @@ -247,6 +247,74 @@ This example will not compile as `type_3` is not allowed to be a source type for
>           (allow type_3 self (property_service (set)))
>       )
>   ```
> +deny
> +----------
> +
> +Remove the access rights defined from any matching allow rules. These rules are processed before [`neverallow`](cil_access_vector_rules.md#neverallow) checking.
> +
> +**Rule definition:**
> +
> +```secil
> +    (deny source_id target_id|self classpermissionset_id ...)
> +```
> +
> +**Where:**
> +
> +<table>
> +<colgroup>
> +<col width="27%" />
> +<col width="72%" />
> +</colgroup>
> +<tbody>
> +<tr class="odd">
> +<td align="left"><p><code>deny</code></p></td>
> +<td align="left"><p>The <code>deny</code> keyword.</p></td>
> +</tr>
> +<tr class="even">
> +<td align="left"><p><code>source_id</code></p></td>
> +<td align="left"><p>A single previously defined source <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p></td>
> +</tr>
> +<tr class="odd">
> +<td align="left"><p><code>target_id</code></p></td>
> +<td align="left"><p>A single previously defined target <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p>
> +<p>The <code>self</code> keyword may be used instead to signify that source and target are the same.</p></td>
> +</tr>
> +<tr class="even">
> +<td align="left"><p><code>classpermissionset_id</code></p></td>
> +<td align="left"><p>A single named or anonymous <code>classpermissionset</code> or a single set of <code>classmap</code>/<code>classmapping</code> identifiers.</p></td>
> +</tr>
> +</tbody>
> +</table>
> +
> +**Example:**
> +
> +```secil
> +    (class class1 (perm1 perm2))
> +
> +	(type type_1)
> +    (type type_2)
> +	(allow type_1 type_2 (class1 (perm1))) ; Allow_1
> +	(deny type_1 type_2 (class1 (perm1)))  ; Deny_1
> +  	; Allow_1 will be complete removed by Deny_1.
> +
> +    (type type_3)
> +	(type type_4)
> +	(allow type_3 type_4 (class1 (perm1 perm2))) ; Allow_2
> +	(deny type_3 type_4 (class1 (perm1)))        ; Deny_2
> +	; Allow_2 will be removed and replaced with the following when Deny_2 is evaluated
> +    ; (allow type_3 type_4 (class1 (perm2)))
> +
> +	(type type_5)
> +	(type type_6)
> +	(typeattribute attr_1)
> +	(typeattributeset attr_1 (type_5 type_6))
> +	(allow attr_1 attr_1 (class1 (perm1))) ; Allow_3
> +	(deny type_5 type_6 (class1 (perm1)))  ; Deny_3
> +	; Allow_3 will be removed and replaced with the following when Deny_3 is evaluated
> +	; (allow type_6 attr_1 (class1 (perm1)))
> +	; (allow attr_1 type_5 (class1 (perm1)))
> +    )
> +```

Looks like theres some intermixing of spaces and tabs messing up 
formatting on the example.

-Daniel
>   
>   allowx
>   ------


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function
  2023-02-03 22:54   ` Daniel Burgener
@ 2023-02-08 21:09     ` James Carter
  0 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2023-02-08 21:09 UTC (permalink / raw)
  To: Daniel Burgener; +Cc: selinux

On Fri, Feb 3, 2023 at 5:54 PM Daniel Burgener
<dburgener@linux.microsoft.com> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > Add the function cil_tree_remove_node() which takes a node pointer
> > as an input, finds the parent, walks the list of nodes to the node
> > prior to the given node, and then updates that nodes next pointer
> > to remove the given node from the tree.
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> >   libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++
> >   libsepol/cil/src/cil_tree.h |  1 +
> >   2 files changed, 28 insertions(+)
> >
> > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> > index 6376c208..73b4e135 100644
> > --- a/libsepol/cil/src/cil_tree.c
> > +++ b/libsepol/cil/src/cil_tree.c
> > @@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node)
> >       *node = NULL;
> >   }
> >
> > +void cil_tree_remove_node(struct cil_tree_node *node)
> > +{
> > +     struct cil_tree_node *parent, *curr;
> > +
> > +     if (node == NULL || node->parent == NULL) {
> > +             return;
> > +     }
> > +
> > +     parent = node->parent;
> > +
> > +     if (parent->cl_head == node) {
> > +             parent->cl_head = node->next;
> > +             return;
> > +     }
> > +
> > +     curr = parent->cl_head;
> > +     while (curr && curr->next != node) {
> > +             curr = curr->next;
> > +     }
> > +
> > +     if (curr == NULL) {
> > +             return;
> > +     }
> > +
> > +     curr->next = node->next;
> > +}
> > +
>
> Is there a reason this leaves it to the caller to call
> cil_tree_node_destroy()?  It just feels a little weird to leave the node
> unreachable without freeing.  It looks like both callers in the series
> (cil_process_deny_rule(s)) call cil_tree_node_destroy() immediately after.
>
> -Daniel
>

Not that I can remember. I can't really think of a scenario where I
would want to remove a node, but not destroy it.
It also seems like it would be better to name it
cil_tree_node_remove() to fit the naming scheme of the other
functions.
Thanks,
Jim


> >   /* Perform depth-first walk of the tree
> >      Parameters:
> >      start_node:          root node to start walking from
> > diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
> > index 5a98da55..e4b3fd04 100644
> > --- a/libsepol/cil/src/cil_tree.h
> > +++ b/libsepol/cil/src/cil_tree.h
> > @@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node);
> >
> >   void cil_tree_node_init(struct cil_tree_node **node);
> >   void cil_tree_node_destroy(struct cil_tree_node **node);
> > +void cil_tree_remove_node(struct cil_tree_node *node);
> >
> >   //finished values
> >   #define CIL_TREE_SKIP_NOTHING       0
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 4/9] libsepol/cil: Process deny rules
  2023-02-03 22:54   ` Daniel Burgener
@ 2023-02-08 21:57     ` James Carter
  0 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2023-02-08 21:57 UTC (permalink / raw)
  To: Daniel Burgener; +Cc: selinux

On Fri, Feb 3, 2023 at 5:54 PM Daniel Burgener
<dburgener@linux.microsoft.com> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > A deny rule is like a neverallow rule, except that permissions are
> > removed rather than an error reported.
> >
> > (allow S1 T1 P1)
> > (deny  S2 T2 P2)
> >
> > In general what is needed is:
> > 1) Allow permissions that do not match the deny rule
> >    P3 = P1 and not P2
> >    (allow S1 T1 P3)
> > 2) Allow source types that do not match the deny rule
> >    S3 = S1 and not S2
> >    (allow S3 T1 P1)
> > 3) Allow target types that do not match the deny rule
> >    T3 = T1 and not T2
> >    (allow S1 T3 P1)
> >
> > If a self rule is involved then
> >    If S1 is an attribute then
> >      If T1 is self and T2 is not self then
> >        S3 = S1 and not (S2 and T2)
> >      else if T1 is not self and T2 is self then
> >        S3 = S1 and not (T1 and S2)
> >      else if T1 is self and T2 is self then
> >        S3 = S1 and not S2
> >      (allow S3 T1 P1)
> >    If T1 is an attribute then
> >      If T2 is self then
> >        T3 = T1 and not (S1 and S2)
> >      (allow S1 T3 P1)
> >      If S1 is attribute and S2 is attribute and T2 is self then
> >        # In addition to any rules involving S3 and T3 above
> >        SX = S1 and S2 and T1
> >        For S in SX do
> >          T = SX and not S
> >          (allow S T P1)
> >
> > In all cases, do not create a rule that has an empty set
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> >   libsepol/cil/src/cil_deny.c | 957 ++++++++++++++++++++++++++++++++++++
> >   libsepol/cil/src/cil_deny.h |  34 ++
> >   libsepol/cil/src/cil_post.c |   7 +
> >   3 files changed, 998 insertions(+)
> >   create mode 100644 libsepol/cil/src/cil_deny.c
> >   create mode 100644 libsepol/cil/src/cil_deny.h
> >
> > diff --git a/libsepol/cil/src/cil_deny.c b/libsepol/cil/src/cil_deny.c
> > new file mode 100644
> > index 00000000..43043c37
> > --- /dev/null
> > +++ b/libsepol/cil/src/cil_deny.c
> > @@ -0,0 +1,957 @@
> > +/*
> > + * This file is public domain software, i.e. not copyrighted.
> > + *
> > + * Warranty Exclusion
> > + * ------------------
> > + * You agree that this software is a non-commercially developed program
> > + * that may contain "bugs" (as that term is used in the industry) and
> > + * that it may not function as intended. The software is licensed
> > + * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
> > + * express, implied, statutory, or otherwise with respect to the software,
> > + * including noninfringement and the implied warranties of merchantability
> > + * and fitness for a particular purpose.
> > + *
> > + * Limitation of Liability
> > + *-----------------------
> > + * In no event will NSA be liable for any damages, including loss of data,
> > + * lost profits, cost of cover, or other special, incidental, consequential,
> > + * direct or indirect damages arising from the software or the use thereof,
> > + * however caused and on any theory of liability. This limitation will apply
> > + * even if NSA has been advised of the possibility of such damage. You
> > + * acknowledge that this is a reasonable allocation of risk.
> > + *
> > + * Original author: James Carter
> > + */
> > +
> > +#include <sepol/policydb/ebitmap.h>
> > +
> > +#include "cil_internal.h"
> > +#include "cil_find.h"
> > +#include "cil_flavor.h"
> > +#include "cil_list.h"
> > +#include "cil_strpool.h"
> > +#include "cil_log.h"
> > +#include "cil_symtab.h"
> > +#include "cil_build_ast.h"
> > +#include "cil_copy_ast.h"
> > +#include "cil_deny.h"
> > +
> > +#define CIL_DENY_ATTR_PREFIX "deny_rule_attr"
> > +
> > +/*
> > + * A deny rule will remove the specified permissions from the CIL AST.
> > + *
> > + * (allow S1 T1 P1)
> > + * (deny  S2 T2 P2)
> > + *
> > + * In general what is needed is:
> > + * 1) Allow permissions that do not match the deny rule
> > + *   P3 = P1 and not P2
> > + *   (allow S1 T1 P3)
> > + * 2) Allow source types that do not match the deny rule
> > + *   S3 = S1 and not S2
> > + *   (allow S3 T1 P1)
> > + * 3) Allow target types that do not match the deny rule
> > + *   T3 = T1 and not T2
> > + *   (allow S1 T3 P1)
> > + *
> > + * If a self rule is involved then
> > + *   If S1 is an attribute then
> > + *     If T1 is self and T2 is not self then
> > + *       S3 = S1 and not (S2 and T2)
> > + *     else if T1 is not self and T2 is self then
> > + *       S3 = S1 and not (T1 and S2)
> > + *     else if T1 is self and T2 is self then
> > + *       S3 = S1 and not S2
> > + *     (allow S3 T1 P1)
> > + *   If T1 is an attribute then
> > + *     If T2 is self then
> > + *       T3 = T1 and not (S1 and S2)
> > + *     (allow S1 T3 P1)
> > + *     If S1 is attribute and S2 is attribute and T2 is self then
>
> nit: The rest of the lines refer to "an" attribute, and this one drops
> the article.
>
> BTW, thanks for this description of the cases.  It's very helpful
> working through the logic.
>

The logic will be changing. I've been looking into eliminating all the overlap.

> > + *       # In addition to any rules involving S3 and T3 above
> > + *       SX = S1 and S2 and T1
> > + *       For S in SX do
> > + *         T = SX and not S
> > + *         (allow S T P1)
> > + *
> > + * Do not create a rule that has an empty set
> > + */
> > +
> > +static int cil_perm_match(const struct cil_perm *p1, const struct cil_list *pl2)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     cil_list_for_each(curr, pl2) {
> > +             struct cil_perm *p = curr->data;
> > +             if (p == p1) {
> > +                     return CIL_TRUE;
> > +             }
>
> Why are we comparing the pointers rather than the value fields?
>

There is only one struct cil_perm for each permission, so there will
never be a case where the pointers are not equal, but it is the same
permission.
Every declared object in CIL has a struct cil_symtab_datum datum as
its first field. The symbol table points to the datum (which is at the
same address as the enclosing struct).
If you are searching for a declared object, then you only need to
check the pointers.

> > +     }
> > +     return CIL_FALSE;
> > +}
> > +
> > +static int cil_class_perm_match(const struct cil_class *c1, const struct cil_perm *p1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     cil_list_for_each(curr, cpl2) {
> > +             if (curr->flavor == CIL_CLASSPERMS) {
> > +                     struct cil_classperms *cp = curr->data;
> > +                     if (FLAVOR(cp->class) == CIL_CLASS) {
> > +                             if (cp->class == c1) {
> > +                                     if (cil_perm_match(p1, cp->perms)) {
> > +                                             return CIL_TRUE;
> > +                                     }
> > +                             }
> > +                     } else { /* MAP */
> > +                             struct cil_list_item *p;
> > +                             cil_list_for_each(p, cp->perms) {
> > +                                     struct cil_perm *cmp = p->data;
> > +                                     if (cil_class_perm_match(c1, p1, cmp->classperms)) {
> > +                                             return CIL_TRUE;
> > +                                     }
> > +                             }
> > +                     }
> > +             } else { /* SET */
> > +                     struct cil_classperms_set *cp_set = curr->data;
> > +                     struct cil_classpermission *cp = cp_set->set;
> > +                     if (cil_class_perm_match(c1, p1, cp->classperms)) {
> > +                             return CIL_TRUE;
> > +                     }
> > +             }
> > +     }
> > +     return CIL_FALSE;
> > +}
> > +
> > +static int cil_classperms_match(const struct cil_classperms *cp1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     cil_list_for_each(curr, cp1->perms) {
> > +             struct cil_perm *perm = curr->data;
> > +             if (cil_class_perm_match(cp1->class, perm, cpl2)) {
> > +                     return CIL_TRUE;
> > +             }
> > +     }
> > +     return CIL_FALSE;
> > +}
> > +
> > +int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     if (!cpl1 || !cpl2) {
> > +             return (!cpl1 && !cpl2) ? CIL_TRUE : CIL_FALSE;
> > +     }
> > +
> > +     cil_list_for_each(curr, cpl1) {
> > +             if (curr->flavor == CIL_CLASSPERMS) {
> > +                     struct cil_classperms *cp = curr->data;
> > +                     if (FLAVOR(cp->class) == CIL_CLASS) {
> > +                             if (cil_classperms_match(cp, cpl2)) {
> > +                                     return CIL_TRUE;
> > +                             }
> > +                     } else { /* MAP */
> > +                             struct cil_list_item *p;
> > +                             cil_list_for_each(p, cp->perms) {
> > +                                     struct cil_perm *cmp = p->data;
> > +                                     if (cil_classperms_list_match(cmp->classperms, cpl2)) {
> > +                                             return CIL_TRUE;
> > +                                     }
> > +                             }
> > +                     }
> > +             } else { /* SET */
> > +                     struct cil_classperms_set *cp_set = curr->data;
> > +                     struct cil_classpermission *cp = cp_set->set;
> > +                     if (cil_classperms_list_match(cp->classperms, cpl2)) {
> > +                             return CIL_TRUE;
> > +                     }
> > +             }
> > +     }
> > +     return CIL_FALSE;
> > +}
> > +
> > +static void cil_classperms_copy(struct cil_classperms **new, const struct cil_classperms *old)
> > +{
> > +     cil_classperms_init(new);
> > +     (*new)->class_str = old->class_str;
> > +     (*new)->class = old->class;
> > +     cil_copy_list(old->perm_strs, &(*new)->perm_strs);
> > +     cil_copy_list(old->perms, &(*new)->perms);
> > +}
> > +
> > +static void cil_classperms_set_copy(struct cil_classperms_set **new, const struct cil_classperms_set *old)
> > +{
> > +     cil_classperms_set_init(new);
> > +     (*new)->set_str = old->set_str;
> > +     (*new)->set = old->set;
> > +}
> > +
> > +void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     if (!new) {
> > +             return;
> > +     }
> > +
> > +     if (!old) {
> > +             *new = NULL;
> > +             return;
> > +     }
> > +
> > +     cil_list_init(new, CIL_LIST);
> > +
> > +     cil_list_for_each(curr, old) {
> > +             if (curr->flavor == CIL_CLASSPERMS) {
> > +                     struct cil_classperms *new_cp;
> > +                     cil_classperms_copy(&new_cp, curr->data);
> > +                     cil_list_append(*new, CIL_CLASSPERMS, new_cp);
> > +             } else { /* SET */
> > +                     struct cil_classperms_set *new_cps;
> > +                     cil_classperms_set_copy(&new_cps, curr->data);
> > +                     cil_list_append(*new, CIL_CLASSPERMS_SET, new_cps);
> > +             }
> > +     }
> > +
> > +     if (cil_list_is_empty(*new)) {
> > +             cil_list_destroy(new, CIL_FALSE);
> > +     }
> > +}
> > +
> > +/* Append cp1 and not cpl2 to result */
> > +static void cil_classperms_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_classperms *new_cp = NULL;
> > +     struct cil_list_item *curr;
> > +
> > +     if (!cil_classperms_match(cp1, cpl2)) {
> > +             cil_classperms_copy(&new_cp, cp1);
> > +             cil_list_append(*result, CIL_CLASSPERMS, new_cp);
> > +             return;
> > +     }
> > +
> > +     cil_list_for_each(curr, cp1->perms) {
> > +             struct cil_perm *perm = curr->data;
> > +             if (!cil_class_perm_match(cp1->class, perm, cpl2)) {
> > +                     if (new_cp == NULL) {
> > +                             cil_classperms_init(&new_cp);
> > +                             new_cp->class_str = cp1->class_str;
> > +                             new_cp->class = cp1->class;
> > +                             cil_list_init(&new_cp->perm_strs, CIL_PERM);
> > +                             cil_list_init(&new_cp->perms, CIL_PERM);
> > +                             cil_list_append(*result, CIL_CLASSPERMS, new_cp) > +                    }
> > +                     cil_list_append(new_cp->perm_strs, CIL_STRING, perm->datum.fqn);
> > +                     cil_list_append(new_cp->perms, CIL_DATUM, perm);
>
> Why can't you just use cil_classperms_copy() here?
>

Because I am not copying every permission. I don't want to initialize
the new classperms struct if there are no matches, so I wait until the
first match to initialize it. Immediately appending the new struct
might be confusing. As soon as there is a match I know that the new
classperms struct is going to be appended to the results, but
permissions will continue to be added to it even after it has been
appended.


> > +             }
> > +     }
> > +}
> > +
> > +/* Append cp1 and not cpl2 to result */
> > +static void cil_classperms_map_andnot(struct cil_list **result, const struct cil_classperms *cp1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_classperms *new_cp = NULL;
> > +     struct cil_list_item *p;
> > +
> > +     cil_list_for_each(p, cp1->perms) {
> > +             struct cil_perm *map_perm = p->data;
> > +             if (!cil_classperms_list_match(map_perm->classperms, cpl2)) {
> > +                     if (new_cp == NULL) {
> > +                             cil_classperms_init(&new_cp);
> > +                             new_cp->class_str = cp1->class_str;
> > +                             new_cp->class = cp1->class;
> > +                             cil_list_init(&new_cp->perm_strs, CIL_PERM);
> > +                             cil_list_init(&new_cp->perms, CIL_PERM);
> > +                             cil_list_append(*result, CIL_CLASSPERMS, new_cp);
> > +                     }
> > +                     cil_list_append(new_cp->perm_strs, CIL_STRING, map_perm->datum.fqn);
> > +                     cil_list_append(new_cp->perms, CIL_DATUM, map_perm);
>
> Same question about cil_classperms_copy().
>

Same thing here.

> > +             } else {
> > +                     struct cil_list *new_cpl = NULL;
> > +                     cil_classperms_list_andnot(&new_cpl, map_perm->classperms, cpl2);
> > +                     if (new_cpl) {
> > +                             struct cil_list_item *i;
> > +                             cil_list_for_each(i, new_cpl) {
> > +                                     cil_list_append(*result, i->flavor, i->data);
> > +                             }
> > +                             cil_list_destroy(&new_cpl, CIL_FALSE);
> > +                     }
> > +             }
> > +     }
> > +}
> > +
> > +/* Append cps1 and not cpl2 to result */
> > +static void cil_classperms_set_andnot(struct cil_list **result, const struct cil_classperms_set *cps1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_classpermission *cp = cps1->set;
> > +
> > +     if (!cil_classperms_list_match(cp->classperms, cpl2)) {
> > +             struct cil_classperms_set *new_cps;
> > +             cil_classperms_set_copy(&new_cps, cps1);
> > +             cil_list_append(*result, CIL_CLASSPERMS_SET, new_cps);
> > +     } else {
> > +             struct cil_list *new_cpl;
> > +             cil_classperms_list_andnot(&new_cpl, cp->classperms, cpl2);
> > +             if (new_cpl) {
> > +                     struct cil_list_item *i;
> > +                     cil_list_for_each(i, new_cpl) {
> > +                             cil_list_append(*result, i->flavor, i->data);
> > +                     }
> > +                     cil_list_destroy(&new_cpl, CIL_FALSE);
> > +             }
> > +     }
> > +}
> > +
> > +/* result = cpl1 and not cpl2 */
> > +void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2)
> > +{
> > +     struct cil_list_item *curr;
> > +
> > +     if (!result) {
> > +             return;
> > +     }
> > +
> > +     if (!cpl1) {
> > +             *result = NULL;
> > +             return;
> > +     }
> > +
> > +     cil_list_init(result, CIL_LIST);
> > +
> > +     if (!cpl2 || !cil_classperms_list_match(cpl1, cpl2)) {
> > +             cil_classperms_list_copy(result, cpl1);
> > +             return;
> > +     }
> > +
> > +     cil_list_for_each(curr, cpl1) {
> > +             if (curr->flavor == CIL_CLASSPERMS) {
> > +                     struct cil_classperms *cp = curr->data;
> > +                     if (FLAVOR(cp->class) == CIL_CLASS) {
> > +                             cil_classperms_andnot(result, cp, cpl2);
> > +                     } else { /* MAP */
> > +                             cil_classperms_map_andnot(result, cp, cpl2);
> > +                     }
> > +             } else { /* SET */
> > +                     struct cil_classperms_set *cps = curr->data;
> > +                     cil_classperms_set_andnot(result, cps, cpl2);
> > +             }
> > +     }
> > +
> > +     if (cil_list_is_empty(*result)) {
> > +             cil_list_destroy(result, CIL_FALSE);
> > +     }
> > +}
> > +
> > +/* result = d1 and d2 */
> > +static int cil_datums_and(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
> > +{
> > +     int rc = SEPOL_OK;
> > +     enum cil_flavor f1 = FLAVOR(d1);
> > +     enum cil_flavor f2 = FLAVOR(d2);
> > +
> > +     if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> > +             struct cil_type *t1 = (struct cil_type *)d1;
> > +             struct cil_type *t2 = (struct cil_type *)d2;
> > +             ebitmap_init(result);
> > +             if (t1->value == t2->value) {
> > +                     rc = ebitmap_set_bit(result, t1->value, 1);
> > +                     if (rc != SEPOL_OK) {
> > +                             ebitmap_destroy(result);
> > +                             goto exit;
> > +                     }
> > +             }
> > +     } else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> > +             struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> > +             struct cil_type *t2 = (struct cil_type *)d2;
> > +             ebitmap_init(result);
> > +             if (ebitmap_get_bit(a1->types, t2->value)) {
> > +                     rc = ebitmap_set_bit(result, t2->value, 1);
> > +                     if (rc != SEPOL_OK) {
> > +                             ebitmap_destroy(result);
> > +                             goto exit;
> > +                     }
> > +             }
> > +     } else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
> > +             struct cil_type *t1 = (struct cil_type *)d1;
> > +             struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> > +             ebitmap_init(result);
> > +             if (ebitmap_get_bit(a2->types, t1->value)) {
> > +                     rc = ebitmap_set_bit(result, t1->value, 1);
> > +                     if (rc != SEPOL_OK) {
> > +                             ebitmap_destroy(result);
> > +                             goto exit;
> > +                     }
> > +             }
> > +     } else {
> > +             /* Both are attributes */
> > +             struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> > +             struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> > +             rc = ebitmap_and(result, a1->types, a2->types);
> > +             if (rc != SEPOL_OK) {
> > +                     ebitmap_destroy(result);
> > +                     goto exit;
> > +             }
> > +     }
> > +exit:
> > +     return rc;
> > +}
> > +
> > +/* result = d1 and not d2 */
> > +static int cil_datums_andnot(ebitmap_t *result, const struct cil_symtab_datum *d1, const struct cil_symtab_datum *d2)
> > +{
> > +     int rc = SEPOL_OK;
> > +     enum cil_flavor f1 = FLAVOR(d1);
> > +     enum cil_flavor f2 = FLAVOR(d2);
> > +
> > +     if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> > +             struct cil_type *t1 = (struct cil_type *)d1;
> > +             struct cil_type *t2 = (struct cil_type *)d2;
> > +             ebitmap_init(result);
> > +             if (t1->value != t2->value) {
> > +                     rc = ebitmap_set_bit(result, t1->value, 1);
> > +                     if (rc != SEPOL_OK) {
> > +                             ebitmap_destroy(result);
> > +                             goto exit;
> > +                     }
> > +             }
> > +     } else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
> > +             struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> > +             struct cil_type *t2 = (struct cil_type *)d2;
> > +             rc = ebitmap_cpy(result, a1->types);
> > +             if (rc != SEPOL_OK) {
> > +                     goto exit;
> > +             }
> > +             rc = ebitmap_set_bit(result, t2->value, 0);
> > +             if (rc != SEPOL_OK) {
> > +                     ebitmap_destroy(result);
> > +                     goto exit;
> > +             }
> > +     } else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
> > +             struct cil_type *t1 = (struct cil_type *)d1;
> > +             struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> > +             ebitmap_init(result);
> > +             if (!ebitmap_get_bit(a2->types, t1->value)) {
> > +                     rc = ebitmap_set_bit(result, t1->value, 1);
> > +                     if (rc != SEPOL_OK) {
> > +                             ebitmap_destroy(result);
> > +                             goto exit;
> > +                     }
> > +             }
> > +     } else {
> > +             /* Both are attributes */
> > +             struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
> > +             struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
> > +             rc = ebitmap_andnot(result, a1->types, a2->types, a1->types->highbit);
> > +             if (rc != SEPOL_OK) {
> > +                     ebitmap_destroy(result);
> > +                     goto exit;
> > +             }
> > +     }
> > +exit:
> > +     return rc;
> > +}
> > +
> > +static size_t num_digits(unsigned n)
> > +{
> > +     size_t num = 1;
> > +     while (n >= 10) {
> > +             n /= 10;
> > +             num++;
> > +     }
> > +     return num;
> > +}
> > +
> > +static char *cil_create_new_attribute_name(unsigned num)
> > +{
> > +     char *s1 = NULL;
> > +     char *s2 = NULL;
> > +     size_t len_num = num_digits(num);
> > +     size_t len = strlen(CIL_DENY_ATTR_PREFIX) + 1 + len_num + 1;
> > +     int rc;
> > +
> > +     if (len >= CIL_MAX_NAME_LENGTH) {
> > +             cil_log(CIL_ERR, "Name length greater than max name length of %d",
> > +                             CIL_MAX_NAME_LENGTH);
> > +             goto exit;
> > +     }
> > +
> > +     s1 = cil_malloc(len);
> > +     rc = snprintf(s1, len, "%s_%u", CIL_DENY_ATTR_PREFIX, num);
> > +     if (rc < 0 || (size_t)rc >= len) {
> > +             cil_log(CIL_ERR, "Error creating new attribute name");
> > +             free(s1);
> > +             goto exit;
> > +     }
> > +
> > +     s2 = cil_strpool_add(s1);
> > +     free(s1);
> > +
> > +exit:
> > +     return s2;
> > +}
> > +
> > +static struct cil_list *cil_create_and_expr_list(enum cil_flavor flavor, void *v1, void *v2)
> > +{
> > +     struct cil_list *expr;
> > +
> > +     cil_list_init(&expr, CIL_TYPE);
> > +     cil_list_append(expr, CIL_OP, (void *)CIL_AND);
> > +     cil_list_append(expr, flavor, v1);
> > +     cil_list_append(expr, flavor, v2);
> > +
> > +     return expr;
> > +}
> > +
> > +static struct cil_list *cil_create_andnot_expr_list(enum cil_flavor flavor, void *v1, void *v2)
> > +{
> > +     struct cil_list *expr, *sub_expr;
> > +
> > +     cil_list_init(&expr, CIL_TYPE);
> > +     cil_list_append(expr, CIL_OP, (void *)CIL_AND);
> > +     cil_list_append(expr, flavor, v1);
> > +     cil_list_init(&sub_expr, CIL_TYPE);
> > +     cil_list_append(sub_expr, CIL_OP, (void *)CIL_NOT);
> > +     cil_list_append(sub_expr, flavor, v2);
> > +     cil_list_append(expr, CIL_LIST, sub_expr);
> > +
> > +     return expr;
> > +}
> > +
> > +static struct cil_tree_node *cil_create_and_insert_node(struct cil_tree_node *prev, enum cil_flavor flavor, void *data)
> > +{
> > +     struct cil_tree_node *new;
> > +
> > +     cil_tree_node_init(&new);
> > +     new->parent = prev->parent;
> > +     new->line = prev->line;
> > +     new->hll_offset = prev->hll_offset;
> > +     new->flavor = flavor;
> > +     new->data = data;
> > +     new->next = prev->next;
> > +     prev->next = new;
> > +
> > +     return new;
> > +}
> > +
> > +static int cil_create_and_insert_attribute_and_set(struct cil_db *db, struct cil_tree_node *prev, struct cil_list *str_expr, struct cil_list *datum_expr, ebitmap_t *types, struct cil_symtab_datum **d)
> > +{
> > +     struct cil_tree_node *attr_node = NULL;
> > +     char *name;
> > +     struct cil_typeattribute *attr = NULL;
> > +     struct cil_tree_node *attrset_node = NULL;
> > +     struct cil_typeattributeset *attrset = NULL;
> > +     symtab_t *symtab = NULL;
> > +     int rc = SEPOL_ERR;
> > +
> > +     name = cil_create_new_attribute_name(db->num_types_and_attrs);
> > +     if (!name) {
> > +             goto exit;
> > +     }
> > +
> > +     cil_typeattributeset_init(&attrset);
> > +     attrset->attr_str = name;
> > +     attrset->str_expr = str_expr;
> > +     attrset->datum_expr = datum_expr;
> > +
> > +     cil_typeattribute_init(&attr);
> > +     cil_list_init(&attr->expr_list, CIL_TYPE);
> > +     cil_list_append(attr->expr_list, CIL_LIST, datum_expr);
> > +     attr->types = types;
> > +     attr->used = CIL_ATTR_AVRULE;
> > +     attr->keep = (ebitmap_cardinality(types) < db->attrs_expand_size) ? CIL_FALSE : CIL_TRUE;
> > +
> > +     attr_node = cil_create_and_insert_node(prev, CIL_TYPEATTRIBUTE, attr);
> > +     attrset_node = cil_create_and_insert_node(attr_node, CIL_TYPEATTRIBUTESET, attrset);
> > +
> > +     rc = cil_get_symtab(prev->parent, &symtab, CIL_SYM_TYPES);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
> > +
> > +     rc = cil_symtab_insert(symtab, name, &attr->datum, attr_node);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
> > +
> > +     db->num_types_and_attrs++;
> > +
> > +     *d = &attr->datum;
> > +
> > +     return SEPOL_OK;
> > +
> > +exit:
> > +     if (attrset_node) {
> > +             prev->next = attrset_node->next;
> > +             cil_destroy_typeattribute(attr_node->data);
> > +             free(attr_node);
> > +             cil_destroy_typeattributeset(attrset_node->data);
> > +             free(attrset_node);
> > +     }
> > +     return rc;
> > +}
> > +
> > +static int cil_create_attribute_d1_and_not_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
> > +{
> > +     struct cil_list *str_expr;
> > +     struct cil_list *datum_expr;
> > +     ebitmap_t *types;
> > +     int rc;
> > +
> > +     *d3 = NULL;
> > +
> > +     if (d1 == d2) {
> > +             return SEPOL_OK;
> > +     }
> > +
> > +     str_expr = cil_create_andnot_expr_list(CIL_STRING, d1->fqn, d2->fqn);
> > +     datum_expr = cil_create_andnot_expr_list(CIL_DATUM, d1, d2);
> > +
> > +     types = cil_malloc(sizeof(*types));
> > +     rc = cil_datums_andnot(types, d1, d2);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
> > +     if (ebitmap_is_empty(types)) {
> > +             rc = SEPOL_OK;
> > +             goto exit;
> > +     }
> > +
> > +     if (ebitmap_cardinality(types) == 1) {
> > +             unsigned i = ebitmap_highest_set_bit(types);
> > +             *d3 = DATUM(db->val_to_type[i]);
> > +             ebitmap_destroy(types);
> > +             rc = SEPOL_OK;
> > +             goto exit;
> > +     }
> > +
> > +     return cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
> > +
> > +exit:
> > +     cil_list_destroy(&str_expr, CIL_FALSE);
> > +     cil_list_destroy(&datum_expr, CIL_FALSE);
> > +     free(types);
> > +     return rc;
> > +}
> > +
> > +static int cil_create_attribute_d1_and_d2(struct cil_db *db, struct cil_tree_node *prev, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2, struct cil_symtab_datum **d3)
> > +{
> > +     struct cil_list *str_expr;
> > +     struct cil_list *datum_expr;
> > +     ebitmap_t *types;
> > +     int rc;
> > +
> > +     if (d1 == d2) {
> > +             *d3 = d1;
> > +             return SEPOL_OK;
> > +     }
> > +
> > +     *d3 = NULL;
> > +
> > +     str_expr = cil_create_and_expr_list(CIL_STRING, d1->fqn, d2->fqn);
> > +     datum_expr = cil_create_and_expr_list(CIL_DATUM, d1, d2);
> > +
> > +     types = cil_malloc(sizeof(*types));
> > +     rc = cil_datums_and(types, d1, d2);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
> > +     if (ebitmap_is_empty(types)) {
> > +             rc = SEPOL_OK;
> > +             goto exit;
> > +     }
> > +
> > +     if (ebitmap_cardinality(types) == 1) {
> > +             unsigned i = ebitmap_highest_set_bit(types);
> > +             *d3 = DATUM(db->val_to_type[i]);
> > +             ebitmap_destroy(types);
> > +             rc = SEPOL_OK;
> > +             goto exit;
> > +     }
> > +
> > +     rc = cil_create_and_insert_attribute_and_set(db, prev, str_expr, datum_expr, types, d3);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
>
> This and the and_not variant above differ in this error handling.  The
> and_not one trusts cil_create_and_insert_attribute_and_set() to do the
> cleanup, and this one does its own cleanup.
>
> I think they're both potentially wrong, as
> cil_create_and_insert_attribute_and_set() calls
> cil_destroy_typeattributeset() which does the same cleanup only if it
> makes it halfway through the function.  So if
> cil_create_new_attribute_name() fails, then this case works correctly
> and the and_not case doesn't free str_expr and datum_expr.  On the other
> hand, if something later in cil_create_and_insert_attribute_and_set()
> fails, then the and_not case is correct, and this one will double free
> str_expr and datum_expr.
>

Yes. I really don't want to call cil_destroy_typeattributeset(), I
want the calling functions to deal with str_expr and datum_expr, since
they passed them in.

> > +
> > +     return SEPOL_OK;
> > +
> > +exit:
> > +     cil_list_destroy(&str_expr, CIL_FALSE);
> > +     cil_list_destroy(&datum_expr, CIL_FALSE);
> > +     free(types);
> > +     return rc;
> > +}
> > +
> > +static struct cil_avrule *cil_create_avrule(struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
> > +{
> > +     struct cil_avrule *new;
> > +
> > +     cil_avrule_init(&new);
> > +     new->is_extended = CIL_FALSE;
> > +     new->rule_kind = CIL_AVRULE_ALLOWED;
> > +     new->src_str = src->name;
> > +     new->src = src;
> > +     new->tgt_str = tgt->name;
> > +     new->tgt = tgt;
> > +     new->perms.classperms = classperms;
> > +
> > +     return new;
> > +}
> > +
> > +static struct cil_tree_node *cil_create_and_add_avrule(struct cil_tree_node *curr, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms)
> > +{
> > +     struct cil_avrule *new_avrule;
> > +
> > +     new_avrule = cil_create_avrule(src, tgt, classperms);
> > +     return cil_create_and_insert_node(curr, CIL_AVRULE, new_avrule);
> > +}
> > +
> > +static int cil_remove_permissions_from_rule(struct cil_db *db, struct cil_tree_node *allow_node, const struct cil_tree_node *deny_node)
> > +{
> > +     struct cil_avrule *allow_rule = allow_node->data;
> > +     struct cil_deny_rule *deny_rule = deny_node->data;
> > +     struct cil_symtab_datum *s1 = allow_rule->src;
> > +     struct cil_symtab_datum *t1 = allow_rule->tgt;
> > +     struct cil_list *p1 = allow_rule->perms.classperms;
> > +     struct cil_symtab_datum *s2 = deny_rule->src;
> > +     struct cil_symtab_datum *t2 = deny_rule->tgt;
> > +     struct cil_list *p2 = deny_rule->classperms;
> > +     struct cil_list *p3 = NULL;
> > +     struct cil_tree_node *curr = allow_node;
> > +     int rc;
> > +
> > +     cil_classperms_list_andnot(&p3, p1, p2);
> > +
> > +     if (!cil_list_is_empty(p3)) {;
> > +             curr = cil_create_and_add_avrule(curr, s1, t1, p3);
> > +     } else {
> > +             cil_destroy_classperms_list(&p3);
> > +             p3 = NULL;
> > +     }
> > +
> > +     if (FLAVOR(s1) == CIL_TYPEATTRIBUTE) {
> > +             struct cil_symtab_datum *s3 = NULL;
> > +             struct cil_symtab_datum *d = NULL > +           struct cil_list *cp = NULL;
> > +             if (t1->fqn == CIL_KEY_SELF && t2->fqn != CIL_KEY_SELF) {
> > +                     /* S3 = S1 and not (S2 and T2) */
> > +                     rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t2, &d);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +                     if (d) {
> > +                             rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
> > +                             if (rc != SEPOL_OK) {
> > +                                     goto exit;
> > +                             }
> > +                     }
> > +             } else if (t1->fqn != CIL_KEY_SELF && t2->fqn == CIL_KEY_SELF) {
> > +                     /* S3 = S1 and not (T1 and S2) */
> > +                     rc = cil_create_attribute_d1_and_d2(db, NODE(t1), t1, s2, &d);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +                     if (d) {
> > +                             rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, d, &s3);
> > +                             if (rc != SEPOL_OK) {
> > +                                     goto exit;
> > +                             }
> > +                     }
> > +             } else {
> > +                     /* S3 = S1 and not S2 */
> > +                     rc = cil_create_attribute_d1_and_not_d2(db, NODE(s1), s1, s2, &s3);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +             }
> > +             if (s3) {
> > +                     cil_classperms_list_copy(&cp, p1);
> > +                     curr = cil_create_and_add_avrule(curr, s3, t1, cp);
> > +             }
> > +     }
> > +
> > +     if (t1->fqn != CIL_KEY_SELF && FLAVOR(t1) == CIL_TYPEATTRIBUTE) {
> > +             struct cil_symtab_datum *t3 = NULL;
> > +             struct cil_symtab_datum *d = NULL;
> > +             struct cil_list *cp = NULL;
> > +             if (t2->fqn == CIL_KEY_SELF) {
> > +                     /* T3 = T1 and not (S1 AND S2) */
> > +                     rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, s2, &d);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +                     if (d) {
> > +                             rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, d, &t3);
> > +                             if (rc != SEPOL_OK) {
> > +                                     goto exit;
> > +                             }
> > +                     }
> > +             } else {
> > +                     /* S3 = T1 and not T2 */
> > +                     rc = cil_create_attribute_d1_and_not_d2(db, NODE(t1), t1, t2, &t3);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +             }
> > +             if (t3) {
> > +                     cil_classperms_list_copy(&cp, p1);
> > +                     curr = cil_create_and_add_avrule(curr, s1, t3, cp);
> > +             }
> > +
> > +             if (FLAVOR(s1) == CIL_TYPEATTRIBUTE && FLAVOR(s2) == CIL_TYPEATTRIBUTE &&
> > +                     t2->fqn == CIL_KEY_SELF) {
> > +                     /* Really special case */
> > +                     /* FLAVOR(t1) == CIL_TYPEATTRIBUTE */
> > +                     /* SX = S1 and S2 and T1 */
> > +                     struct cil_symtab_datum *sx = NULL;
> > +                     struct cil_typeattribute *sx_attr = NULL;
> > +
> > +                     d = NULL;
> > +                     cp = NULL;
> > +
> > +                     rc = cil_create_attribute_d1_and_d2(db, NODE(s2), s2, t1, &d);
> > +                     if (rc != SEPOL_OK) {
> > +                             goto exit;
> > +                     }
> > +                     if (d) {
> > +                             rc = cil_create_attribute_d1_and_d2(db, NODE(s1), s1, d, &sx);
> > +                             if (rc != SEPOL_OK) {
> > +                                     goto exit;
> > +                             }
> > +                             sx_attr = (struct cil_typeattribute *)sx;
> > +                     }
> > +                     if (sx && ebitmap_cardinality(sx_attr->types) > 1) {
> > +                             struct cil_symtab_datum *s = NULL;
> > +                             struct cil_symtab_datum *t = NULL;
> > +                             ebitmap_node_t *n;
> > +                             unsigned i;
> > +                             ebitmap_for_each_positive_bit(sx_attr->types, n, i) {
> > +                                     s = DATUM(db->val_to_type[i]);
> > +                                     rc = cil_create_attribute_d1_and_not_d2(db, NODE(sx), sx, s, &t);
> > +                                     if (rc != SEPOL_OK) {
> > +                                             goto exit;
> > +                                     }
> > +                                     if (t) {
> > +                                             cil_classperms_list_copy(&cp, p1);
> > +                                             curr = cil_create_and_add_avrule(curr, s, t, cp);
> > +                                     }
> > +                             }
> > +                     }
> > +             }
> > +     }
> > +
> > +exit:
> > +     return rc;
> > +}
>
> As I mentioned briefly back in December, it might be in the attributes
> case to only grant p2 rather than p3.  I *think* that's safe, since the
> initial allow rule grants everything p3, so the only things needed to be
> added back in the attributes are the denied permissions (p2).
>

Not p2, but p1 and p2

So the new logic that I am looking at is (for the general case):

P3 = P1 and not P2
(allow S1 T1 P3)

P4 = P1 and P2
S3 = S1 and not S2
(allow S3 T1 P4)      ; T1 is attribute, type, self, notself, or other

S4 = S1 and S2
T3 = T1 and not T2  ; Only if T1 is attribute, otherwise there are
different rules
(allow S4 T3 P4)

I am also looking at how adding "notself" and "other" (previously
called "minusself") will effect things. Using "other" actually makes
some cases easier.

Thanks for the thorough review,
Jim

> > +
> > +static int cil_find_matching_allow_rules(struct cil_list *matching, struct cil_tree_node *start, struct cil_tree_node *deny_node)
> > +{
> > +     struct cil_deny_rule *deny_rule = deny_node->data;
> > +     struct cil_avrule target;
> > +
> > +     target.rule_kind = CIL_AVRULE_ALLOWED;
> > +     target.is_extended = CIL_FALSE;
> > +     target.src = deny_rule->src;
> > +     target.tgt = deny_rule->tgt;
> > +     target.perms.classperms = deny_rule->classperms;
> > +
> > +     return cil_find_matching_avrule_in_ast(start, CIL_AVRULE, &target, matching, CIL_FALSE);
> > +}
> > +
> > +static int cil_process_deny_rule(struct cil_db *db, struct cil_tree_node *start, struct cil_tree_node *deny_node)
> > +{
> > +     struct cil_list *matching;
> > +     struct cil_list_item *item;
> > +     int rc;
> > +
> > +     cil_list_init(&matching, CIL_NODE);
> > +
> > +     rc = cil_find_matching_allow_rules(matching, start, deny_node);
> > +     if (rc != SEPOL_OK) {
> > +             goto exit;
> > +     }
> > +
> > +     cil_list_for_each(item, matching) {
> > +             struct cil_tree_node *allow_node = item->data;
> > +             rc = cil_remove_permissions_from_rule(db, allow_node, deny_node);
> > +             cil_tree_remove_node(allow_node);
> > +             cil_tree_node_destroy(&allow_node);
> > +             if (rc != SEPOL_OK) {
> > +                     goto exit;
> > +             }
> > +
> > +     }
> > +
> > +exit:
> > +     cil_list_destroy(&matching, CIL_FALSE);
> > +     return rc;
> > +}
> > +
> > +static int cil_process_deny_rules(struct cil_db *db, struct cil_tree_node *start, struct cil_list *deny_rules)
> > +{
> > +     struct cil_list_item *item;
> > +     int rc;
> > +
> > +     cil_list_for_each(item, deny_rules) {
> > +             struct cil_tree_node *deny_node = item->data;
> > +             rc = cil_process_deny_rule(db, start, deny_node);
> > +             if (rc != SEPOL_OK) {
> > +                     goto exit;
> > +             }
> > +             cil_tree_remove_node(deny_node);
> > +             cil_tree_node_destroy(&deny_node);
> > +     }
> > +
> > +exit:
> > +     return rc;
> > +}
> > +
> > +static int __cil_find_deny_rules(struct cil_tree_node *node,  uint32_t *finished, void *extra_args)
> > +{
> > +     struct cil_list *deny_rules = extra_args;
> > +
> > +     if (node->flavor == CIL_BLOCK) {
> > +             struct cil_block *block = node->data;
> > +             if (block->is_abstract == CIL_TRUE) {
> > +                     *finished = CIL_TREE_SKIP_HEAD;
> > +             }
> > +     } else if (node->flavor == CIL_MACRO) {
> > +             *finished = CIL_TREE_SKIP_HEAD;
> > +     } else if (node->flavor == CIL_DENY_RULE) {
> > +             cil_list_append(deny_rules, CIL_DENY_RULE, node);
> > +     }
> > +     return SEPOL_OK;
> > +}
> > +
> > +int cil_process_deny_rules_in_ast(struct cil_db *db)
> > +{
> > +     struct cil_tree_node *start;
> > +     struct cil_list *deny_rules;
> > +     int rc = SEPOL_ERR;
> > +
> > +     cil_list_init(&deny_rules, CIL_DENY_RULE);
> > +
> > +     if (!db) {
> > +             cil_log(CIL_ERR, "No CIL db provided to process deny rules\n");
> > +             goto exit;
> > +     }
> > +
> > +     start = db->ast->root;
> > +     rc = cil_tree_walk(start, __cil_find_deny_rules, NULL, NULL, deny_rules);
> > +     if (rc != SEPOL_OK) {
> > +             cil_log(CIL_ERR, "An error occurred while getting deny rules\n");
> > +             goto exit;
> > +     }
> > +
> > +     rc = cil_process_deny_rules(db, start, deny_rules);
> > +     if (rc != SEPOL_OK) {
> > +             cil_log(CIL_ERR, "An error occurred while processing deny rules\n");
> > +             goto exit;
> > +     }
> > +
> > +exit:
> > +     cil_list_destroy(&deny_rules, CIL_FALSE);
> > +     return rc;
> > +}
> > diff --git a/libsepol/cil/src/cil_deny.h b/libsepol/cil/src/cil_deny.h
> > new file mode 100644
> > index 00000000..4a9e92d1
> > --- /dev/null
> > +++ b/libsepol/cil/src/cil_deny.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * This file is public domain software, i.e. not copyrighted.
> > + *
> > + * Warranty Exclusion
> > + * ------------------
> > + * You agree that this software is a non-commercially developed program
> > + * that may contain "bugs" (as that term is used in the industry) and
> > + * that it may not function as intended. The software is licensed
> > + * "as is". NSA makes no, and hereby expressly disclaims all, warranties,
> > + * express, implied, statutory, or otherwise with respect to the software,
> > + * including noninfringement and the implied warranties of merchantability
> > + * and fitness for a particular purpose.
> > + *
> > + * Limitation of Liability
> > + *-----------------------
> > + * In no event will NSA be liable for any damages, including loss of data,
> > + * lost profits, cost of cover, or other special, incidental, consequential,
> > + * direct or indirect damages arising from the software or the use thereof,
> > + * however caused and on any theory of liability. This limitation will apply
> > + * even if NSA has been advised of the possibility of such damage. You
> > + * acknowledge that this is a reasonable allocation of risk.
> > + *
> > + * Original author: James Carter
> > + */
> > +
> > +#ifndef CIL_DENY_H_
> > +#define CIL_DENY_H_
> > +
> > +int cil_classperms_list_match(const struct cil_list *cpl1, const struct cil_list *cpl2);
> > +void cil_classperms_list_copy(struct cil_list **new, const struct cil_list *old);
> > +void cil_classperms_list_andnot(struct cil_list **result, const struct cil_list *cpl1, const struct cil_list *cpl2);
> > +int cil_process_deny_rules_in_ast(struct cil_db *db);
> > +
> > +#endif /* CIL_DENY_H_ */
> > diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
> > index 11e572e2..fb7eec2d 100644
> > --- a/libsepol/cil/src/cil_post.c
> > +++ b/libsepol/cil/src/cil_post.c
> > @@ -47,6 +47,7 @@
> >   #include "cil_policy.h"
> >   #include "cil_verify.h"
> >   #include "cil_symtab.h"
> > +#include "cil_deny.h"
> >
> >   #define GEN_REQUIRE_ATTR "cil_gen_require" /* Also in libsepol/src/module_to_cil.c */
> >   #define TYPEATTR_INFIX "_typeattr_"        /* Also in libsepol/src/module_to_cil.c */
> > @@ -2551,6 +2552,12 @@ int cil_post_process(struct cil_db *db)
> >               goto exit;
> >       }
> >
> > +     rc = cil_process_deny_rules_in_ast(db);
> > +     if (rc != SEPOL_OK) {
> > +             cil_log(CIL_ERR, "Failed to process deny rules\n");
> > +             goto exit;
> > +     }
> > +
> >       rc = cil_post_verify(db);
> >       if (rc != SEPOL_OK) {
> >               cil_log(CIL_ERR, "Failed to verify cil database\n");
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 8/9] secilc/test: Add a deny rule test
  2023-02-03 22:54   ` Daniel Burgener
@ 2023-02-09 14:31     ` James Carter
  0 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2023-02-09 14:31 UTC (permalink / raw)
  To: Daniel Burgener; +Cc: selinux

On Fri, Feb 3, 2023 at 5:54 PM Daniel Burgener
<dburgener@linux.microsoft.com> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> >   secilc/test/deny_rule_test.cil | 384 +++++++++++++++++++++++++++++++++
> >   1 file changed, 384 insertions(+)
> >   create mode 100644 secilc/test/deny_rule_test.cil
> >
> > diff --git a/secilc/test/deny_rule_test.cil b/secilc/test/deny_rule_test.cil
> > new file mode 100644
> > index 00000000..3ef4ac98
> > --- /dev/null
> > +++ b/secilc/test/deny_rule_test.cil
> > @@ -0,0 +1,384 @@
> > +(class CLASS (PERM))
> > +(class C1 (p1a p1b p1c p1d))
> > +(class C2 (p2a p2b p2c p2d))
> > +(class C3 (p3a p3b p3c p3d))
> > +(class C4 (p4a p4b p4c p4d))
> > +(classorder (CLASS C1 C2 C3 C4))
> > +(sid SID)
> > +(sidorder (SID))
> > +(user USER)
> > +(role ROLE)
> > +(type TYPE)
> > +(category CAT)
> > +(categoryorder (CAT))
> > +(sensitivity SENS)
> > +(sensitivityorder (SENS))
> > +(sensitivitycategory SENS (CAT))
> > +(allow TYPE self (CLASS (PERM)))
> > +(roletype ROLE TYPE)
> > +(userrole USER ROLE)
> > +(userlevel USER (SENS))
> > +(userrange USER ((SENS)(SENS (CAT))))
> > +(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
> > +
> > +(classmap cm5 (mp5a mp5b))
> > +(classmapping cm5 mp5a
> > +           (C2 (p2a p2b)))
> > +(classmapping cm5 mp5b
> > +           (C2 (p2c p2d)))
> > +
> > +(classpermission cp6)
> > +(classpermissionset cp6 (C3 (p3a p3b)))
> > +(classpermissionset cp6 (C4 (p4a p4b)))
> > +
> > +(classpermission cp7)
> > +(classpermissionset cp7 (C2 (p2a p2b)))
> > +(classpermissionset cp7 (C2 (p2c p2d)))
> > +
> > +; Test 1
> > +(type t01a)
> > +(type t01b)
> > +(allow t01a t01b (C1 (p1a)))
> > +(deny t01a t01b (C1 (p1a)))
> > +(neverallow t01a t01b (C1 (p1a)))
> > +
> > +; Test 2
> > +(type t02a)
> > +(type t02b)
> > +(allow t02a t02b (C1 (p1a p1b)))
> > +(deny t02a t02b (C1 (p1a)))
> > +(neverallow t02a t02b (C1 (p1a)))
> > +; (neverallow t02a t02b (C1 (p1b))) ; This check should fail
> > +
> > +; Test 3
> > +(type t03a)
> > +(type t03b)
> > +(allow t03a t03b (C1 (p1a)))
> > +(deny t03a t03b (C1 (p1a p1b)))
> > +(neverallow t03a t03b (C1 (p1a p1b)))
> > +
> > +
> > +; Test 11
> > +(type t11a)
> > +(type t11b)
> > +(type t11c)
> > +(type t11d)
> > +(typeattribute a11a)
> > +(typeattribute a11b)
> > +(typeattributeset a11a (t11a t11b))
> > +(typeattributeset a11b (t11c t11d))
> > +(allow a11a a11b (C1 (p1a)))
> > +(deny a11a a11b (C1 (p1a)))
> > +(neverallow a11a a11b (C1 (p1a)))
> > +
> > +; Test 12
> > +(type t12a)
> > +(type t12b)
> > +(type t12c)
> > +(type t12d)
> > +(typeattribute a12a)
> > +(typeattribute a12b)
> > +(typeattributeset a12a (t12a t12b))
> > +(typeattributeset a12b (t12c t12d))
> > +(allow t12a t12c (C1 (p1a)))
> > +(deny a12a a12b (C1 (p1a)))
> > +(neverallow a12a a12b (C1 (p1a)))
> > +
> > +; Test 13
> > +(type t13a)
> > +(type t13b)
> > +(type t13c)
> > +(type t13d)
> > +(typeattribute a13a)
> > +(typeattribute a13b)
> > +(typeattributeset a13a (t13a t13b))
> > +(typeattributeset a13b (t13c t13d))
> > +(allow a13a a13b (C1 (p1a)))
> > +(deny t13a t13c (C1 (p1a)))
> > +(neverallow t13a t13c (C1 (p1a)))
> > +; (neverallow t13b t13d (C1 (p1a))) ; This check should fail
> > +
> > +; Test 21
> > +(type t21a)
> > +(type t21b)
> > +(allow t21a t21b (cm5 (mp5a)))
> > +(deny t21a t21b (cm5 (mp5a)))
> > +(neverallow t21a t21b (cm5 (mp5a)))
> > +
> > +; Test 22
> > +(type t22a)
> > +(type t22b)
> > +(allow t22a t22b (cm5 (mp5a mp5b)))
> > +(deny t22a t22b (cm5 (mp5a)))
> > +(neverallow t22a t22b (cm5 (mp5a)))
> > +; (neverallow t22a t22b (cm5 (mp5b))) ; This check should fail
> > +
> > +; Test 23
> > +(type t23a)
> > +(type t23b)
> > +(allow t23a t23b (cm5 (mp5a)))
> > +(deny t23a t23b (cm5 (mp5a mp5b)))
> > +(neverallow t23a t23b (cm5 (mp5a mp5b)))
> > +
> > +; Test 24
> > +(type t24a)
> > +(type t24b)
> > +(allow t24a t24b (C2 (p2a)))
> > +(deny t24a t24b (cm5 (mp5a)))
> > +(neverallow t24a t24b (cm5 (mp5a)))
> > +
> > +; Test 25
> > +(type t25a)
> > +(type t25b)
> > +(allow t25a t25b (cm5 (mp5a)))
> > +(deny t25a t25b (C2 (p2a)))
> > +(neverallow t25a t25b (C2 (p2a)))
> > +; (neverallow t25a t25b (C2 (p2b))) ; This check should fail
> > +
> > +; Test 31
> > +(type t31a)
> > +(type t31b)
> > +(allow t31a t31b cp6)
> > +(deny t31a t31b cp6)
> > +(neverallow t31a t31b cp6)
> > +
> > +; Test 32
> > +(type t32a)
> > +(type t32b)
> > +(allow t32a t32b cp6)
> > +(deny t32a t32b (C3 (p3a p3b)))
> > +(neverallow t32a t32b (C3 (p3a p3b)))
> > +; (neverallow t32a t32b (C4 (p4a p4b))) ; This check should fail
> > +
> > +; Test 33
> > +(type t33a)
> > +(type t33b)
> > +(allow t33a t33b (C3 (p3a)))
> > +(deny t33a t33b cp6)
> > +(neverallow t33a t33b cp6)
> > +
> > +; Test 34
> > +(type t34a)
> > +(type t34b)
> > +(allow t34a t34b cp7)
> > +(deny t34a t34b (cm5 (mp5a mp5b)))
> > +(neverallow t34a t34b (cm5 (mp5a mp5b)))
> > +
> > +; Test 35
> > +(type t35a)
> > +(type t35b)
> > +(allow t35a t35b (cm5 (mp5a mp5b)))
> > +(deny t35a t35b cp7)
> > +(neverallow t35a t35b cp7)
> > +
> > +; Test 41
> > +(type t41a)
> > +(allow t41a self (C1 (p1a)))
> > +(deny t41a self (C1 (p1a)))
> > +(neverallow t41a self (C1 (p1a)))
> > +
> > +; Test 42
> > +(type t42a)
> > +(allow t42a self (C1 (p1a)))
> > +(deny t42a t42a (C1 (p1a)))
> > +(neverallow t42a t42a (C1 (p1a)))
> > +
> > +; Test 43
> > +(type t43a)
> > +(allow t43a t43a (C1 (p1a)))
> > +(deny t43a self (C1 (p1a)))
> > +(neverallow t43a self (C1 (p1a)))
> > +
> > +; Test 51
> > +(type t51a)
> > +(type t51b)
> > +(typeattribute a51a)
> > +(typeattributeset a51a (t51a t51b))
> > +(allow a51a self (C1 (p1a)))
> > +(deny a51a self (C1 (p1a)))
> > +(neverallow a51a self (C1 (p1a)))
> > +
> > +; Test 52
> > +(type t52a)
> > +(type t52b)
> > +(typeattribute a52a)
> > +(typeattributeset a52a (t52a t52b))
> > +(allow t52a self (C1 (p1a)))
> > +(deny a52a self (C1 (p1a)))
> > +(neverallow a52a self (C1 (p1a)))
> > +
> > +; Test 53
> > +(type t53a)
> > +(type t53b)
> > +(typeattribute a53a)
> > +(typeattributeset a53a (t53a t53b))
> > +(allow a53a self (C1 (p1a)))
> > +(deny t53a self (C1 (p1a)))
> > +(neverallow t53a self (C1 (p1a)))
> > +; (neverallow t53b self (C1 (p1a))) ; This check should fail
> > +
> > +; Test 54
> > +(type t54a)
> > +(type t54b)
> > +(typeattribute a54a)
> > +(typeattributeset a54a (t54a t54b))
> > +(allow a54a self (C1 (p1a)))
> > +(deny a54a a54a (C1 (p1a)))
> > +(neverallow a54a a54a (C1 (p1a)))
> > +
> > +; Test 55
> > +(type t55a)
> > +(type t55b)
> > +(typeattribute a55a)
> > +(typeattributeset a55a (t55a t55b))
> > +(allow a55a a55a (C1 (p1a)))
> > +(deny a55a self (C1 (p1a)))
> > +(neverallow a55a self (C1 (p1a)))
> > +; (neverallow t55a t55b (C1 (p1a))) ; This check should fail
> > +; (neverallow t55b t55a (C1 (p1a))) ; This check should fail
> > +
> > +; Test 61
> > +(type t61a)
> > +(type t61b)
> > +(type t61c)
> > +(type t61d)
> > +(type t61e)
> > +(type t61f)
> > +(type t61g)
> > +(type t61h)
> > +(type t61i)
> > +(type t61j)
> > +(type t61k)
> > +(type t61l)
> > +(type t61m)
> > +(type t61n)
> > +(type t61o)
> > +(type t61p)
> > +(type t61q)
> > +(type t61r)
> > +(type t61s)
> > +(type t61t)
> > +(typeattribute a61a)
> > +(typeattribute a61b)
> > +(typeattribute a61c)
> > +(typeattribute a61x)
> > +(typeattribute a61s)
> > +(typeattribute a61t)
> > +(typeattributeset a61a (t61a t61b t61c t61d t61e t61f t61g t61h t61o t61p))
> > +(typeattributeset a61b (t61a t61b t61c t61d t61i t61j t61k t61l t61q t61r))
> > +(typeattributeset a61c (t61a t61b t61e t61f t61i t61j t61m t61n t61s t61t))
> > +(typeattributeset a61x (t61a t61b))
> > +(typeattributeset a61s (t61c t61d t61e t61f t61g t61h t61o t61p))
> > +(typeattributeset a61t (t61c t61d t61i t61j t61k t61l t61q t61r))
>
> Was the intention to have a failing check on a61x below?  I don't see
> a61x referenced anywhere except where it was declared and set.
>
> -Daniel
>

It probably was and then I decided that I didn't need it. I don't
know. It definitely is not needed now.
Thanks,
Jim


> > +(allow a61a a61b (C1 (p1a)))
> > +(deny a61c self (C1 (p1a)))
> > +(neverallow a61c self (C1 (p1a)))
> > +; (neverallow a61s a61b (C1 (p1a))) ; This check should fail
> > +; (neverallow a61a a61t (C1 (p1a))) ; This check should fail
> > +; (neverallow t61a t61b (C1 (p1a))) ; This check should fail
> > +; (neverallow t61b t61a (C1 (p1a))) ; This check should fail
> > +
> > +; Test 62
> > +(type t62a)
> > +(type t62b)
> > +(type t62c)
> > +(type t62d)
> > +(type t62e)
> > +(type t62f)
> > +(type t62g)
> > +(type t62h)
> > +(type t62i)
> > +(type t62j)
> > +(type t62k)
> > +(type t62l)
> > +(type t62m)
> > +(type t62n)
> > +(type t62o)
> > +(type t62p)
> > +(type t62s)
> > +(type t62t)
> > +(type t62u)
> > +(type t62v)
> > +(typeattribute a62a)
> > +(typeattribute a62c)
> > +(typeattribute a62d)
> > +(typeattribute a62s)
> > +(typeattributeset a62a (t62a t62b t62c t62d t62e t62f t62g t62h t62o t62p))
> > +(typeattributeset a62c (t62a t62b t62e t62f t62i t62j t62m t62n t62s t62t))
> > +(typeattributeset a62d (t62a t62b t62g t62h t62k t62l t62m t62n t62u t62v))
> > +(typeattributeset a62s (t62c t62d t62e t62f t62g t62h t62o t62p))
> > +(allow a62a self (C1 (p1a)))
> > +(deny a62c a62d (C1 (p1a)))
> > +(neverallow a62c a62d (C1 (p1a)))
> > +; (neverallow a62s self (C1 (p1a))) ; This check should fail
> > +
> > +; Test 63
> > +(type t63a)
> > +(type t63b)
> > +(type t63c)
> > +(type t63d)
> > +(type t63e)
> > +(type t63f)
> > +(type t63g)
> > +(type t63h)
> > +(type t63i)
> > +(type t63j)
> > +(type t63k)
> > +(type t63l)
> > +(type t63m)
> > +(type t63n)
> > +(type t63o)
> > +(type t63p)
> > +(type t63s)
> > +(type t63t)
> > +(typeattribute a63a)
> > +(typeattribute a63c)
> > +(typeattribute a63s)
> > +(typeattributeset a63a (t63a t63b t63c t63d t63e t63f t63g t63h t63o t63p))
> > +(typeattributeset a63c (t63a t63b t63e t63f t63i t63j t63m t63n t63s t63t))
> > +(typeattributeset a63s (t62c t62d t62e t62f t62g t62h t62o t62p))
> > +(allow a63a self (C1 (p1a)))
> > +(deny a63c self (C1 (p1a)))
> > +(neverallow a63c self (C1 (p1a)))
> > +; (neverallow a63s self (C1 (p1a))) ; This check should fail
> > +
> > +; Test 64
> > +(type t64a)
> > +(type t64b)
> > +(type t64c)
> > +(type t64d)
> > +(type t64e)
> > +(type t64f)
> > +(type t64g)
> > +(type t64h)
> > +(type t64i)
> > +(type t64j)
> > +(type t64k)
> > +(type t64l)
> > +(type t64m)
> > +(type t64n)
> > +(type t64o)
> > +(type t64p)
> > +(type t64q)
> > +(type t64r)
> > +(type t64s)
> > +(type t64t)
> > +(type t64u)
> > +(type t64v)
> > +(typeattribute a64a)
> > +(typeattribute a64b)
> > +(typeattribute a64c)
> > +(typeattribute a64d)
> > +(typeattribute a64s)
> > +(typeattribute a64t)
> > +(typeattributeset a64a (t64a t64b t64c t64d t64e t64f t64g t64h t64o t64p))
> > +(typeattributeset a64b (t64a t64b t64c t64d t64i t64j t64k t64l t64q t64r))
> > +(typeattributeset a64c (t64a t64b t64e t64f t64i t64j t64m t64n t64s t64t))
> > +(typeattributeset a64d (t64a t64b t64g t64h t64k t64l t64m t64n t64u t64v))
> > +(typeattributeset a64s (t64c t64d t64g t64h t64o t64p))
> > +(typeattributeset a64t (t64c t64d t64i t64j t64q t64r))
> > +(allow a64a a64b (C1 (p1a)))
> > +(deny a64c a64d (C1 (p1a)))
> > +(neverallow a64c a64d (C1 (p1a)))
> > +; (neverallow a64s a64b (C1 (p1a))) ; This check should fail
> > +; (neverallow a64a a64t (C1 (p1a))) ; This check should fail
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation
  2023-02-03 22:55   ` Daniel Burgener
@ 2023-02-09 14:39     ` James Carter
  0 siblings, 0 replies; 20+ messages in thread
From: James Carter @ 2023-02-09 14:39 UTC (permalink / raw)
  To: Daniel Burgener; +Cc: selinux

On Fri, Feb 3, 2023 at 5:55 PM Daniel Burgener
<dburgener@linux.microsoft.com> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> >   secilc/docs/cil_access_vector_rules.md | 68 ++++++++++++++++++++++++++
> >   1 file changed, 68 insertions(+)
> >
> > diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
> > index f0ba4a90..35825283 100644
> > --- a/secilc/docs/cil_access_vector_rules.md
> > +++ b/secilc/docs/cil_access_vector_rules.md
> > @@ -247,6 +247,74 @@ This example will not compile as `type_3` is not allowed to be a source type for
> >           (allow type_3 self (property_service (set)))
> >       )
> >   ```
> > +deny
> > +----------
> > +
> > +Remove the access rights defined from any matching allow rules. These rules are processed before [`neverallow`](cil_access_vector_rules.md#neverallow) checking.
> > +
> > +**Rule definition:**
> > +
> > +```secil
> > +    (deny source_id target_id|self classpermissionset_id ...)
> > +```
> > +
> > +**Where:**
> > +
> > +<table>
> > +<colgroup>
> > +<col width="27%" />
> > +<col width="72%" />
> > +</colgroup>
> > +<tbody>
> > +<tr class="odd">
> > +<td align="left"><p><code>deny</code></p></td>
> > +<td align="left"><p>The <code>deny</code> keyword.</p></td>
> > +</tr>
> > +<tr class="even">
> > +<td align="left"><p><code>source_id</code></p></td>
> > +<td align="left"><p>A single previously defined source <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p></td>
> > +</tr>
> > +<tr class="odd">
> > +<td align="left"><p><code>target_id</code></p></td>
> > +<td align="left"><p>A single previously defined target <code>type</code>, <code>typealias</code> or <code>typeattribute</code> identifier.</p>
> > +<p>The <code>self</code> keyword may be used instead to signify that source and target are the same.</p></td>
> > +</tr>
> > +<tr class="even">
> > +<td align="left"><p><code>classpermissionset_id</code></p></td>
> > +<td align="left"><p>A single named or anonymous <code>classpermissionset</code> or a single set of <code>classmap</code>/<code>classmapping</code> identifiers.</p></td>
> > +</tr>
> > +</tbody>
> > +</table>
> > +
> > +**Example:**
> > +
> > +```secil
> > +    (class class1 (perm1 perm2))
> > +
> > +     (type type_1)
> > +    (type type_2)
> > +     (allow type_1 type_2 (class1 (perm1))) ; Allow_1
> > +     (deny type_1 type_2 (class1 (perm1)))  ; Deny_1
> > +     ; Allow_1 will be complete removed by Deny_1.
> > +
> > +    (type type_3)
> > +     (type type_4)
> > +     (allow type_3 type_4 (class1 (perm1 perm2))) ; Allow_2
> > +     (deny type_3 type_4 (class1 (perm1)))        ; Deny_2
> > +     ; Allow_2 will be removed and replaced with the following when Deny_2 is evaluated
> > +    ; (allow type_3 type_4 (class1 (perm2)))
> > +
> > +     (type type_5)
> > +     (type type_6)
> > +     (typeattribute attr_1)
> > +     (typeattributeset attr_1 (type_5 type_6))
> > +     (allow attr_1 attr_1 (class1 (perm1))) ; Allow_3
> > +     (deny type_5 type_6 (class1 (perm1)))  ; Deny_3
> > +     ; Allow_3 will be removed and replaced with the following when Deny_3 is evaluated
> > +     ; (allow type_6 attr_1 (class1 (perm1)))
> > +     ; (allow attr_1 type_5 (class1 (perm1)))
> > +    )
> > +```
>
> Looks like theres some intermixing of spaces and tabs messing up
> formatting on the example.
>
> -Daniel

That final ")" isn't needed as well.
Thanks,
Jim


> >
> >   allowx
> >   ------
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-02-09 14:39 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 21:34 [RFC PATCH 0/9] Add CIL Deny Rule James Carter
2022-12-15 21:34 ` [RFC PATCH 1/9] libsepol/cil: Parse and add deny rule to AST, but do not process James Carter
2022-12-15 21:34 ` [RFC PATCH 2/9] libsepol/cil: Add cil_list_is_empty macro James Carter
2022-12-15 21:34 ` [RFC PATCH 3/9] libsepol/cil: Add cil_tree_remove_node function James Carter
2023-02-03 22:54   ` Daniel Burgener
2023-02-08 21:09     ` James Carter
2022-12-15 21:34 ` [RFC PATCH 4/9] libsepol/cil: Process deny rules James Carter
2023-02-03 22:54   ` Daniel Burgener
2023-02-08 21:57     ` James Carter
2022-12-15 21:34 ` [RFC PATCH 5/9] libsepol/cil: Add cil_write_post_ast function James Carter
2022-12-15 21:34 ` [RFC PATCH 6/9] libsepol: Export the " James Carter
2022-12-15 21:34 ` [RFC PATCH 7/9] secilc/secil2tree: Add option to write CIL AST after post processing James Carter
2022-12-15 21:34 ` [RFC PATCH 8/9] secilc/test: Add a deny rule test James Carter
2023-02-03 22:54   ` Daniel Burgener
2023-02-09 14:31     ` James Carter
2022-12-15 21:34 ` [RFC PATCH 9/9] secilc/docs: Add deny rule to CIL documentation James Carter
2023-02-03 22:55   ` Daniel Burgener
2023-02-09 14:39     ` James Carter
2022-12-16 18:51 ` [RFC PATCH 0/9] Add CIL Deny Rule Daniel Burgener
2022-12-16 20:23   ` James Carter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.