All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH userspace 0/2] Support the 'self' keyword in type transitions
@ 2022-04-22 15:43 Ondrej Mosnacek
  2022-04-22 15:43 ` [PATCH userspace 1/2] libsepol/cil: add support for self " Ondrej Mosnacek
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ondrej Mosnacek @ 2022-04-22 15:43 UTC (permalink / raw)
  To: selinux; +Cc: Dominick Grift

With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.

This series implements the self keyword support in the CIL & TE
languages and the module policydb format. The kernel policydb format
doesn't need any changes, as type transitions are always expanded in the
kernel policydb.

The patches have been tested using the following WIP beakerlib/tmt test:
https://src.fedoraproject.org/fork/omos/tests/selinux/blob/self-in-tt/f/libsepol/self-keyword-in-type-transitions

Ondrej Mosnacek (2):
  libsepol/cil: add support for self keyword in type transitions
  libsepol,checkpolicy: add support for self keyword in type transitions

 checkpolicy/policy_define.c                |  42 +++++-
 libsepol/cil/src/cil_binary.c              | 168 +++++++++++++++------
 libsepol/cil/src/cil_resolve_ast.c         |  25 ++-
 libsepol/include/sepol/policydb/policydb.h |   4 +-
 libsepol/src/expand.c                      |  69 ++++++---
 libsepol/src/link.c                        |   1 +
 libsepol/src/module_to_cil.c               |  30 ++--
 libsepol/src/policydb.c                    |  33 +++-
 libsepol/src/write.c                       |  19 ++-
 secilc/test/policy.cil                     |   3 +
 10 files changed, 293 insertions(+), 101 deletions(-)

-- 
2.35.1


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

* [PATCH userspace 1/2] libsepol/cil: add support for self keyword in type transitions
  2022-04-22 15:43 [PATCH userspace 0/2] Support the 'self' keyword in type transitions Ondrej Mosnacek
@ 2022-04-22 15:43 ` Ondrej Mosnacek
  2022-04-22 15:43 ` [PATCH userspace 2/2] libsepol,checkpolicy: " Ondrej Mosnacek
  2022-04-23  7:52 ` [PATCH userspace 0/2] Support the 'self' " Christian Göttsche
  2 siblings, 0 replies; 8+ messages in thread
From: Ondrej Mosnacek @ 2022-04-22 15:43 UTC (permalink / raw)
  To: selinux; +Cc: Dominick Grift

With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.

This patch implements the self keyword in all 'typetransition'
statements at the CIL level and adds some basic coverage to the secilc
test.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libsepol/cil/src/cil_binary.c      | 168 +++++++++++++++++++++--------
 libsepol/cil/src/cil_resolve_ast.c |  25 +++--
 secilc/test/policy.cil             |   3 +
 3 files changed, 145 insertions(+), 51 deletions(-)

diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index aceb90e0..40615db2 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -1063,24 +1063,53 @@ exit:
 	return rc;
 }
 
+static int __cil_type_rule_to_avtab_helper(policydb_t *pdb,
+					   type_datum_t *sepol_src,
+					   type_datum_t *sepol_tgt,
+					   struct cil_list *class_list,
+					   type_datum_t *sepol_result,
+					   struct cil_type_rule *cil_rule,
+					   cond_node_t *cond_node,
+					   enum cil_flavor cond_flavor)
+{
+	int rc;
+	class_datum_t *sepol_obj = NULL;
+	struct cil_list_item *c;
+
+	cil_list_for_each(c, class_list) {
+		rc = __cil_get_sepol_class_datum(pdb, DATUM(c->data), &sepol_obj);
+		if (rc != SEPOL_OK) return rc;
+
+		rc = __cil_insert_type_rule(
+			pdb, cil_rule->rule_kind, sepol_src->s.value,
+			sepol_tgt->s.value, sepol_obj->s.value,
+			sepol_result->s.value, cil_rule, cond_node, cond_flavor
+		);
+		if (rc != SEPOL_OK) return rc;
+	}
+	return SEPOL_OK;
+}
+
 static int __cil_type_rule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_type_rule *cil_rule, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
 	int rc = SEPOL_ERR;
-	uint16_t kind = cil_rule->rule_kind;
+	struct cil_symtab_datum *src = NULL;
+	struct cil_symtab_datum *tgt = NULL;
 	type_datum_t *sepol_src = NULL;
 	type_datum_t *sepol_tgt = NULL;
-	class_datum_t *sepol_obj = NULL;
 	struct cil_list *class_list = NULL;
 	type_datum_t *sepol_result = NULL;
 	ebitmap_t src_bitmap, tgt_bitmap;
 	ebitmap_node_t *node1, *node2;
 	unsigned int i, j;
-	struct cil_list_item *c;
 
-	rc = __cil_expand_type(cil_rule->src, &src_bitmap);
-	if (rc != SEPOL_OK) goto exit;
+	ebitmap_init(&src_bitmap);
+	ebitmap_init(&tgt_bitmap);
+
+	src = cil_rule->src;
+	tgt = cil_rule->tgt;
 
-	rc = __cil_expand_type(cil_rule->tgt, &tgt_bitmap);
+	rc = __cil_expand_type(src, &src_bitmap);
 	if (rc != SEPOL_OK) goto exit;
 
 	class_list = cil_expand_class(cil_rule->obj);
@@ -1088,19 +1117,34 @@ static int __cil_type_rule_to_avtab(policydb_t *pdb, const struct cil_db *db, st
 	rc = __cil_get_sepol_type_datum(pdb, DATUM(cil_rule->result), &sepol_result);
 	if (rc != SEPOL_OK) goto exit;
 
-	ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
-		rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
+	if (tgt->fqn == CIL_KEY_SELF) {
+		ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
+			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
+			if (rc != SEPOL_OK) goto exit;
+
+			rc = __cil_type_rule_to_avtab_helper(
+				pdb, sepol_src, sepol_src, class_list,
+				sepol_result, cil_rule, cond_node, cond_flavor
+			);
+			if (rc != SEPOL_OK) goto exit;
+		}
+	} else {
+		rc = __cil_expand_type(tgt, &tgt_bitmap);
 		if (rc != SEPOL_OK) goto exit;
 
-		ebitmap_for_each_positive_bit(&tgt_bitmap, node2, j) {
-			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[j]), &sepol_tgt);
+		ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
+			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
 			if (rc != SEPOL_OK) goto exit;
 
-			cil_list_for_each(c, class_list) {
-				rc = __cil_get_sepol_class_datum(pdb, DATUM(c->data), &sepol_obj);
+			ebitmap_for_each_positive_bit(&tgt_bitmap, node2, j) {
+				rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[j]), &sepol_tgt);
 				if (rc != SEPOL_OK) goto exit;
 
-				rc = __cil_insert_type_rule(pdb, kind, sepol_src->s.value, sepol_tgt->s.value, sepol_obj->s.value, sepol_result->s.value, cil_rule, cond_node, cond_flavor);
+				rc = __cil_type_rule_to_avtab_helper(
+					pdb, sepol_src, sepol_tgt, class_list,
+					sepol_result, cil_rule, cond_node,
+					cond_flavor
+				);
 				if (rc != SEPOL_OK) goto exit;
 			}
 		}
@@ -1120,19 +1164,57 @@ int cil_type_rule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct c
 	return  __cil_type_rule_to_avtab(pdb, db, cil_rule, NULL, CIL_FALSE);
 }
 
+static int __cil_typetransition_to_avtab_helper(policydb_t *pdb,
+						type_datum_t *sepol_src,
+						type_datum_t *sepol_tgt,
+						struct cil_list *class_list,
+						char *name,
+						type_datum_t *sepol_result)
+{
+	int rc;
+	class_datum_t *sepol_obj = NULL;
+	uint32_t otype;
+	struct cil_list_item *c;
+
+	cil_list_for_each(c, class_list) {
+		rc = __cil_get_sepol_class_datum(pdb, DATUM(c->data), &sepol_obj);
+		if (rc != SEPOL_OK) return rc;
+
+		rc = policydb_filetrans_insert(
+			pdb, sepol_src->s.value, sepol_tgt->s.value,
+			sepol_obj->s.value, name, NULL,
+			sepol_result->s.value, &otype
+		);
+		if (rc != SEPOL_OK) {
+			if (rc == SEPOL_EEXIST) {
+				if (sepol_result->s.value!= otype) {
+					cil_log(CIL_ERR, "Conflicting name type transition rules\n");
+				} else {
+					rc = SEPOL_OK;
+				}
+			} else {
+				cil_log(CIL_ERR, "Out of memory\n");
+			}
+			if (rc != SEPOL_OK) {
+				return rc;
+			}
+		}
+	}
+	return SEPOL_OK;
+}
+
 static int __cil_typetransition_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_nametypetransition *typetrans, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
 	int rc = SEPOL_ERR;
+	struct cil_symtab_datum *src = NULL;
+	struct cil_symtab_datum *tgt = NULL;
 	type_datum_t *sepol_src = NULL;
 	type_datum_t *sepol_tgt = NULL;
-	class_datum_t *sepol_obj = NULL;
 	struct cil_list *class_list = NULL;
 	type_datum_t *sepol_result = NULL;
 	ebitmap_t src_bitmap, tgt_bitmap;
 	ebitmap_node_t *node1, *node2;
 	unsigned int i, j;
-	uint32_t otype;
-	struct cil_list_item *c;
 	char *name = DATUM(typetrans->name)->name;
 
 	if (name == CIL_KEY_STAR) {
@@ -1149,10 +1231,13 @@ static int __cil_typetransition_to_avtab(policydb_t *pdb, const struct cil_db *d
 		return __cil_type_rule_to_avtab(pdb, db, &trans, cond_node, cond_flavor);
 	}
 
-	rc = __cil_expand_type(typetrans->src, &src_bitmap);
-	if (rc != SEPOL_OK) goto exit;
+	ebitmap_init(&src_bitmap);
+	ebitmap_init(&tgt_bitmap);
 
-	rc = __cil_expand_type(typetrans->tgt, &tgt_bitmap);
+	src = typetrans->src;
+	tgt = typetrans->tgt;
+
+	rc = __cil_expand_type(src, &src_bitmap);
 	if (rc != SEPOL_OK) goto exit;
 
 	class_list = cil_expand_class(typetrans->obj);
@@ -1160,37 +1245,34 @@ static int __cil_typetransition_to_avtab(policydb_t *pdb, const struct cil_db *d
 	rc = __cil_get_sepol_type_datum(pdb, DATUM(typetrans->result), &sepol_result);
 	if (rc != SEPOL_OK) goto exit;
 
-	ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
-		rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
+	if (tgt->fqn == CIL_KEY_SELF) {
+		ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
+			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
+			if (rc != SEPOL_OK) goto exit;
+
+			rc = __cil_typetransition_to_avtab_helper(
+				pdb, sepol_src, sepol_src, class_list,
+				name, sepol_result
+			);
+			if (rc != SEPOL_OK) goto exit;
+		}
+	} else {
+		rc = __cil_expand_type(tgt, &tgt_bitmap);
 		if (rc != SEPOL_OK) goto exit;
 
-		ebitmap_for_each_positive_bit(&tgt_bitmap, node2, j) {
-			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[j]), &sepol_tgt);
+		ebitmap_for_each_positive_bit(&src_bitmap, node1, i) {
+			rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[i]), &sepol_src);
 			if (rc != SEPOL_OK) goto exit;
 
-			cil_list_for_each(c, class_list) {
-				rc = __cil_get_sepol_class_datum(pdb, DATUM(c->data), &sepol_obj);
+			ebitmap_for_each_positive_bit(&tgt_bitmap, node2, j) {
+				rc = __cil_get_sepol_type_datum(pdb, DATUM(db->val_to_type[j]), &sepol_tgt);
 				if (rc != SEPOL_OK) goto exit;
 
-				rc = policydb_filetrans_insert(
-					pdb, sepol_src->s.value, sepol_tgt->s.value,
-					sepol_obj->s.value, name, NULL,
-					sepol_result->s.value, &otype
+				rc = __cil_typetransition_to_avtab_helper(
+					pdb, sepol_src, sepol_tgt, class_list,
+					name, sepol_result
 				);
-				if (rc != SEPOL_OK) {
-					if (rc == SEPOL_EEXIST) {
-						if (sepol_result->s.value!= otype) {
-							cil_log(CIL_ERR, "Conflicting name type transition rules\n");
-						} else {
-							rc = SEPOL_OK;
-						}
-					} else {
-						cil_log(CIL_ERR, "Out of memory\n");
-					}
-					if (rc != SEPOL_OK) {
-						goto exit;
-					}
-				}
+				if (rc != SEPOL_OK) goto exit;
 			}
 		}
 	}
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 7823eb94..f5e22c97 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -373,6 +373,7 @@ exit:
 
 int cil_resolve_type_rule(struct cil_tree_node *current, void *extra_args)
 {
+	struct cil_args_resolve *args = extra_args;
 	struct cil_type_rule *rule = current->data;
 	struct cil_symtab_datum *src_datum = NULL;
 	struct cil_symtab_datum *tgt_datum = NULL;
@@ -387,11 +388,15 @@ int cil_resolve_type_rule(struct cil_tree_node *current, void *extra_args)
 	}
 	rule->src = src_datum;
 
-	rc = cil_resolve_name(current, rule->tgt_str, CIL_SYM_TYPES, extra_args, &tgt_datum);
-	if (rc != SEPOL_OK) {
-		goto exit;
+	if (rule->tgt_str == CIL_KEY_SELF) {
+		rule->tgt = args->db->selftype;
+	} else {
+		rc = cil_resolve_name(current, rule->tgt_str, CIL_SYM_TYPES, extra_args, &tgt_datum);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+		rule->tgt = tgt_datum;
 	}
-	rule->tgt = tgt_datum;
 
 	rc = cil_resolve_name(current, rule->obj_str, CIL_SYM_CLASSES, extra_args, &obj_datum);
 	if (rc != SEPOL_OK) {
@@ -638,11 +643,15 @@ int cil_resolve_nametypetransition(struct cil_tree_node *current, void *extra_ar
 	}
 	nametypetrans->src = src_datum;
 
-	rc = cil_resolve_name(current, nametypetrans->tgt_str, CIL_SYM_TYPES, extra_args, &tgt_datum);
-	if (rc != SEPOL_OK) {
-		goto exit;
+	if (nametypetrans->tgt_str == CIL_KEY_SELF) {
+		nametypetrans->tgt = args->db->selftype;
+	} else {
+		rc = cil_resolve_name(current, nametypetrans->tgt_str, CIL_SYM_TYPES, extra_args, &tgt_datum);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+		nametypetrans->tgt = tgt_datum;
 	}
-	nametypetrans->tgt = tgt_datum;
 
 	rc = cil_resolve_name(current, nametypetrans->obj_str, CIL_SYM_CLASSES, extra_args, &obj_datum);
 	if (rc != SEPOL_OK) {
diff --git a/secilc/test/policy.cil b/secilc/test/policy.cil
index 02f4f88d..9e812f59 100644
--- a/secilc/test/policy.cil
+++ b/secilc/test/policy.cil
@@ -130,6 +130,9 @@
 	(typepermissive device_t) 
 	(typemember device_t bin_t file exec_t)
 	(typetransition device_t console_t files console_device_t)
+	(typetransition device_t exec_type files console_device_t)
+	(typetransition exec_type self files console_device_t)
+	(typetransition exec_type self files "filename" console_device_t)
 
 	(roleattribute exec_role)
 	(roleattribute foo_role)
-- 
2.35.1


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

* [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions
  2022-04-22 15:43 [PATCH userspace 0/2] Support the 'self' keyword in type transitions Ondrej Mosnacek
  2022-04-22 15:43 ` [PATCH userspace 1/2] libsepol/cil: add support for self " Ondrej Mosnacek
@ 2022-04-22 15:43 ` Ondrej Mosnacek
  2022-04-22 17:02   ` Dominick Grift
  2022-04-23  7:42   ` Christian Göttsche
  2022-04-23  7:52 ` [PATCH userspace 0/2] Support the 'self' " Christian Göttsche
  2 siblings, 2 replies; 8+ messages in thread
From: Ondrej Mosnacek @ 2022-04-22 15:43 UTC (permalink / raw)
  To: selinux; +Cc: Dominick Grift

With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.

This patch implements the self keyword in all 'typetransition'
statements at the TE language level and adds the support to the module
policydb format. Note that changing the kernel policydb format is not
necessary at all, as type transitions are always expanded in the kernel
policydb.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 checkpolicy/policy_define.c                | 42 ++++++++++++-
 libsepol/include/sepol/policydb/policydb.h |  4 +-
 libsepol/src/expand.c                      | 69 ++++++++++++++--------
 libsepol/src/link.c                        |  1 +
 libsepol/src/module_to_cil.c               | 30 ++++++----
 libsepol/src/policydb.c                    | 33 +++++++++--
 libsepol/src/write.c                       | 19 +++---
 7 files changed, 148 insertions(+), 50 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 16b78346..7e328edc 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1634,6 +1634,15 @@ static int define_compute_type_helper(int which, avrule_t ** rule)
 	}
 	add = 1;
 	while ((id = queue_remove(id_queue))) {
+		if (strcmp(id, "self") == 0) {
+			free(id);
+			if (add == 0) {
+				yyerror("-self is not supported");
+				goto bad;
+			}
+			avrule->flags |= RULE_SELF;
+			continue;
+		}
 		if (set_types(&avrule->ttypes, id, &add, 0))
 			goto bad;
 	}
@@ -3300,7 +3309,7 @@ int define_filename_trans(void)
 	type_datum_t *typdatum;
 	uint32_t otype;
 	unsigned int c, s, t;
-	int add, rc;
+	int add, self, rc;
 
 	if (pass == 1) {
 		/* stype */
@@ -3333,8 +3342,18 @@ int define_filename_trans(void)
 			goto bad;
 	}
 
-	add =1;
+	self = 0;
+	add = 1;
 	while ((id = queue_remove(id_queue))) {
+		if (strcmp(id, "self") == 0) {
+			free(id);
+			if (add == 0) {
+				yyerror("-self is not supported");
+				goto bad;
+			}
+			self = 1;
+			continue;
+		}
 		if (set_types(&ttypes, id, &add, 0))
 			goto bad;
 	}
@@ -3396,6 +3415,24 @@ int define_filename_trans(void)
 					goto bad;
 				}
 			}
+			if (self) {
+				rc = policydb_filetrans_insert(
+					policydbp, s+1, s+1, c+1, name,
+					NULL, otype, NULL
+				);
+				if (rc != SEPOL_OK) {
+					if (rc == SEPOL_EEXIST) {
+						yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
+							name,
+							policydbp->p_type_val_to_name[s],
+							policydbp->p_type_val_to_name[s],
+							policydbp->p_class_val_to_name[c]);
+						goto bad;
+					}
+					yyerror("out of memory");
+					goto bad;
+				}
+			}
 		}
 	
 		/* Now add the real rule since we didn't find any duplicates */
@@ -3418,6 +3455,7 @@ int define_filename_trans(void)
 		}
 		ftr->tclass = c + 1;
 		ftr->otype = otype;
+		ftr->flags = self ? RULE_SELF : 0;
 	}
 
 	free(name);
diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 4bf9f05d..de0068a6 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -314,6 +314,7 @@ typedef struct role_allow_rule {
 } role_allow_rule_t;
 
 typedef struct filename_trans_rule {
+	uint32_t flags; /* may have RULE_SELF set */
 	type_set_t stypes;
 	type_set_t ttypes;
 	uint32_t tclass;
@@ -781,9 +782,10 @@ extern int policydb_set_target_platform(policydb_t *p, int platform);
 #define MOD_POLICYDB_VERSION_XPERMS_IOCTL  18
 #define MOD_POLICYDB_VERSION_INFINIBAND		19
 #define MOD_POLICYDB_VERSION_GLBLUB		20
+#define MOD_POLICYDB_VERSION_SELF_TYPETRANS	21
 
 #define MOD_POLICYDB_VERSION_MIN MOD_POLICYDB_VERSION_BASE
-#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_GLBLUB
+#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_SELF_TYPETRANS
 
 #define POLICYDB_CONFIG_MLS    1
 
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 7316124f..8d19850e 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -1407,6 +1407,40 @@ static int copy_role_trans(expand_state_t * state, role_trans_rule_t * rules)
 	return 0;
 }
 
+static int expand_filename_trans_helper(expand_state_t *state,
+					filename_trans_rule_t *rule,
+					unsigned int s, unsigned int t)
+{
+	uint32_t mapped_otype, present_otype;
+	int rc;
+
+	mapped_otype = state->typemap[rule->otype - 1];
+
+	rc = policydb_filetrans_insert(
+		state->out, s + 1, t + 1,
+		rule->tclass, rule->name,
+		NULL, mapped_otype, &present_otype
+	);
+	if (rc == SEPOL_EEXIST) {
+		/* duplicate rule, ignore */
+		if (present_otype == mapped_otype)
+			return 0;
+
+		ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
+		    state->out->p_type_val_to_name[s],
+		    state->out->p_type_val_to_name[t],
+		    state->out->p_class_val_to_name[rule->tclass - 1],
+		    rule->name,
+		    state->out->p_type_val_to_name[present_otype - 1],
+		    state->out->p_type_val_to_name[mapped_otype - 1]);
+		return -1;
+	} else if (rc < 0) {
+		ERR(state->handle, "Out of memory!");
+		return -1;
+	}
+	return 0;
+}
+
 static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *rules)
 {
 	unsigned int i, j;
@@ -1417,8 +1451,6 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
 
 	cur_rule = rules;
 	while (cur_rule) {
-		uint32_t mapped_otype, present_otype;
-
 		ebitmap_init(&stypes);
 		ebitmap_init(&ttypes);
 
@@ -1434,32 +1466,21 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
 			return -1;
 		}
 
-		mapped_otype = state->typemap[cur_rule->otype - 1];
 
 		ebitmap_for_each_positive_bit(&stypes, snode, i) {
 			ebitmap_for_each_positive_bit(&ttypes, tnode, j) {
-				rc = policydb_filetrans_insert(
-					state->out, i + 1, j + 1,
-					cur_rule->tclass, cur_rule->name,
-					NULL, mapped_otype, &present_otype
+				rc = expand_filename_trans_helper(
+					state, cur_rule, i, j
 				);
-				if (rc == SEPOL_EEXIST) {
-					/* duplicate rule, ignore */
-					if (present_otype == mapped_otype)
-						continue;
-
-					ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
-					    state->out->p_type_val_to_name[i],
-					    state->out->p_type_val_to_name[j],
-					    state->out->p_class_val_to_name[cur_rule->tclass - 1],
-					    cur_rule->name,
-					    state->out->p_type_val_to_name[present_otype - 1],
-					    state->out->p_type_val_to_name[mapped_otype - 1]);
-					return -1;
-				} else if (rc < 0) {
-					ERR(state->handle, "Out of memory!");
-					return -1;
-				}
+				if (rc)
+					return rc;
+			}
+			if (cur_rule->flags & RULE_SELF) {
+				rc = expand_filename_trans_helper(
+					state, cur_rule, i, i
+				);
+				if (rc)
+					return rc;
 			}
 		}
 
diff --git a/libsepol/src/link.c b/libsepol/src/link.c
index ecfb5786..7e8313cb 100644
--- a/libsepol/src/link.c
+++ b/libsepol/src/link.c
@@ -1482,6 +1482,7 @@ static int copy_filename_trans_list(filename_trans_rule_t * list,
 
 		new_rule->tclass = module->map[SYM_CLASSES][cur->tclass - 1];
 		new_rule->otype = module->map[SYM_TYPES][cur->otype - 1];
+		new_rule->flags = cur->flags;
 
 		cur = cur->next;
 	}
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 156a74a2..0e211c93 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -1608,20 +1608,30 @@ static int filename_trans_to_cil(int indent, struct policydb *pdb, struct filena
 			goto exit;
 		}
 
-		ts = &rule->ttypes;
-		rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
-		if (rc != 0) {
-			goto exit;
-		}
-
-		for (stype = 0; stype < num_stypes; stype++) {
-			for (ttype = 0; ttype < num_ttypes; ttype++) {
-				cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
-					    stypes[stype], ttypes[ttype],
+		if (rule->flags & RULE_SELF) {
+			for (stype = 0; stype < num_stypes; stype++) {
+				cil_println(indent, "(typetransition %s self %s \"%s\" %s)",
+					    stypes[stype],
 					    pdb->p_class_val_to_name[rule->tclass - 1],
 					    rule->name,
 					    pdb->p_type_val_to_name[rule->otype - 1]);
 			}
+		} else {
+			ts = &rule->ttypes;
+			rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
+			if (rc != 0) {
+				goto exit;
+			}
+
+			for (stype = 0; stype < num_stypes; stype++) {
+				for (ttype = 0; ttype < num_ttypes; ttype++) {
+					cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
+						    stypes[stype], ttypes[ttype],
+						    pdb->p_class_val_to_name[rule->tclass - 1],
+						    rule->name,
+						    pdb->p_type_val_to_name[rule->otype - 1]);
+				}
+			}
 		}
 
 		names_destroy(&stypes, &num_stypes);
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 7c99571f..fc260eb6 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -327,6 +327,13 @@ static const struct policydb_compat_info policydb_compat[] = {
 	 .ocon_num = OCON_IBENDPORT + 1,
 	 .target_platform = SEPOL_TARGET_SELINUX,
 	},
+	{
+	 .type = POLICY_BASE,
+	 .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
+	 .sym_num = SYM_NUM,
+	 .ocon_num = OCON_IBENDPORT + 1,
+	 .target_platform = SEPOL_TARGET_SELINUX,
+	},
 	{
 	 .type = POLICY_MOD,
 	 .version = MOD_POLICYDB_VERSION_BASE,
@@ -446,7 +453,13 @@ static const struct policydb_compat_info policydb_compat[] = {
 	 .ocon_num = 0,
 	 .target_platform = SEPOL_TARGET_SELINUX,
 	},
-
+	{
+	 .type = POLICY_MOD,
+	 .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
+	 .sym_num = SYM_NUM,
+	 .ocon_num = 0,
+	 .target_platform = SEPOL_TARGET_SELINUX,
+	},
 };
 
 #if 0
@@ -3822,10 +3835,11 @@ static int role_allow_rule_read(role_allow_rule_t ** r, struct policy_file *fp)
 	return 0;
 }
 
-static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_file *fp)
+static int filename_trans_rule_read(policydb_t *p, filename_trans_rule_t **r,
+				    struct policy_file *fp)
 {
-	uint32_t buf[2], nel;
-	unsigned int i, len;
+	uint32_t buf[3], nel, i, len;
+	unsigned int entries;
 	filename_trans_rule_t *ftr, *lftr;
 	int rc;
 
@@ -3870,11 +3884,18 @@ static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_fi
 		if (type_set_read(&ftr->ttypes, fp))
 			return -1;
 
-		rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
+		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
+			entries = 3;
+		else
+			entries = 2;
+
+		rc = next_entry(buf, fp, sizeof(uint32_t) * entries);
 		if (rc < 0)
 			return -1;
 		ftr->tclass = le32_to_cpu(buf[0]);
 		ftr->otype = le32_to_cpu(buf[1]);
+		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
+			ftr->flags = le32_to_cpu(buf[2]);
 	}
 
 	return 0;
@@ -3977,7 +3998,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
 	}
 
 	if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
-	    filename_trans_rule_read(&decl->filename_trans_rules, fp))
+	    filename_trans_rule_read(p, &decl->filename_trans_rules, fp))
 		return -1;
 
 	if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
diff --git a/libsepol/src/write.c b/libsepol/src/write.c
index d7ac2b25..0b4f5d9a 100644
--- a/libsepol/src/write.c
+++ b/libsepol/src/write.c
@@ -1929,11 +1929,12 @@ static int role_allow_rule_write(role_allow_rule_t * r, struct policy_file *fp)
 	return POLICYDB_SUCCESS;
 }
 
-static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_file *fp)
+static int filename_trans_rule_write(policydb_t *p, filename_trans_rule_t *t,
+				     struct policy_file *fp)
 {
 	int nel = 0;
-	size_t items;
-	uint32_t buf[2], len;
+	size_t items, entries;
+	uint32_t buf[3], len;
 	filename_trans_rule_t *ftr;
 
 	for (ftr = t; ftr; ftr = ftr->next)
@@ -1962,10 +1963,14 @@ static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_fi
 
 		buf[0] = cpu_to_le32(ftr->tclass);
 		buf[1] = cpu_to_le32(ftr->otype);
+		buf[2] = cpu_to_le32(ftr->flags);
 
-		items = put_entry(buf, sizeof(uint32_t), 2, fp);
-		if (items != 2)
-			return POLICYDB_ERROR;
+		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
+			entries = 3;
+		else
+			entries = 2;
+
+		items = put_entry(buf, sizeof(uint32_t), entries, fp);
 	}
 	return POLICYDB_SUCCESS;
 }
@@ -2039,7 +2044,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
 	}
 
 	if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
-	    filename_trans_rule_write(decl->filename_trans_rules, fp))
+	    filename_trans_rule_write(p, decl->filename_trans_rules, fp))
 		return POLICYDB_ERROR;
 
 	if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
-- 
2.35.1


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

* Re: [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions
  2022-04-22 15:43 ` [PATCH userspace 2/2] libsepol,checkpolicy: " Ondrej Mosnacek
@ 2022-04-22 17:02   ` Dominick Grift
  2022-04-23  7:42   ` Christian Göttsche
  1 sibling, 0 replies; 8+ messages in thread
From: Dominick Grift @ 2022-04-22 17:02 UTC (permalink / raw)
  To: Ondrej Mosnacek; +Cc: selinux

Ondrej Mosnacek <omosnace@redhat.com> writes:

> With the addition of the anon_inode class in the kernel, 'self'
> transition rules became useful, but haven't been implemented.
>
> This patch implements the self keyword in all 'typetransition'
> statements at the TE language level and adds the support to the module
> policydb format. Note that changing the kernel policydb format is not
> necessary at all, as type transitions are always expanded in the kernel
> policydb.

Thanks for implementing this. I don't have anything useful to write
about the code but looks like you tested the functionaliy thoroughly.

>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  checkpolicy/policy_define.c                | 42 ++++++++++++-
>  libsepol/include/sepol/policydb/policydb.h |  4 +-
>  libsepol/src/expand.c                      | 69 ++++++++++++++--------
>  libsepol/src/link.c                        |  1 +
>  libsepol/src/module_to_cil.c               | 30 ++++++----
>  libsepol/src/policydb.c                    | 33 +++++++++--
>  libsepol/src/write.c                       | 19 +++---
>  7 files changed, 148 insertions(+), 50 deletions(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 16b78346..7e328edc 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -1634,6 +1634,15 @@ static int define_compute_type_helper(int which, avrule_t ** rule)
>  	}
>  	add = 1;
>  	while ((id = queue_remove(id_queue))) {
> +		if (strcmp(id, "self") == 0) {
> +			free(id);
> +			if (add == 0) {
> +				yyerror("-self is not supported");
> +				goto bad;
> +			}
> +			avrule->flags |= RULE_SELF;
> +			continue;
> +		}
>  		if (set_types(&avrule->ttypes, id, &add, 0))
>  			goto bad;
>  	}
> @@ -3300,7 +3309,7 @@ int define_filename_trans(void)
>  	type_datum_t *typdatum;
>  	uint32_t otype;
>  	unsigned int c, s, t;
> -	int add, rc;
> +	int add, self, rc;
>  
>  	if (pass == 1) {
>  		/* stype */
> @@ -3333,8 +3342,18 @@ int define_filename_trans(void)
>  			goto bad;
>  	}
>  
> -	add =1;
> +	self = 0;
> +	add = 1;
>  	while ((id = queue_remove(id_queue))) {
> +		if (strcmp(id, "self") == 0) {
> +			free(id);
> +			if (add == 0) {
> +				yyerror("-self is not supported");
> +				goto bad;
> +			}
> +			self = 1;
> +			continue;
> +		}
>  		if (set_types(&ttypes, id, &add, 0))
>  			goto bad;
>  	}
> @@ -3396,6 +3415,24 @@ int define_filename_trans(void)
>  					goto bad;
>  				}
>  			}
> +			if (self) {
> +				rc = policydb_filetrans_insert(
> +					policydbp, s+1, s+1, c+1, name,
> +					NULL, otype, NULL
> +				);
> +				if (rc != SEPOL_OK) {
> +					if (rc == SEPOL_EEXIST) {
> +						yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
> +							name,
> +							policydbp->p_type_val_to_name[s],
> +							policydbp->p_type_val_to_name[s],
> +							policydbp->p_class_val_to_name[c]);
> +						goto bad;
> +					}
> +					yyerror("out of memory");
> +					goto bad;
> +				}
> +			}
>  		}
>  	
>  		/* Now add the real rule since we didn't find any duplicates */
> @@ -3418,6 +3455,7 @@ int define_filename_trans(void)
>  		}
>  		ftr->tclass = c + 1;
>  		ftr->otype = otype;
> +		ftr->flags = self ? RULE_SELF : 0;
>  	}
>  
>  	free(name);
> diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> index 4bf9f05d..de0068a6 100644
> --- a/libsepol/include/sepol/policydb/policydb.h
> +++ b/libsepol/include/sepol/policydb/policydb.h
> @@ -314,6 +314,7 @@ typedef struct role_allow_rule {
>  } role_allow_rule_t;
>  
>  typedef struct filename_trans_rule {
> +	uint32_t flags; /* may have RULE_SELF set */
>  	type_set_t stypes;
>  	type_set_t ttypes;
>  	uint32_t tclass;
> @@ -781,9 +782,10 @@ extern int policydb_set_target_platform(policydb_t *p, int platform);
>  #define MOD_POLICYDB_VERSION_XPERMS_IOCTL  18
>  #define MOD_POLICYDB_VERSION_INFINIBAND		19
>  #define MOD_POLICYDB_VERSION_GLBLUB		20
> +#define MOD_POLICYDB_VERSION_SELF_TYPETRANS	21
>  
>  #define MOD_POLICYDB_VERSION_MIN MOD_POLICYDB_VERSION_BASE
> -#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_GLBLUB
> +#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_SELF_TYPETRANS
>  
>  #define POLICYDB_CONFIG_MLS    1
>  
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 7316124f..8d19850e 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -1407,6 +1407,40 @@ static int copy_role_trans(expand_state_t * state, role_trans_rule_t * rules)
>  	return 0;
>  }
>  
> +static int expand_filename_trans_helper(expand_state_t *state,
> +					filename_trans_rule_t *rule,
> +					unsigned int s, unsigned int t)
> +{
> +	uint32_t mapped_otype, present_otype;
> +	int rc;
> +
> +	mapped_otype = state->typemap[rule->otype - 1];
> +
> +	rc = policydb_filetrans_insert(
> +		state->out, s + 1, t + 1,
> +		rule->tclass, rule->name,
> +		NULL, mapped_otype, &present_otype
> +	);
> +	if (rc == SEPOL_EEXIST) {
> +		/* duplicate rule, ignore */
> +		if (present_otype == mapped_otype)
> +			return 0;
> +
> +		ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
> +		    state->out->p_type_val_to_name[s],
> +		    state->out->p_type_val_to_name[t],
> +		    state->out->p_class_val_to_name[rule->tclass - 1],
> +		    rule->name,
> +		    state->out->p_type_val_to_name[present_otype - 1],
> +		    state->out->p_type_val_to_name[mapped_otype - 1]);
> +		return -1;
> +	} else if (rc < 0) {
> +		ERR(state->handle, "Out of memory!");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *rules)
>  {
>  	unsigned int i, j;
> @@ -1417,8 +1451,6 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
>  
>  	cur_rule = rules;
>  	while (cur_rule) {
> -		uint32_t mapped_otype, present_otype;
> -
>  		ebitmap_init(&stypes);
>  		ebitmap_init(&ttypes);
>  
> @@ -1434,32 +1466,21 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
>  			return -1;
>  		}
>  
> -		mapped_otype = state->typemap[cur_rule->otype - 1];
>  
>  		ebitmap_for_each_positive_bit(&stypes, snode, i) {
>  			ebitmap_for_each_positive_bit(&ttypes, tnode, j) {
> -				rc = policydb_filetrans_insert(
> -					state->out, i + 1, j + 1,
> -					cur_rule->tclass, cur_rule->name,
> -					NULL, mapped_otype, &present_otype
> +				rc = expand_filename_trans_helper(
> +					state, cur_rule, i, j
>  				);
> -				if (rc == SEPOL_EEXIST) {
> -					/* duplicate rule, ignore */
> -					if (present_otype == mapped_otype)
> -						continue;
> -
> -					ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
> -					    state->out->p_type_val_to_name[i],
> -					    state->out->p_type_val_to_name[j],
> -					    state->out->p_class_val_to_name[cur_rule->tclass - 1],
> -					    cur_rule->name,
> -					    state->out->p_type_val_to_name[present_otype - 1],
> -					    state->out->p_type_val_to_name[mapped_otype - 1]);
> -					return -1;
> -				} else if (rc < 0) {
> -					ERR(state->handle, "Out of memory!");
> -					return -1;
> -				}
> +				if (rc)
> +					return rc;
> +			}
> +			if (cur_rule->flags & RULE_SELF) {
> +				rc = expand_filename_trans_helper(
> +					state, cur_rule, i, i
> +				);
> +				if (rc)
> +					return rc;
>  			}
>  		}
>  
> diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> index ecfb5786..7e8313cb 100644
> --- a/libsepol/src/link.c
> +++ b/libsepol/src/link.c
> @@ -1482,6 +1482,7 @@ static int copy_filename_trans_list(filename_trans_rule_t * list,
>  
>  		new_rule->tclass = module->map[SYM_CLASSES][cur->tclass - 1];
>  		new_rule->otype = module->map[SYM_TYPES][cur->otype - 1];
> +		new_rule->flags = cur->flags;
>  
>  		cur = cur->next;
>  	}
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 156a74a2..0e211c93 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -1608,20 +1608,30 @@ static int filename_trans_to_cil(int indent, struct policydb *pdb, struct filena
>  			goto exit;
>  		}
>  
> -		ts = &rule->ttypes;
> -		rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
> -		if (rc != 0) {
> -			goto exit;
> -		}
> -
> -		for (stype = 0; stype < num_stypes; stype++) {
> -			for (ttype = 0; ttype < num_ttypes; ttype++) {
> -				cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
> -					    stypes[stype], ttypes[ttype],
> +		if (rule->flags & RULE_SELF) {
> +			for (stype = 0; stype < num_stypes; stype++) {
> +				cil_println(indent, "(typetransition %s self %s \"%s\" %s)",
> +					    stypes[stype],
>  					    pdb->p_class_val_to_name[rule->tclass - 1],
>  					    rule->name,
>  					    pdb->p_type_val_to_name[rule->otype - 1]);
>  			}
> +		} else {
> +			ts = &rule->ttypes;
> +			rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
> +			if (rc != 0) {
> +				goto exit;
> +			}
> +
> +			for (stype = 0; stype < num_stypes; stype++) {
> +				for (ttype = 0; ttype < num_ttypes; ttype++) {
> +					cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
> +						    stypes[stype], ttypes[ttype],
> +						    pdb->p_class_val_to_name[rule->tclass - 1],
> +						    rule->name,
> +						    pdb->p_type_val_to_name[rule->otype - 1]);
> +				}
> +			}
>  		}
>  
>  		names_destroy(&stypes, &num_stypes);
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 7c99571f..fc260eb6 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -327,6 +327,13 @@ static const struct policydb_compat_info policydb_compat[] = {
>  	 .ocon_num = OCON_IBENDPORT + 1,
>  	 .target_platform = SEPOL_TARGET_SELINUX,
>  	},
> +	{
> +	 .type = POLICY_BASE,
> +	 .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
> +	 .sym_num = SYM_NUM,
> +	 .ocon_num = OCON_IBENDPORT + 1,
> +	 .target_platform = SEPOL_TARGET_SELINUX,
> +	},
>  	{
>  	 .type = POLICY_MOD,
>  	 .version = MOD_POLICYDB_VERSION_BASE,
> @@ -446,7 +453,13 @@ static const struct policydb_compat_info policydb_compat[] = {
>  	 .ocon_num = 0,
>  	 .target_platform = SEPOL_TARGET_SELINUX,
>  	},
> -
> +	{
> +	 .type = POLICY_MOD,
> +	 .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
> +	 .sym_num = SYM_NUM,
> +	 .ocon_num = 0,
> +	 .target_platform = SEPOL_TARGET_SELINUX,
> +	},
>  };
>  
>  #if 0
> @@ -3822,10 +3835,11 @@ static int role_allow_rule_read(role_allow_rule_t ** r, struct policy_file *fp)
>  	return 0;
>  }
>  
> -static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_file *fp)
> +static int filename_trans_rule_read(policydb_t *p, filename_trans_rule_t **r,
> +				    struct policy_file *fp)
>  {
> -	uint32_t buf[2], nel;
> -	unsigned int i, len;
> +	uint32_t buf[3], nel, i, len;
> +	unsigned int entries;
>  	filename_trans_rule_t *ftr, *lftr;
>  	int rc;
>  
> @@ -3870,11 +3884,18 @@ static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_fi
>  		if (type_set_read(&ftr->ttypes, fp))
>  			return -1;
>  
> -		rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
> +		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +			entries = 3;
> +		else
> +			entries = 2;
> +
> +		rc = next_entry(buf, fp, sizeof(uint32_t) * entries);
>  		if (rc < 0)
>  			return -1;
>  		ftr->tclass = le32_to_cpu(buf[0]);
>  		ftr->otype = le32_to_cpu(buf[1]);
> +		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +			ftr->flags = le32_to_cpu(buf[2]);
>  	}
>  
>  	return 0;
> @@ -3977,7 +3998,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>  	}
>  
>  	if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> -	    filename_trans_rule_read(&decl->filename_trans_rules, fp))
> +	    filename_trans_rule_read(p, &decl->filename_trans_rules, fp))
>  		return -1;
>  
>  	if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index d7ac2b25..0b4f5d9a 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -1929,11 +1929,12 @@ static int role_allow_rule_write(role_allow_rule_t * r, struct policy_file *fp)
>  	return POLICYDB_SUCCESS;
>  }
>  
> -static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_file *fp)
> +static int filename_trans_rule_write(policydb_t *p, filename_trans_rule_t *t,
> +				     struct policy_file *fp)
>  {
>  	int nel = 0;
> -	size_t items;
> -	uint32_t buf[2], len;
> +	size_t items, entries;
> +	uint32_t buf[3], len;
>  	filename_trans_rule_t *ftr;
>  
>  	for (ftr = t; ftr; ftr = ftr->next)
> @@ -1962,10 +1963,14 @@ static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_fi
>  
>  		buf[0] = cpu_to_le32(ftr->tclass);
>  		buf[1] = cpu_to_le32(ftr->otype);
> +		buf[2] = cpu_to_le32(ftr->flags);
>  
> -		items = put_entry(buf, sizeof(uint32_t), 2, fp);
> -		if (items != 2)
> -			return POLICYDB_ERROR;
> +		if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +			entries = 3;
> +		else
> +			entries = 2;
> +
> +		items = put_entry(buf, sizeof(uint32_t), entries, fp);
>  	}
>  	return POLICYDB_SUCCESS;
>  }
> @@ -2039,7 +2044,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>  	}
>  
>  	if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> -	    filename_trans_rule_write(decl->filename_trans_rules, fp))
> +	    filename_trans_rule_write(p, decl->filename_trans_rules, fp))
>  		return POLICYDB_ERROR;
>  
>  	if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
Dominick Grift

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

* Re: [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions
  2022-04-22 15:43 ` [PATCH userspace 2/2] libsepol,checkpolicy: " Ondrej Mosnacek
  2022-04-22 17:02   ` Dominick Grift
@ 2022-04-23  7:42   ` Christian Göttsche
  2022-04-25 11:32     ` Ondrej Mosnacek
  1 sibling, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2022-04-23  7:42 UTC (permalink / raw)
  To: Ondrej Mosnacek; +Cc: SElinux list, Dominick Grift

On Fri, 22 Apr 2022 at 17:44, Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> With the addition of the anon_inode class in the kernel, 'self'
> transition rules became useful, but haven't been implemented.
>
> This patch implements the self keyword in all 'typetransition'
> statements at the TE language level and adds the support to the module
> policydb format. Note that changing the kernel policydb format is not
> necessary at all, as type transitions are always expanded in the kernel
> policydb.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  checkpolicy/policy_define.c                | 42 ++++++++++++-
>  libsepol/include/sepol/policydb/policydb.h |  4 +-
>  libsepol/src/expand.c                      | 69 ++++++++++++++--------
>  libsepol/src/link.c                        |  1 +
>  libsepol/src/module_to_cil.c               | 30 ++++++----
>  libsepol/src/policydb.c                    | 33 +++++++++--
>  libsepol/src/write.c                       | 19 +++---
>  7 files changed, 148 insertions(+), 50 deletions(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 16b78346..7e328edc 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -1634,6 +1634,15 @@ static int define_compute_type_helper(int which, avrule_t ** rule)
>         }
>         add = 1;
>         while ((id = queue_remove(id_queue))) {
> +               if (strcmp(id, "self") == 0) {
> +                       free(id);
> +                       if (add == 0) {
> +                               yyerror("-self is not supported");
> +                               goto bad;
> +                       }
> +                       avrule->flags |= RULE_SELF;
> +                       continue;
> +               }
>                 if (set_types(&avrule->ttypes, id, &add, 0))
>                         goto bad;
>         }
> @@ -3300,7 +3309,7 @@ int define_filename_trans(void)
>         type_datum_t *typdatum;
>         uint32_t otype;
>         unsigned int c, s, t;
> -       int add, rc;
> +       int add, self, rc;
>
>         if (pass == 1) {
>                 /* stype */
> @@ -3333,8 +3342,18 @@ int define_filename_trans(void)
>                         goto bad;
>         }
>
> -       add =1;
> +       self = 0;
> +       add = 1;
>         while ((id = queue_remove(id_queue))) {
> +               if (strcmp(id, "self") == 0) {
> +                       free(id);
> +                       if (add == 0) {
> +                               yyerror("-self is not supported");
> +                               goto bad;
> +                       }
> +                       self = 1;
> +                       continue;
> +               }
>                 if (set_types(&ttypes, id, &add, 0))
>                         goto bad;
>         }
> @@ -3396,6 +3415,24 @@ int define_filename_trans(void)
>                                         goto bad;
>                                 }
>                         }
> +                       if (self) {
> +                               rc = policydb_filetrans_insert(
> +                                       policydbp, s+1, s+1, c+1, name,
> +                                       NULL, otype, NULL
> +                               );
> +                               if (rc != SEPOL_OK) {
> +                                       if (rc == SEPOL_EEXIST) {
> +                                               yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
> +                                                       name,
> +                                                       policydbp->p_type_val_to_name[s],
> +                                                       policydbp->p_type_val_to_name[s],
> +                                                       policydbp->p_class_val_to_name[c]);
> +                                               goto bad;
> +                                       }
> +                                       yyerror("out of memory");
> +                                       goto bad;
> +                               }
> +                       }
>                 }
>
>                 /* Now add the real rule since we didn't find any duplicates */
> @@ -3418,6 +3455,7 @@ int define_filename_trans(void)
>                 }
>                 ftr->tclass = c + 1;
>                 ftr->otype = otype;
> +               ftr->flags = self ? RULE_SELF : 0;
>         }
>
>         free(name);
> diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> index 4bf9f05d..de0068a6 100644
> --- a/libsepol/include/sepol/policydb/policydb.h
> +++ b/libsepol/include/sepol/policydb/policydb.h
> @@ -314,6 +314,7 @@ typedef struct role_allow_rule {
>  } role_allow_rule_t;
>
>  typedef struct filename_trans_rule {
> +       uint32_t flags; /* may have RULE_SELF set */
>         type_set_t stypes;
>         type_set_t ttypes;
>         uint32_t tclass;
> @@ -781,9 +782,10 @@ extern int policydb_set_target_platform(policydb_t *p, int platform);
>  #define MOD_POLICYDB_VERSION_XPERMS_IOCTL  18
>  #define MOD_POLICYDB_VERSION_INFINIBAND                19
>  #define MOD_POLICYDB_VERSION_GLBLUB            20
> +#define MOD_POLICYDB_VERSION_SELF_TYPETRANS    21
>
>  #define MOD_POLICYDB_VERSION_MIN MOD_POLICYDB_VERSION_BASE
> -#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_GLBLUB
> +#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_SELF_TYPETRANS
>
>  #define POLICYDB_CONFIG_MLS    1
>
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 7316124f..8d19850e 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -1407,6 +1407,40 @@ static int copy_role_trans(expand_state_t * state, role_trans_rule_t * rules)
>         return 0;
>  }
>
> +static int expand_filename_trans_helper(expand_state_t *state,
> +                                       filename_trans_rule_t *rule,
> +                                       unsigned int s, unsigned int t)
> +{
> +       uint32_t mapped_otype, present_otype;
> +       int rc;
> +
> +       mapped_otype = state->typemap[rule->otype - 1];
> +
> +       rc = policydb_filetrans_insert(
> +               state->out, s + 1, t + 1,
> +               rule->tclass, rule->name,
> +               NULL, mapped_otype, &present_otype
> +       );
> +       if (rc == SEPOL_EEXIST) {
> +               /* duplicate rule, ignore */
> +               if (present_otype == mapped_otype)
> +                       return 0;
> +
> +               ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
> +                   state->out->p_type_val_to_name[s],
> +                   state->out->p_type_val_to_name[t],
> +                   state->out->p_class_val_to_name[rule->tclass - 1],
> +                   rule->name,
> +                   state->out->p_type_val_to_name[present_otype - 1],
> +                   state->out->p_type_val_to_name[mapped_otype - 1]);
> +               return -1;
> +       } else if (rc < 0) {
> +               ERR(state->handle, "Out of memory!");
> +               return -1;
> +       }
> +       return 0;
> +}
> +
>  static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *rules)
>  {
>         unsigned int i, j;
> @@ -1417,8 +1451,6 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
>
>         cur_rule = rules;
>         while (cur_rule) {
> -               uint32_t mapped_otype, present_otype;
> -
>                 ebitmap_init(&stypes);
>                 ebitmap_init(&ttypes);
>
> @@ -1434,32 +1466,21 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
>                         return -1;
>                 }
>
> -               mapped_otype = state->typemap[cur_rule->otype - 1];
>
>                 ebitmap_for_each_positive_bit(&stypes, snode, i) {
>                         ebitmap_for_each_positive_bit(&ttypes, tnode, j) {
> -                               rc = policydb_filetrans_insert(
> -                                       state->out, i + 1, j + 1,
> -                                       cur_rule->tclass, cur_rule->name,
> -                                       NULL, mapped_otype, &present_otype
> +                               rc = expand_filename_trans_helper(
> +                                       state, cur_rule, i, j
>                                 );
> -                               if (rc == SEPOL_EEXIST) {
> -                                       /* duplicate rule, ignore */
> -                                       if (present_otype == mapped_otype)
> -                                               continue;
> -
> -                                       ERR(state->handle, "Conflicting name-based type_transition %s %s:%s \"%s\":  %s vs %s",
> -                                           state->out->p_type_val_to_name[i],
> -                                           state->out->p_type_val_to_name[j],
> -                                           state->out->p_class_val_to_name[cur_rule->tclass - 1],
> -                                           cur_rule->name,
> -                                           state->out->p_type_val_to_name[present_otype - 1],
> -                                           state->out->p_type_val_to_name[mapped_otype - 1]);
> -                                       return -1;
> -                               } else if (rc < 0) {
> -                                       ERR(state->handle, "Out of memory!");
> -                                       return -1;
> -                               }
> +                               if (rc)
> +                                       return rc;
> +                       }
> +                       if (cur_rule->flags & RULE_SELF) {
> +                               rc = expand_filename_trans_helper(
> +                                       state, cur_rule, i, i
> +                               );
> +                               if (rc)
> +                                       return rc;
>                         }
>                 }
>
> diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> index ecfb5786..7e8313cb 100644
> --- a/libsepol/src/link.c
> +++ b/libsepol/src/link.c
> @@ -1482,6 +1482,7 @@ static int copy_filename_trans_list(filename_trans_rule_t * list,
>
>                 new_rule->tclass = module->map[SYM_CLASSES][cur->tclass - 1];
>                 new_rule->otype = module->map[SYM_TYPES][cur->otype - 1];
> +               new_rule->flags = cur->flags;
>
>                 cur = cur->next;
>         }
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 156a74a2..0e211c93 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -1608,20 +1608,30 @@ static int filename_trans_to_cil(int indent, struct policydb *pdb, struct filena
>                         goto exit;
>                 }
>
> -               ts = &rule->ttypes;
> -               rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
> -               if (rc != 0) {
> -                       goto exit;
> -               }
> -
> -               for (stype = 0; stype < num_stypes; stype++) {
> -                       for (ttype = 0; ttype < num_ttypes; ttype++) {
> -                               cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
> -                                           stypes[stype], ttypes[ttype],
> +               if (rule->flags & RULE_SELF) {
> +                       for (stype = 0; stype < num_stypes; stype++) {
> +                               cil_println(indent, "(typetransition %s self %s \"%s\" %s)",
> +                                           stypes[stype],
>                                             pdb->p_class_val_to_name[rule->tclass - 1],
>                                             rule->name,
>                                             pdb->p_type_val_to_name[rule->otype - 1]);
>                         }
> +               } else {
> +                       ts = &rule->ttypes;
> +                       rc = process_typeset(pdb, ts, attr_list, &ttypes, &num_ttypes);
> +                       if (rc != 0) {
> +                               goto exit;
> +                       }
> +
> +                       for (stype = 0; stype < num_stypes; stype++) {
> +                               for (ttype = 0; ttype < num_ttypes; ttype++) {
> +                                       cil_println(indent, "(typetransition %s %s %s \"%s\" %s)",
> +                                                   stypes[stype], ttypes[ttype],
> +                                                   pdb->p_class_val_to_name[rule->tclass - 1],
> +                                                   rule->name,
> +                                                   pdb->p_type_val_to_name[rule->otype - 1]);
> +                               }
> +                       }
>                 }
>
>                 names_destroy(&stypes, &num_stypes);
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 7c99571f..fc260eb6 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -327,6 +327,13 @@ static const struct policydb_compat_info policydb_compat[] = {
>          .ocon_num = OCON_IBENDPORT + 1,
>          .target_platform = SEPOL_TARGET_SELINUX,
>         },
> +       {
> +        .type = POLICY_BASE,
> +        .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
> +        .sym_num = SYM_NUM,
> +        .ocon_num = OCON_IBENDPORT + 1,
> +        .target_platform = SEPOL_TARGET_SELINUX,
> +       },
>         {
>          .type = POLICY_MOD,
>          .version = MOD_POLICYDB_VERSION_BASE,
> @@ -446,7 +453,13 @@ static const struct policydb_compat_info policydb_compat[] = {
>          .ocon_num = 0,
>          .target_platform = SEPOL_TARGET_SELINUX,
>         },
> -
> +       {
> +        .type = POLICY_MOD,
> +        .version = MOD_POLICYDB_VERSION_SELF_TYPETRANS,
> +        .sym_num = SYM_NUM,
> +        .ocon_num = 0,
> +        .target_platform = SEPOL_TARGET_SELINUX,
> +       },
>  };
>
>  #if 0
> @@ -3822,10 +3835,11 @@ static int role_allow_rule_read(role_allow_rule_t ** r, struct policy_file *fp)
>         return 0;
>  }
>
> -static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_file *fp)
> +static int filename_trans_rule_read(policydb_t *p, filename_trans_rule_t **r,
> +                                   struct policy_file *fp)
>  {
> -       uint32_t buf[2], nel;
> -       unsigned int i, len;
> +       uint32_t buf[3], nel, i, len;
> +       unsigned int entries;
>         filename_trans_rule_t *ftr, *lftr;
>         int rc;
>
> @@ -3870,11 +3884,18 @@ static int filename_trans_rule_read(filename_trans_rule_t ** r, struct policy_fi
>                 if (type_set_read(&ftr->ttypes, fp))
>                         return -1;
>
> -               rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
> +               if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +                       entries = 3;
> +               else
> +                       entries = 2;
> +
> +               rc = next_entry(buf, fp, sizeof(uint32_t) * entries);
>                 if (rc < 0)
>                         return -1;
>                 ftr->tclass = le32_to_cpu(buf[0]);
>                 ftr->otype = le32_to_cpu(buf[1]);
> +               if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +                       ftr->flags = le32_to_cpu(buf[2]);
>         }
>
>         return 0;
> @@ -3977,7 +3998,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>         }
>
>         if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> -           filename_trans_rule_read(&decl->filename_trans_rules, fp))
> +           filename_trans_rule_read(p, &decl->filename_trans_rules, fp))
>                 return -1;
>
>         if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index d7ac2b25..0b4f5d9a 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -1929,11 +1929,12 @@ static int role_allow_rule_write(role_allow_rule_t * r, struct policy_file *fp)
>         return POLICYDB_SUCCESS;
>  }
>
> -static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_file *fp)
> +static int filename_trans_rule_write(policydb_t *p, filename_trans_rule_t *t,
> +                                    struct policy_file *fp)
>  {
>         int nel = 0;
> -       size_t items;
> -       uint32_t buf[2], len;
> +       size_t items, entries;
> +       uint32_t buf[3], len;
>         filename_trans_rule_t *ftr;
>
>         for (ftr = t; ftr; ftr = ftr->next)
> @@ -1962,10 +1963,14 @@ static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_fi
>
>                 buf[0] = cpu_to_le32(ftr->tclass);
>                 buf[1] = cpu_to_le32(ftr->otype);
> +               buf[2] = cpu_to_le32(ftr->flags);
>
> -               items = put_entry(buf, sizeof(uint32_t), 2, fp);
> -               if (items != 2)
> -                       return POLICYDB_ERROR;
> +               if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> +                       entries = 3;
> +               else
> +                       entries = 2;
> +
> +               items = put_entry(buf, sizeof(uint32_t), entries, fp);

+ if (items != entries)
+     return POLICYDB_ERROR;

>         }
>         return POLICYDB_SUCCESS;
>  }
> @@ -2039,7 +2044,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>         }
>
>         if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> -           filename_trans_rule_write(decl->filename_trans_rules, fp))
> +           filename_trans_rule_write(p, decl->filename_trans_rules, fp))
>                 return POLICYDB_ERROR;
>
>         if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
> --
> 2.35.1
>

--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -1184,6 +1184,14 @@ static int
validate_filename_trans_rules(sepol_handle_t *handle, filename_trans_
                       goto bad;
               if (validate_value(filename_trans->otype, &flavors[SYM_TYPES]))
                       goto bad;
+
+               switch (filename_trans->flags) {
+               case 0:
+               case RULE_SELF:
+                       break;
+               default:
+                       goto bad;
+               }
       }

       return 0;

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

* Re: [PATCH userspace 0/2] Support the 'self' keyword in type transitions
  2022-04-22 15:43 [PATCH userspace 0/2] Support the 'self' keyword in type transitions Ondrej Mosnacek
  2022-04-22 15:43 ` [PATCH userspace 1/2] libsepol/cil: add support for self " Ondrej Mosnacek
  2022-04-22 15:43 ` [PATCH userspace 2/2] libsepol,checkpolicy: " Ondrej Mosnacek
@ 2022-04-23  7:52 ` Christian Göttsche
  2022-04-25 11:29   ` Ondrej Mosnacek
  2 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2022-04-23  7:52 UTC (permalink / raw)
  To: Ondrej Mosnacek; +Cc: SElinux list, Dominick Grift

On Fri, 22 Apr 2022 at 17:44, Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> With the addition of the anon_inode class in the kernel, 'self'
> transition rules became useful, but haven't been implemented.
>
> This series implements the self keyword support in the CIL & TE
> languages and the module policydb format. The kernel policydb format
> doesn't need any changes, as type transitions are always expanded in the
> kernel policydb.

Since the type transitions are expanded a single usage of

    type_transition domain self iouring_t:anon_inode "[io_uring]";

will result of thousands of filetrans entries in the binary policy.
When using a limited type-attribute

     type_transition iouring_domain self iouring_t:anon_inode "[io_uring]";

what is the benefit of implementing the interface kernel_iouring_domain() as

    typeattribute $1 iouring_domain;

instead of

    type_transition $1 $1 iouring_t:anon_inode "[io_uring]";

?

Wouldn't true policydb support be much more efficient (not only
regarding size but also (lookup) performance)?

> The patches have been tested using the following WIP beakerlib/tmt test:
> https://src.fedoraproject.org/fork/omos/tests/selinux/blob/self-in-tt/f/libsepol/self-keyword-in-type-transitions
>
> Ondrej Mosnacek (2):
>   libsepol/cil: add support for self keyword in type transitions
>   libsepol,checkpolicy: add support for self keyword in type transitions
>
>  checkpolicy/policy_define.c                |  42 +++++-
>  libsepol/cil/src/cil_binary.c              | 168 +++++++++++++++------
>  libsepol/cil/src/cil_resolve_ast.c         |  25 ++-
>  libsepol/include/sepol/policydb/policydb.h |   4 +-
>  libsepol/src/expand.c                      |  69 ++++++---
>  libsepol/src/link.c                        |   1 +
>  libsepol/src/module_to_cil.c               |  30 ++--
>  libsepol/src/policydb.c                    |  33 +++-
>  libsepol/src/write.c                       |  19 ++-
>  secilc/test/policy.cil                     |   3 +
>  10 files changed, 293 insertions(+), 101 deletions(-)
>
> --
> 2.35.1
>

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

* Re: [PATCH userspace 0/2] Support the 'self' keyword in type transitions
  2022-04-23  7:52 ` [PATCH userspace 0/2] Support the 'self' " Christian Göttsche
@ 2022-04-25 11:29   ` Ondrej Mosnacek
  0 siblings, 0 replies; 8+ messages in thread
From: Ondrej Mosnacek @ 2022-04-25 11:29 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list, Dominick Grift

On Sat, Apr 23, 2022 at 9:52 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Fri, 22 Apr 2022 at 17:44, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > With the addition of the anon_inode class in the kernel, 'self'
> > transition rules became useful, but haven't been implemented.
> >
> > This series implements the self keyword support in the CIL & TE
> > languages and the module policydb format. The kernel policydb format
> > doesn't need any changes, as type transitions are always expanded in the
> > kernel policydb.
>
> Since the type transitions are expanded a single usage of
>
>     type_transition domain self iouring_t:anon_inode "[io_uring]";
>
> will result of thousands of filetrans entries in the binary policy.
> When using a limited type-attribute
>
>      type_transition iouring_domain self iouring_t:anon_inode "[io_uring]";
>
> what is the benefit of implementing the interface kernel_iouring_domain() as
>
>     typeattribute $1 iouring_domain;
>
> instead of
>
>     type_transition $1 $1 iouring_t:anon_inode "[io_uring]";
>
> ?

Indeed currently it will only make a cosmetic difference. Still, I
think it is more intuitive to write the self rule than to add a rule
into the interface. Plus, it would allow you to write today a policy
that is ready for policy handling optimizations in the future (e.g. by
implementing a better attribute support for type transitions).

> Wouldn't true policydb support be much more efficient (not only
> regarding size but also (lookup) performance)?

It would only be more space-efficient. The lookup performance would
actually get worse, since you would need to look up the cartesian
product of all attributes for source and target in case of attribute
support. The individual lookups would not become faster by making the
table smaller, since the hash table size already scales with the
number of rules (elements).

> > The patches have been tested using the following WIP beakerlib/tmt test:
> > https://src.fedoraproject.org/fork/omos/tests/selinux/blob/self-in-tt/f/libsepol/self-keyword-in-type-transitions
> >
> > Ondrej Mosnacek (2):
> >   libsepol/cil: add support for self keyword in type transitions
> >   libsepol,checkpolicy: add support for self keyword in type transitions
> >
> >  checkpolicy/policy_define.c                |  42 +++++-
> >  libsepol/cil/src/cil_binary.c              | 168 +++++++++++++++------
> >  libsepol/cil/src/cil_resolve_ast.c         |  25 ++-
> >  libsepol/include/sepol/policydb/policydb.h |   4 +-
> >  libsepol/src/expand.c                      |  69 ++++++---
> >  libsepol/src/link.c                        |   1 +
> >  libsepol/src/module_to_cil.c               |  30 ++--
> >  libsepol/src/policydb.c                    |  33 +++-
> >  libsepol/src/write.c                       |  19 ++-
> >  secilc/test/policy.cil                     |   3 +
> >  10 files changed, 293 insertions(+), 101 deletions(-)
> >
> > --
> > 2.35.1
> >
>

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.


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

* Re: [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions
  2022-04-23  7:42   ` Christian Göttsche
@ 2022-04-25 11:32     ` Ondrej Mosnacek
  0 siblings, 0 replies; 8+ messages in thread
From: Ondrej Mosnacek @ 2022-04-25 11:32 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list, Dominick Grift

On Sat, Apr 23, 2022 at 9:43 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Fri, 22 Apr 2022 at 17:44, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > With the addition of the anon_inode class in the kernel, 'self'
> > transition rules became useful, but haven't been implemented.
> >
> > This patch implements the self keyword in all 'typetransition'
> > statements at the TE language level and adds the support to the module
> > policydb format. Note that changing the kernel policydb format is not
> > necessary at all, as type transitions are always expanded in the kernel
> > policydb.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  checkpolicy/policy_define.c                | 42 ++++++++++++-
> >  libsepol/include/sepol/policydb/policydb.h |  4 +-
> >  libsepol/src/expand.c                      | 69 ++++++++++++++--------
> >  libsepol/src/link.c                        |  1 +
> >  libsepol/src/module_to_cil.c               | 30 ++++++----
> >  libsepol/src/policydb.c                    | 33 +++++++++--
> >  libsepol/src/write.c                       | 19 +++---
> >  7 files changed, 148 insertions(+), 50 deletions(-)
> >
[...]
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index d7ac2b25..0b4f5d9a 100644
> > --- a/libsepol/src/write.c
> > +++ b/libsepol/src/write.c
> > @@ -1929,11 +1929,12 @@ static int role_allow_rule_write(role_allow_rule_t * r, struct policy_file *fp)
> >         return POLICYDB_SUCCESS;
> >  }
> >
> > -static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_file *fp)
> > +static int filename_trans_rule_write(policydb_t *p, filename_trans_rule_t *t,
> > +                                    struct policy_file *fp)
> >  {
> >         int nel = 0;
> > -       size_t items;
> > -       uint32_t buf[2], len;
> > +       size_t items, entries;
> > +       uint32_t buf[3], len;
> >         filename_trans_rule_t *ftr;
> >
> >         for (ftr = t; ftr; ftr = ftr->next)
> > @@ -1962,10 +1963,14 @@ static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_fi
> >
> >                 buf[0] = cpu_to_le32(ftr->tclass);
> >                 buf[1] = cpu_to_le32(ftr->otype);
> > +               buf[2] = cpu_to_le32(ftr->flags);
> >
> > -               items = put_entry(buf, sizeof(uint32_t), 2, fp);
> > -               if (items != 2)
> > -                       return POLICYDB_ERROR;
> > +               if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> > +                       entries = 3;
> > +               else
> > +                       entries = 2;
> > +
> > +               items = put_entry(buf, sizeof(uint32_t), entries, fp);
>
> + if (items != entries)
> +     return POLICYDB_ERROR;
>
> >         }
> >         return POLICYDB_SUCCESS;
> >  }
> > @@ -2039,7 +2044,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
> >         }
> >
> >         if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> > -           filename_trans_rule_write(decl->filename_trans_rules, fp))
> > +           filename_trans_rule_write(p, decl->filename_trans_rules, fp))
> >                 return POLICYDB_ERROR;
> >
> >         if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
> > --
> > 2.35.1
> >
>
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -1184,6 +1184,14 @@ static int
> validate_filename_trans_rules(sepol_handle_t *handle, filename_trans_
>                        goto bad;
>                if (validate_value(filename_trans->otype, &flavors[SYM_TYPES]))
>                        goto bad;
> +
> +               switch (filename_trans->flags) {
> +               case 0:
> +               case RULE_SELF:
> +                       break;
> +               default:
> +                       goto bad;
> +               }
>        }
>
>        return 0;

Good catches, thanks! I will include these changes in the next respin.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.


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

end of thread, other threads:[~2022-04-25 11:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-22 15:43 [PATCH userspace 0/2] Support the 'self' keyword in type transitions Ondrej Mosnacek
2022-04-22 15:43 ` [PATCH userspace 1/2] libsepol/cil: add support for self " Ondrej Mosnacek
2022-04-22 15:43 ` [PATCH userspace 2/2] libsepol,checkpolicy: " Ondrej Mosnacek
2022-04-22 17:02   ` Dominick Grift
2022-04-23  7:42   ` Christian Göttsche
2022-04-25 11:32     ` Ondrej Mosnacek
2022-04-23  7:52 ` [PATCH userspace 0/2] Support the 'self' " Christian Göttsche
2022-04-25 11:29   ` Ondrej Mosnacek

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.