[RFC PATCH 1/3] libsepol: introduce ebitmap_subtract()

[RFC PATCH 1/3] libsepol: introduce ebitmap_subtract()

[Christian Göttsche]

Add a subtract method for ebitmaps.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/include/sepol/policydb/ebitmap.h |  1 +
 libsepol/src/ebitmap.c                    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
index 81d0c7a6..daca67a2 100644
--- a/libsepol/include/sepol/policydb/ebitmap.h
+++ b/libsepol/include/sepol/policydb/ebitmap.h
@@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
 extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
+extern int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1);
 extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
index 1de3816a..faa9e42e 100644
--- a/libsepol/src/ebitmap.c
+++ b/libsepol/src/ebitmap.c
@@ -72,6 +72,19 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
 	return 0;
 }
 
+int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1)
+{
+	unsigned int i, length = min(ebitmap_length(dst), ebitmap_length(e1));
+	for (i=0; i < length; i++) {
+		if (ebitmap_get_bit(e1, i)) {
+			int rc = ebitmap_set_bit(dst, i, 0);
+			if (rc < 0)
+				return rc;
+		}
+	}
+	return 0;
+}
+
 int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
 {
 	unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
-- 
2.34.0

[RFC PATCH 2/3] libsepol: add not-self neverallow support

[Christian Göttsche]

Add support for not-self neverallow rules. These do not trigger on allow
rules where the source type is exactly equal to the target type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/include/sepol/policydb/policydb.h |  3 +-
 libsepol/src/assertion.c                   | 39 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 4bf9f05d..da5256e8 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -285,7 +285,8 @@ typedef struct avrule {
 #define AVRULE_XPERMS	(AVRULE_XPERMS_ALLOWED | AVRULE_XPERMS_AUDITALLOW | \
 				AVRULE_XPERMS_DONTAUDIT | AVRULE_XPERMS_NEVERALLOW)
 	uint32_t specified;
-#define RULE_SELF 1
+#define RULE_SELF       (1U << 1)
+#define RULE_NOTSELF    (1U << 2)
 	uint32_t flags;
 	type_set_t stypes;
 	type_set_t ttypes;
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index dd2749a0..9bbeddb0 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -241,7 +241,7 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
 		if (rc)
 			goto oom;
@@ -268,6 +268,8 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 
 		ebitmap_for_each_positive_bit(&src_matches, snode, i) {
 			ebitmap_for_each_positive_bit(&tgt_matches, tnode, j) {
+				if ((avrule->flags & RULE_NOTSELF) && i == j)
+					continue;
 				if (avrule->specified == AVRULE_XPERMS_NEVERALLOW) {
 					a->errors += report_assertion_extended_permissions(handle,p, avrule,
 											i, j, cp, perms, k, avtab);
@@ -402,7 +404,7 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1],
 				&p->attr_type_map[k->target_type - 1]);
 		if (rc)
@@ -418,6 +420,21 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 		}
 	}
 
+	if (avrule->flags & RULE_NOTSELF) {
+		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
+		if (rc)
+			goto oom;
+		rc = ebitmap_and(&self_matches, &avrule->ttypes.types, &matches);
+		if (rc)
+			goto oom;
+
+		if (!ebitmap_is_empty(&self_matches)) {
+			rc = ebitmap_subtract(&tgt_matches, &self_matches);
+			if (rc)
+				goto oom;
+		}
+	}
+
 	if (ebitmap_is_empty(&tgt_matches))
 		goto exit;
 
@@ -463,7 +480,7 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 	if (rc == 0)
 		goto exit;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		/* If the neverallow uses SELF, then it is not enough that the
 		 * neverallow's source matches the src and tgt of the rule being checked.
 		 * It must match the same thing in the src and tgt, so AND the source
@@ -479,6 +496,22 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 		ebitmap_destroy(&match);
 	}
 
+	if (avrule->flags & RULE_NOTSELF) {
+		ebitmap_t match;
+		rc = ebitmap_cpy(&match, &p->attr_type_map[k->source_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc = ebitmap_subtract(&match, &p->attr_type_map[k->target_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc2 = ebitmap_match_any(&avrule->ttypes.types, &match);
+		ebitmap_destroy(&match);
+	}
+
 	/* neverallow may have tgts even if it uses SELF */
 	rc = ebitmap_match_any(&avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
 	if (rc == 0 && rc2 == 0)
-- 
2.34.0

[RFC PATCH 3/3] checkpolicy: add not-self neverallow support

[Christian Göttsche]

Add support for using negated or complemented self in the target type of
neverallow rules.

Some refpolicy examples:

    neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *;
    # no violations

    neverallow domain domain:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

    neverallow domain { domain -self }:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

Using negated self in a complement `~{ domain -self }` is not supported.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++-----
 checkpolicy/test/dismod.c   |  6 ++++-
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index d3eb6111..662b35e7 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_NEVERALLOW) {
+				yyerror("-self is only supported in neverallowx rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
@@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_NEVERALLOW) {
+				yyerror("-self is only supported in neverallow rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index ec2a3e9a..a2d74d42 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 	}
 
 	num_types = 0;
-	if (flags & RULE_SELF) {
+	if (flags & (RULE_SELF | RULE_NOTSELF)) {
 		num_types++;
 	}
 
@@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 		fprintf(fp, " self");
 	}
 
+	if (flags & RULE_NOTSELF) {
+		fprintf(fp, " -self");
+	}
+
 	if (num_types > 1)
 		fprintf(fp, " }");
 
-- 
2.34.0

[RFC PATCH v2 1/4] libsepol: introduce ebitmap_subtract()

[Christian Göttsche]

Add a subtract method for ebitmaps.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
   - add shortcut for empty ebitmaps
---
 libsepol/include/sepol/policydb/ebitmap.h |  1 +
 libsepol/src/ebitmap.c                    | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
index 81d0c7a6..daca67a2 100644
--- a/libsepol/include/sepol/policydb/ebitmap.h
+++ b/libsepol/include/sepol/policydb/ebitmap.h
@@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
 extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
+extern int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1);
 extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
index 1de3816a..80f0e201 100644
--- a/libsepol/src/ebitmap.c
+++ b/libsepol/src/ebitmap.c
@@ -72,6 +72,24 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
 	return 0;
 }
 
+int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1)
+{
+	unsigned int i, length;
+
+	if (ebitmap_is_empty(dst) || ebitmap_is_empty(e1))
+		return 0;
+	
+	length = min(ebitmap_length(dst), ebitmap_length(e1));
+	for (i=0; i < length; i++) {
+		if (ebitmap_get_bit(e1, i)) {
+			int rc = ebitmap_set_bit(dst, i, 0);
+			if (rc < 0)
+				return rc;
+		}
+	}
+	return 0;
+}
+
 int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
 {
 	unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
-- 
2.34.0

[RFC PATCH v2 2/4] libsepol: add not-self neverallow support

[Christian Göttsche]

Add support for not-self neverallow rules. These do not trigger on allow
rules where the source type is exactly equal to the target type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
  - do not change the value of RULE_SELF
---
 libsepol/include/sepol/policydb/policydb.h |  3 +-
 libsepol/src/assertion.c                   | 36 ++++++++++++++++++++--
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 4bf9f05d..11637fe8 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -285,7 +285,8 @@ typedef struct avrule {
 #define AVRULE_XPERMS	(AVRULE_XPERMS_ALLOWED | AVRULE_XPERMS_AUDITALLOW | \
 				AVRULE_XPERMS_DONTAUDIT | AVRULE_XPERMS_NEVERALLOW)
 	uint32_t specified;
-#define RULE_SELF 1
+#define RULE_SELF       (1U << 0)
+#define RULE_NOTSELF    (1U << 1)
 	uint32_t flags;
 	type_set_t stypes;
 	type_set_t ttypes;
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index dd2749a0..efa136c8 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -241,7 +241,7 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
 		if (rc)
 			goto oom;
@@ -268,6 +268,8 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 
 		ebitmap_for_each_positive_bit(&src_matches, snode, i) {
 			ebitmap_for_each_positive_bit(&tgt_matches, tnode, j) {
+				if ((avrule->flags & RULE_NOTSELF) && i == j)
+					continue;
 				if (avrule->specified == AVRULE_XPERMS_NEVERALLOW) {
 					a->errors += report_assertion_extended_permissions(handle,p, avrule,
 											i, j, cp, perms, k, avtab);
@@ -402,7 +404,7 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1],
 				&p->attr_type_map[k->target_type - 1]);
 		if (rc)
@@ -418,6 +420,18 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 		}
 	}
 
+	if (avrule->flags & RULE_NOTSELF) {
+		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
+		if (rc)
+			goto oom;
+		rc = ebitmap_and(&self_matches, &avrule->ttypes.types, &matches);
+		if (rc)
+			goto oom;
+		rc = ebitmap_subtract(&tgt_matches, &self_matches);
+		if (rc)
+			goto oom;
+	}
+
 	if (ebitmap_is_empty(&tgt_matches))
 		goto exit;
 
@@ -463,7 +477,7 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 	if (rc == 0)
 		goto exit;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		/* If the neverallow uses SELF, then it is not enough that the
 		 * neverallow's source matches the src and tgt of the rule being checked.
 		 * It must match the same thing in the src and tgt, so AND the source
@@ -479,6 +493,22 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 		ebitmap_destroy(&match);
 	}
 
+	if (avrule->flags & RULE_NOTSELF) {
+		ebitmap_t match;
+		rc = ebitmap_cpy(&match, &p->attr_type_map[k->source_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc = ebitmap_subtract(&match, &p->attr_type_map[k->target_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc2 = ebitmap_match_any(&avrule->ttypes.types, &match);
+		ebitmap_destroy(&match);
+	}
+
 	/* neverallow may have tgts even if it uses SELF */
 	rc = ebitmap_match_any(&avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
 	if (rc == 0 && rc2 == 0)
-- 
2.34.0

[RFC PATCH v2 3/4] checkpolicy: add not-self neverallow support

[Christian Göttsche]

Add support for using negated or complemented self in the target type of
neverallow rules.

Some refpolicy examples:

    neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *;
    # no violations

    neverallow domain domain:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

    neverallow domain { domain -self }:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

Using negated self in a complement `~{ domain -self }` is not supported.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
   - fix neverallowxperm usage
---
 checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++-----
 checkpolicy/test/dismod.c   |  6 ++++-
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index d3eb6111..f27a6f33 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) {
+				yyerror("-self is only supported in neverallowxperm rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
@@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_NEVERALLOW) {
+				yyerror("-self is only supported in neverallow rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index ec2a3e9a..a2d74d42 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 	}
 
 	num_types = 0;
-	if (flags & RULE_SELF) {
+	if (flags & (RULE_SELF | RULE_NOTSELF)) {
 		num_types++;
 	}
 
@@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 		fprintf(fp, " self");
 	}
 
+	if (flags & RULE_NOTSELF) {
+		fprintf(fp, " -self");
+	}
+
 	if (num_types > 1)
 		fprintf(fp, " }");
 
-- 
2.34.0

[RFC PATCH v2 4/4] libsepol: free ebitmap on end of function

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/assertion.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index efa136c8..429e61f5 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -455,6 +455,7 @@ oom:
 exit:
 	ebitmap_destroy(&src_matches);
 	ebitmap_destroy(&tgt_matches);
+	ebitmap_destroy(&self_matches);
 	ebitmap_destroy(&matches);
 	return ret;
 }
-- 
2.34.0

Re: [RFC PATCH v2 1/4] libsepol: introduce ebitmap_subtract()

[James Carter]

On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add a subtract method for ebitmaps.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
>    - add shortcut for empty ebitmaps
> ---
>  libsepol/include/sepol/policydb/ebitmap.h |  1 +
>  libsepol/src/ebitmap.c                    | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
> index 81d0c7a6..daca67a2 100644
> --- a/libsepol/include/sepol/policydb/ebitmap.h
> +++ b/libsepol/include/sepol/policydb/ebitmap.h
> @@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
>  extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
>  extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
>  extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
> +extern int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1);
>  extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
>  extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
>  extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
> diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
> index 1de3816a..80f0e201 100644
> --- a/libsepol/src/ebitmap.c
> +++ b/libsepol/src/ebitmap.c
> @@ -72,6 +72,24 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
>         return 0;
>  }
>
> +int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1)
> +{
> +       unsigned int i, length;
> +
> +       if (ebitmap_is_empty(dst) || ebitmap_is_empty(e1))
> +               return 0;
> +
> +       length = min(ebitmap_length(dst), ebitmap_length(e1));
> +       for (i=0; i < length; i++) {
> +               if (ebitmap_get_bit(e1, i)) {
> +                       int rc = ebitmap_set_bit(dst, i, 0);
> +                       if (rc < 0)
> +                               return rc;
> +               }
> +       }
> +       return 0;
> +}
> +
>  int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
>  {
>         unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
> --
> 2.34.0
>

We already have ebitmap_andnot() which does the same thing (although
it does take three ebitmaps). Also, subtract is not a bit operation,
so it doesn't seem to fit.
Thanks,
Jim

Re: [RFC PATCH v2 1/4] libsepol: introduce ebitmap_subtract()

[Christian Göttsche]

On Mon, 29 Nov 2021 at 18:48, James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Add a subtract method for ebitmaps.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> > v2:
> >    - add shortcut for empty ebitmaps
> > ---
> >  libsepol/include/sepol/policydb/ebitmap.h |  1 +
> >  libsepol/src/ebitmap.c                    | 18 ++++++++++++++++++
> >  2 files changed, 19 insertions(+)
> >
> > diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
> > index 81d0c7a6..daca67a2 100644
> > --- a/libsepol/include/sepol/policydb/ebitmap.h
> > +++ b/libsepol/include/sepol/policydb/ebitmap.h
> > @@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
> >  extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
> >  extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
> >  extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
> > +extern int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1);
> >  extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
> >  extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
> >  extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
> > diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
> > index 1de3816a..80f0e201 100644
> > --- a/libsepol/src/ebitmap.c
> > +++ b/libsepol/src/ebitmap.c
> > @@ -72,6 +72,24 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
> >         return 0;
> >  }
> >
> > +int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1)
> > +{
> > +       unsigned int i, length;
> > +
> > +       if (ebitmap_is_empty(dst) || ebitmap_is_empty(e1))
> > +               return 0;
> > +
> > +       length = min(ebitmap_length(dst), ebitmap_length(e1));
> > +       for (i=0; i < length; i++) {
> > +               if (ebitmap_get_bit(e1, i)) {
> > +                       int rc = ebitmap_set_bit(dst, i, 0);
> > +                       if (rc < 0)
> > +                               return rc;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > +
> >  int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
> >  {
> >         unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
> > --
> > 2.34.0
> >
>
> We already have ebitmap_andnot() which does the same thing (although
> it does take three ebitmaps). Also, subtract is not a bit operation,
> so it doesn't seem to fit.

ebitmap_andnot() does perform a not and an and operation (so two
iterations over the ebitmaps) and allocates a temporary ebitmap on the
way.
So I think this new helper has its justification for performance reasons.
Do you prefer another name, e.g. ebitmap_rel_comp() (see
https://en.wikipedia.org/wiki/Complement_(set_theory)#Relative_complement)?

> Thanks,
> Jim

Re: [RFC PATCH v2 1/4] libsepol: introduce ebitmap_subtract()

[James Carter]

On Tue, Nov 30, 2021 at 6:13 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Mon, 29 Nov 2021 at 18:48, James Carter <jwcart2@gmail.com> wrote:
> >
> > On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > >
> > > Add a subtract method for ebitmaps.
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > > v2:
> > >    - add shortcut for empty ebitmaps
> > > ---
> > >  libsepol/include/sepol/policydb/ebitmap.h |  1 +
> > >  libsepol/src/ebitmap.c                    | 18 ++++++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > >
> > > diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
> > > index 81d0c7a6..daca67a2 100644
> > > --- a/libsepol/include/sepol/policydb/ebitmap.h
> > > +++ b/libsepol/include/sepol/policydb/ebitmap.h
> > > @@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
> > >  extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
> > >  extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
> > >  extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
> > > +extern int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1);
> > >  extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
> > >  extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
> > >  extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
> > > diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
> > > index 1de3816a..80f0e201 100644
> > > --- a/libsepol/src/ebitmap.c
> > > +++ b/libsepol/src/ebitmap.c
> > > @@ -72,6 +72,24 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
> > >         return 0;
> > >  }
> > >
> > > +int ebitmap_subtract(ebitmap_t *dst, const ebitmap_t *e1)
> > > +{
> > > +       unsigned int i, length;
> > > +
> > > +       if (ebitmap_is_empty(dst) || ebitmap_is_empty(e1))
> > > +               return 0;
> > > +
> > > +       length = min(ebitmap_length(dst), ebitmap_length(e1));
> > > +       for (i=0; i < length; i++) {
> > > +               if (ebitmap_get_bit(e1, i)) {
> > > +                       int rc = ebitmap_set_bit(dst, i, 0);
> > > +                       if (rc < 0)
> > > +                               return rc;
> > > +               }
> > > +       }
> > > +       return 0;
> > > +}
> > > +
> > >  int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
> > >  {
> > >         unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
> > > --
> > > 2.34.0
> > >
> >
> > We already have ebitmap_andnot() which does the same thing (although
> > it does take three ebitmaps). Also, subtract is not a bit operation,
> > so it doesn't seem to fit.
>
> ebitmap_andnot() does perform a not and an and operation (so two
> iterations over the ebitmaps) and allocates a temporary ebitmap on the
> way.
> So I think this new helper has its justification for performance reasons.
> Do you prefer another name, e.g. ebitmap_rel_comp() (see
> https://en.wikipedia.org/wiki/Complement_(set_theory)#Relative_complement)?
>

I do prefer that name and will be fine with the patch with that name.
Thanks,
Jim

> > Thanks,
> > Jim

Re: [RFC PATCH v2 3/4] checkpolicy: add not-self neverallow support

[James Carter]

On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add support for using negated or complemented self in the target type of
> neverallow rules.
>
> Some refpolicy examples:
>
>     neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *;
>     # no violations
>
>     neverallow domain domain:file ~{ append read_file_perms write };
>
>     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
>     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create };
>     libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };
>
>     neverallow domain { domain -self }:file ~{ append read_file_perms write };
>
>     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
>     libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };
>
> Using negated self in a complement `~{ domain -self }` is not supported.
>

I am thinking about what to do with this patch set. If this is
valuable in checkpolicy, then I would like it in CIL as well. But CIL
obviously cannot support the above syntax.

What would be lost if we used "notself"?

We could define the behavior of "neverallow SRC notself . . ." so that
if SRC was a type, the rule would be expanded so that the target types
would be all types except SRC, and if SRC was an attribute the rules
would be expanded into multiple rules with all combinations of the
types in SRC used for the source and target except for the cases where
the source and target type would be the same.
That would make your examples work.
"neverallow * notself . . ." would expand like "neverallow * ~self . . ."
"neverallow domain notself . . ." would expand like "neverallow domain
{ domain -self } . . ."
"neverallow domain notself . . ." and "neverallow domain ~domain . .
." together would expand like "neverallow domain ~self . . ." (I
think)

What would be missing would be the ability to express "neverallow
domain { subdomain -self } . . ."

A few minor comments below.

> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
>    - fix neverallowxperm usage
> ---
>  checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++-----
>  checkpolicy/test/dismod.c   |  6 ++++-
>  2 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index d3eb6111..f27a6f33 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
>         while ((id = queue_remove(id_queue))) {
>                 if (strcmp(id, "self") == 0) {
>                         free(id);
> -                       if (add == 0) {
> -                               yyerror("-self is not supported");
> +                       if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) {
> +                               yyerror("-self is only supported in neverallowxperm rules");

"-self is only supported in neverallow and neverallowxperm rules"

> +                               ret = -1;
> +                               goto out;
> +                       }
> +                       avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
> +                       if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
> +                               yyerror("self and -self is not supported");
>                                 ret = -1;
>                                 goto out;
>                         }
> -                       avrule->flags |= RULE_SELF;
>                         continue;
>                 }
>                 if (set_types
> @@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
>                 }
>         }
>
> +       if ((avrule->ttypes.flags & TYPE_COMP)) {
> +               if (avrule->flags & RULE_NOTSELF) {
> +                       yyerror("-self is not supported in complements");
> +                       ret = -1;
> +                       goto out;
> +               }
> +               if (avrule->flags & RULE_SELF) {
> +                       avrule->flags &= ~RULE_SELF;
> +                       avrule->flags |= RULE_NOTSELF;
> +               }
> +       }
> +
>         ebitmap_init(&tclasses);
>         ret = read_classes(&tclasses);
>         if (ret)
> @@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
>         while ((id = queue_remove(id_queue))) {
>                 if (strcmp(id, "self") == 0) {
>                         free(id);
> -                       if (add == 0) {
> -                               yyerror("-self is not supported");
> +                       if (add == 0 && which != AVRULE_NEVERALLOW) {
> +                               yyerror("-self is only supported in neverallow rules");

"-self is only supported in neverallow and neverallowxperm rules"

Thanks,
Jim


> +                               ret = -1;
> +                               goto out;
> +                       }
> +                       avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
> +                       if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
> +                               yyerror("self and -self is not supported");
>                                 ret = -1;
>                                 goto out;
>                         }
> -                       avrule->flags |= RULE_SELF;
>                         continue;
>                 }
>                 if (set_types
> @@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
>                 }
>         }
>
> +       if ((avrule->ttypes.flags & TYPE_COMP)) {
> +               if (avrule->flags & RULE_NOTSELF) {
> +                       yyerror("-self is not supported in complements");
> +                       ret = -1;
> +                       goto out;
> +               }
> +               if (avrule->flags & RULE_SELF) {
> +                       avrule->flags &= ~RULE_SELF;
> +                       avrule->flags |= RULE_NOTSELF;
> +               }
> +       }
> +
>         ebitmap_init(&tclasses);
>         ret = read_classes(&tclasses);
>         if (ret)
> diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
> index ec2a3e9a..a2d74d42 100644
> --- a/checkpolicy/test/dismod.c
> +++ b/checkpolicy/test/dismod.c
> @@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
>         }
>
>         num_types = 0;
> -       if (flags & RULE_SELF) {
> +       if (flags & (RULE_SELF | RULE_NOTSELF)) {
>                 num_types++;
>         }
>
> @@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
>                 fprintf(fp, " self");
>         }
>
> +       if (flags & RULE_NOTSELF) {
> +               fprintf(fp, " -self");
> +       }
> +
>         if (num_types > 1)
>                 fprintf(fp, " }");
>
> --
> 2.34.0
>

Re: [RFC PATCH v2 2/4] libsepol: add not-self neverallow support

[James Carter]

On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add support for not-self neverallow rules. These do not trigger on allow
> rules where the source type is exactly equal to the target type.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
>   - do not change the value of RULE_SELF
> ---
>  libsepol/include/sepol/policydb/policydb.h |  3 +-
>  libsepol/src/assertion.c                   | 36 ++++++++++++++++++++--
>  2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> index 4bf9f05d..11637fe8 100644
> --- a/libsepol/include/sepol/policydb/policydb.h
> +++ b/libsepol/include/sepol/policydb/policydb.h
> @@ -285,7 +285,8 @@ typedef struct avrule {
>  #define AVRULE_XPERMS  (AVRULE_XPERMS_ALLOWED | AVRULE_XPERMS_AUDITALLOW | \
>                                 AVRULE_XPERMS_DONTAUDIT | AVRULE_XPERMS_NEVERALLOW)
>         uint32_t specified;
> -#define RULE_SELF 1
> +#define RULE_SELF       (1U << 0)
> +#define RULE_NOTSELF    (1U << 1)
>         uint32_t flags;
>         type_set_t stypes;
>         type_set_t ttypes;
> diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
> index dd2749a0..efa136c8 100644
> --- a/libsepol/src/assertion.c
> +++ b/libsepol/src/assertion.c
> @@ -241,7 +241,7 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
>         if (rc)
>                 goto oom;
>
> -       if (avrule->flags == RULE_SELF) {
> +       if (avrule->flags & RULE_SELF) {
>                 rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
>                 if (rc)
>                         goto oom;
> @@ -268,6 +268,8 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
>
>                 ebitmap_for_each_positive_bit(&src_matches, snode, i) {
>                         ebitmap_for_each_positive_bit(&tgt_matches, tnode, j) {
> +                               if ((avrule->flags & RULE_NOTSELF) && i == j)
> +                                       continue;
>                                 if (avrule->specified == AVRULE_XPERMS_NEVERALLOW) {
>                                         a->errors += report_assertion_extended_permissions(handle,p, avrule,
>                                                                                         i, j, cp, perms, k, avtab);
> @@ -402,7 +404,7 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
>         if (rc)
>                 goto oom;
>
> -       if (avrule->flags == RULE_SELF) {
> +       if (avrule->flags & RULE_SELF) {
>                 rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1],
>                                 &p->attr_type_map[k->target_type - 1]);
>                 if (rc)
> @@ -418,6 +420,18 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
>                 }
>         }
>
> +       if (avrule->flags & RULE_NOTSELF) {
> +               rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
> +               if (rc)
> +                       goto oom;
> +               rc = ebitmap_and(&self_matches, &avrule->ttypes.types, &matches);
> +               if (rc)
> +                       goto oom;
> +               rc = ebitmap_subtract(&tgt_matches, &self_matches);
> +               if (rc)
> +                       goto oom;
> +       }
> +
>         if (ebitmap_is_empty(&tgt_matches))
>                 goto exit;
>

Something is not right with how it is working with extended permissions.

I am using these types rules with the following examples.
type TYPE1;
type TYPE2;
type TYPE3;
typeattribute TYPE1 TATTR1, TATTR2;
typeattribute TYPE2 TATTR1, TATTR2;
typeattribute TYPE3 TATTR1;


For normal extended permissions.

These rules give an assertion failure as expected.
allow TATTR1 TATTR1 : CLASS4 ioctl;
neverallowxperm TYPE1 self : CLASS4 ioctl 0x9411;

These rules do not, again, as expected.
allow TATTR1 TATTR1 : CLASS4 ioctl;
allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9401;
neverallowxperm TYPE1 self : CLASS4 ioctl 0x9411;

For the not-self extended permissions.

These rules give an assertion failure, but they shouldn't.
allow TATTR1 TATTR1 : CLASS5 ioctl;
allowxperm TATTR1 TATTR1 : CLASS5 ioctl 0x9501;
neverallowxperm TYPE1 ~self : CLASS5 ioctl 0x9511;

libsepol.report_assertion_extended_permissions: neverallowxperm on
line 153 of policy.conf (or line 153 of policy.conf) violated by
allow TYPE1 TYPE2:CLASS5 { ioctl };
libsepol.report_assertion_extended_permissions: neverallowxperm on
line 153 of policy.conf (or line 153 of policy.conf) violated by
allow TYPE1 TYPE3:CLASS5 { ioctl };


Thanks,
Jim


> @@ -463,7 +477,7 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
>         if (rc == 0)
>                 goto exit;
>
> -       if (avrule->flags == RULE_SELF) {
> +       if (avrule->flags & RULE_SELF) {
>                 /* If the neverallow uses SELF, then it is not enough that the
>                  * neverallow's source matches the src and tgt of the rule being checked.
>                  * It must match the same thing in the src and tgt, so AND the source
> @@ -479,6 +493,22 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
>                 ebitmap_destroy(&match);
>         }
>
> +       if (avrule->flags & RULE_NOTSELF) {
> +               ebitmap_t match;
> +               rc = ebitmap_cpy(&match, &p->attr_type_map[k->source_type - 1]);
> +               if (rc) {
> +                       ebitmap_destroy(&match);
> +                       goto oom;
> +               }
> +               rc = ebitmap_subtract(&match, &p->attr_type_map[k->target_type - 1]);
> +               if (rc) {
> +                       ebitmap_destroy(&match);
> +                       goto oom;
> +               }
> +               rc2 = ebitmap_match_any(&avrule->ttypes.types, &match);
> +               ebitmap_destroy(&match);
> +       }
> +
>         /* neverallow may have tgts even if it uses SELF */
>         rc = ebitmap_match_any(&avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
>         if (rc == 0 && rc2 == 0)
> --
> 2.34.0
>

[RFC PATCH v3 1/5] libsepol: introduce ebitmap_relative_complement()

[Christian Göttsche]

Add a method for ebitmaps that computes the relative complement.
All bits set in the second ebitmap are set to zero in the first one.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v3:
   - rename from ebitmap_subtract() to ebitmap_relative_complement()
v2:
   - add shortcut for empty ebitmaps
---
 libsepol/include/sepol/policydb/ebitmap.h |  1 +
 libsepol/src/ebitmap.c                    | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
index 81d0c7a6..076b9341 100644
--- a/libsepol/include/sepol/policydb/ebitmap.h
+++ b/libsepol/include/sepol/policydb/ebitmap.h
@@ -83,6 +83,7 @@ static inline int ebitmap_node_get_bit(const ebitmap_node_t * n, unsigned int bi
 extern int ebitmap_cmp(const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_or(ebitmap_t * dst, const ebitmap_t * e1, const ebitmap_t * e2);
 extern int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1);
+extern int ebitmap_relative_complement(ebitmap_t *dst, const ebitmap_t *e1);
 extern int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_xor(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_not(ebitmap_t *dst, const ebitmap_t *e1, unsigned int maxbit);
diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
index 1de3816a..5f166e8c 100644
--- a/libsepol/src/ebitmap.c
+++ b/libsepol/src/ebitmap.c
@@ -72,6 +72,24 @@ int ebitmap_union(ebitmap_t * dst, const ebitmap_t * e1)
 	return 0;
 }
 
+int ebitmap_relative_complement(ebitmap_t *dst, const ebitmap_t *e1)
+{
+	unsigned int i, length;
+
+	if (ebitmap_is_empty(dst) || ebitmap_is_empty(e1))
+		return 0;
+	
+	length = min(ebitmap_length(dst), ebitmap_length(e1));
+	for (i=0; i < length; i++) {
+		if (ebitmap_get_bit(e1, i)) {
+			int rc = ebitmap_set_bit(dst, i, 0);
+			if (rc < 0)
+				return rc;
+		}
+	}
+	return 0;
+}
+
 int ebitmap_and(ebitmap_t *dst, const ebitmap_t *e1, const ebitmap_t *e2)
 {
 	unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2));
-- 
2.34.1

[RFC PATCH v3 2/5] libsepol: add not-self neverallow support

[Christian Göttsche]

Add support for not-self neverallow rules. These do not trigger on allow
rules where the source type is exactly equal to the target type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v3:
  - use renamed ebitmap_relative_complement(), see previous commit
  - cache not-self status of avrules and add loop shortcut on target and
    source type match
v2:
  - do not change the value of RULE_SELF
---
 libsepol/include/sepol/policydb/policydb.h |  3 +-
 libsepol/src/assertion.c                   | 41 ++++++++++++++++++++--
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 4bf9f05d..11637fe8 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -285,7 +285,8 @@ typedef struct avrule {
 #define AVRULE_XPERMS	(AVRULE_XPERMS_ALLOWED | AVRULE_XPERMS_AUDITALLOW | \
 				AVRULE_XPERMS_DONTAUDIT | AVRULE_XPERMS_NEVERALLOW)
 	uint32_t specified;
-#define RULE_SELF 1
+#define RULE_SELF       (1U << 0)
+#define RULE_NOTSELF    (1U << 1)
 	uint32_t flags;
 	type_set_t stypes;
 	type_set_t ttypes;
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index dd2749a0..fe6b88ae 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -216,6 +216,7 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 	uint32_t perms;
 	ebitmap_t src_matches, tgt_matches, self_matches, matches;
 	ebitmap_node_t *snode, *tnode;
+	const int is_avrule_notself = (avrule->flags & RULE_NOTSELF) != 0;
 	unsigned int i, j;
 
 	if ((k->specified & AVTAB_ALLOWED) == 0)
@@ -241,7 +242,7 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
 		if (rc)
 			goto oom;
@@ -268,6 +269,8 @@ static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void
 
 		ebitmap_for_each_positive_bit(&src_matches, snode, i) {
 			ebitmap_for_each_positive_bit(&tgt_matches, tnode, j) {
+				if (is_avrule_notself && i == j)
+					continue;
 				if (avrule->specified == AVRULE_XPERMS_NEVERALLOW) {
 					a->errors += report_assertion_extended_permissions(handle,p, avrule,
 											i, j, cp, perms, k, avtab);
@@ -381,6 +384,7 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 	unsigned int i, j;
 	ebitmap_node_t *snode, *tnode;
 	class_perm_node_t *cp;
+	const int is_avrule_notself = (avrule->flags & RULE_NOTSELF) != 0;
 	int rc;
 	int ret = 1;
 
@@ -402,7 +406,7 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 	if (rc)
 		goto oom;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1],
 				&p->attr_type_map[k->target_type - 1]);
 		if (rc)
@@ -418,6 +422,18 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 		}
 	}
 
+	if (is_avrule_notself) {
+		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
+		if (rc)
+			goto oom;
+		rc = ebitmap_and(&self_matches, &avrule->ttypes.types, &matches);
+		if (rc)
+			goto oom;
+		rc = ebitmap_relative_complement(&tgt_matches, &self_matches);
+		if (rc)
+			goto oom;
+	}
+
 	if (ebitmap_is_empty(&tgt_matches))
 		goto exit;
 
@@ -426,6 +442,9 @@ static int check_assertion_extended_permissions(avrule_t *avrule, avtab_t *avtab
 			continue;
 		ebitmap_for_each_positive_bit(&src_matches, snode, i) {
 			ebitmap_for_each_positive_bit(&tgt_matches, tnode, j) {
+				if (is_avrule_notself && i == j)
+					continue;
+
 				ret = check_assertion_extended_permissions_avtab(
 						avrule, avtab, i, j, k, p);
 				if (ret)
@@ -463,7 +482,7 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 	if (rc == 0)
 		goto exit;
 
-	if (avrule->flags == RULE_SELF) {
+	if (avrule->flags & RULE_SELF) {
 		/* If the neverallow uses SELF, then it is not enough that the
 		 * neverallow's source matches the src and tgt of the rule being checked.
 		 * It must match the same thing in the src and tgt, so AND the source
@@ -479,6 +498,22 @@ static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *a
 		ebitmap_destroy(&match);
 	}
 
+	if (avrule->flags & RULE_NOTSELF) {
+		ebitmap_t match;
+		rc = ebitmap_cpy(&match, &p->attr_type_map[k->source_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc = ebitmap_relative_complement(&match, &p->attr_type_map[k->target_type - 1]);
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc2 = ebitmap_match_any(&avrule->ttypes.types, &match);
+		ebitmap_destroy(&match);
+	}
+
 	/* neverallow may have tgts even if it uses SELF */
 	rc = ebitmap_match_any(&avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
 	if (rc == 0 && rc2 == 0)
-- 
2.34.1

[RFC PATCH v3 3/5] checkpolicy: add not-self neverallow support

[Christian Göttsche]

Add support for using negated or complemented self in the target type of
neverallow rules.

Some refpolicy examples:

    neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *;
    # no violations

    neverallow domain domain:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

    neverallow domain { domain -self }:file ~{ append read_file_perms write };

    libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
    libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };

Using negated self in a complement `~{ domain -self }` is not supported.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v3:
   - mention both neverallow rule types when using -self within an
     unsupported rule type
v2:
   - fix neverallowxperm usage

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++-----
 checkpolicy/test/dismod.c   |  6 ++++-
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index d3eb6111..e74b1a9a 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) {
+				yyerror("-self is only supported in neverallow and neverallowxperm rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
@@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			free(id);
-			if (add == 0) {
-				yyerror("-self is not supported");
+			if (add == 0 && which != AVRULE_NEVERALLOW) {
+				yyerror("-self is only supported in neverallow and neverallowxperm rules");
+				ret = -1;
+				goto out;
+			}
+			avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
+			if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
+				yyerror("self and -self is not supported");
 				ret = -1;
 				goto out;
 			}
-			avrule->flags |= RULE_SELF;
 			continue;
 		}
 		if (set_types
@@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 		}
 	}
 
+	if ((avrule->ttypes.flags & TYPE_COMP)) {
+		if (avrule->flags & RULE_NOTSELF) {
+			yyerror("-self is not supported in complements");
+			ret = -1;
+			goto out;
+		}
+		if (avrule->flags & RULE_SELF) {
+			avrule->flags &= ~RULE_SELF;
+			avrule->flags |= RULE_NOTSELF;
+		}
+	}
+
 	ebitmap_init(&tclasses);
 	ret = read_classes(&tclasses);
 	if (ret)
diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index ec2a3e9a..a2d74d42 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 	}
 
 	num_types = 0;
-	if (flags & RULE_SELF) {
+	if (flags & (RULE_SELF | RULE_NOTSELF)) {
 		num_types++;
 	}
 
@@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
 		fprintf(fp, " self");
 	}
 
+	if (flags & RULE_NOTSELF) {
+		fprintf(fp, " -self");
+	}
+
 	if (num_types > 1)
 		fprintf(fp, " }");
 
-- 
2.34.1

[RFC PATCH v3 4/5] libsepol: free ebitmap on end of function

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/assertion.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index fe6b88ae..4600be41 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -460,6 +460,7 @@ oom:
 exit:
 	ebitmap_destroy(&src_matches);
 	ebitmap_destroy(&tgt_matches);
+	ebitmap_destroy(&self_matches);
 	ebitmap_destroy(&matches);
 	return ret;
 }
-- 
2.34.1

[RFC PATCH v3 5/5] libsepol: pass avtab to report function

[Christian Göttsche]

Populate the avtab member before passing as argument to the report
function. Without the avtab avtab_search_node() is unable to find
allowxperm rules and this results in false-positive reports, e.g. on:

    allow TATTR1 TATTR1 : CLASS1 ioctl;
    allowxperm TATTR1 TATTR1 : CLASS1 ioctl 0x9501;
    neverallowxperm TYPE1 ~self : CLASS1 0x9501;

Reported-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/assertion.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index 4600be41..a0eebb93 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -304,10 +304,12 @@ static int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avru
 	args.avrule = avrule;
 	args.errors = 0;
 
+	args.avtab = &p->te_avtab;
 	rc = avtab_map(&p->te_avtab, report_assertion_avtab_matches, &args);
 	if (rc)
 		goto oom;
 
+	args.avtab = &p->te_cond_avtab;
 	rc = avtab_map(&p->te_cond_avtab, report_assertion_avtab_matches, &args);
 	if (rc)
 		goto oom;
-- 
2.34.1

Re: [RFC PATCH v2 3/4] checkpolicy: add not-self neverallow support

[Christian Göttsche]

On Fri, 3 Dec 2021 at 22:56, James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Add support for using negated or complemented self in the target type of
> > neverallow rules.
> >
> > Some refpolicy examples:
> >
> >     neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *;
> >     # no violations
> >
> >     neverallow domain domain:file ~{ append read_file_perms write };
> >
> >     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
> >     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create };
> >     libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };
> >
> >     neverallow domain { domain -self }:file ~{ append read_file_perms write };
> >
> >     libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename };
> >     libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create };
> >
> > Using negated self in a complement `~{ domain -self }` is not supported.
> >
>
> I am thinking about what to do with this patch set. If this is
> valuable in checkpolicy, then I would like it in CIL as well. But CIL
> obviously cannot support the above syntax.
>
> What would be lost if we used "notself"?
>
> We could define the behavior of "neverallow SRC notself . . ." so that
> if SRC was a type, the rule would be expanded so that the target types
> would be all types except SRC, and if SRC was an attribute the rules
> would be expanded into multiple rules with all combinations of the
> types in SRC used for the source and target except for the cases where
> the source and target type would be the same.
> That would make your examples work.
> "neverallow * notself . . ." would expand like "neverallow * ~self . . ."
> "neverallow domain notself . . ." would expand like "neverallow domain
> { domain -self } . . ."
> "neverallow domain notself . . ." and "neverallow domain ~domain . .
> ." together would expand like "neverallow domain ~self . . ." (I
> think)
>
> What would be missing would be the ability to express "neverallow
> domain { subdomain -self } . . ."
>

I would suggest to introduce `all` as a new keyword for neverallow
rules. Yes, this would break backwards compatibility, but in the long
run it makes a saner language and with appropriate communication and
good CIL error messages the transition should be manageable. Then one
could write something like:

neverallow * ~self:process fork;
(neverallow all (not self) (process (fork)))

neverallow domain { domain -self -foo_t }:dir create;
(neverallow domain (and domain (not (and self foo_t))) (dir (create)))


> A few minor comments below.
>
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> > v2:
> >    - fix neverallowxperm usage
> > ---
> >  checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++-----
> >  checkpolicy/test/dismod.c   |  6 ++++-
> >  2 files changed, 45 insertions(+), 7 deletions(-)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index d3eb6111..f27a6f33 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
> >         while ((id = queue_remove(id_queue))) {
> >                 if (strcmp(id, "self") == 0) {
> >                         free(id);
> > -                       if (add == 0) {
> > -                               yyerror("-self is not supported");
> > +                       if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) {
> > +                               yyerror("-self is only supported in neverallowxperm rules");
>
> "-self is only supported in neverallow and neverallowxperm rules"
>
> > +                               ret = -1;
> > +                               goto out;
> > +                       }
> > +                       avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
> > +                       if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
> > +                               yyerror("self and -self is not supported");
> >                                 ret = -1;
> >                                 goto out;
> >                         }
> > -                       avrule->flags |= RULE_SELF;
> >                         continue;
> >                 }
> >                 if (set_types
> > @@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule)
> >                 }
> >         }
> >
> > +       if ((avrule->ttypes.flags & TYPE_COMP)) {
> > +               if (avrule->flags & RULE_NOTSELF) {
> > +                       yyerror("-self is not supported in complements");
> > +                       ret = -1;
> > +                       goto out;
> > +               }
> > +               if (avrule->flags & RULE_SELF) {
> > +                       avrule->flags &= ~RULE_SELF;
> > +                       avrule->flags |= RULE_NOTSELF;
> > +               }
> > +       }
> > +
> >         ebitmap_init(&tclasses);
> >         ret = read_classes(&tclasses);
> >         if (ret)
> > @@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
> >         while ((id = queue_remove(id_queue))) {
> >                 if (strcmp(id, "self") == 0) {
> >                         free(id);
> > -                       if (add == 0) {
> > -                               yyerror("-self is not supported");
> > +                       if (add == 0 && which != AVRULE_NEVERALLOW) {
> > +                               yyerror("-self is only supported in neverallow rules");
>
> "-self is only supported in neverallow and neverallowxperm rules"
>
> Thanks,
> Jim
>
>
> > +                               ret = -1;
> > +                               goto out;
> > +                       }
> > +                       avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF);
> > +                       if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) {
> > +                               yyerror("self and -self is not supported");
> >                                 ret = -1;
> >                                 goto out;
> >                         }
> > -                       avrule->flags |= RULE_SELF;
> >                         continue;
> >                 }
> >                 if (set_types
> > @@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
> >                 }
> >         }
> >
> > +       if ((avrule->ttypes.flags & TYPE_COMP)) {
> > +               if (avrule->flags & RULE_NOTSELF) {
> > +                       yyerror("-self is not supported in complements");
> > +                       ret = -1;
> > +                       goto out;
> > +               }
> > +               if (avrule->flags & RULE_SELF) {
> > +                       avrule->flags &= ~RULE_SELF;
> > +                       avrule->flags |= RULE_NOTSELF;
> > +               }
> > +       }
> > +
> >         ebitmap_init(&tclasses);
> >         ret = read_classes(&tclasses);
> >         if (ret)
> > diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
> > index ec2a3e9a..a2d74d42 100644
> > --- a/checkpolicy/test/dismod.c
> > +++ b/checkpolicy/test/dismod.c
> > @@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
> >         }
> >
> >         num_types = 0;
> > -       if (flags & RULE_SELF) {
> > +       if (flags & (RULE_SELF | RULE_NOTSELF)) {
> >                 num_types++;
> >         }
> >
> > @@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic
> >                 fprintf(fp, " self");
> >         }
> >
> > +       if (flags & RULE_NOTSELF) {
> > +               fprintf(fp, " -self");
> > +       }
> > +
> >         if (num_types > 1)
> >                 fprintf(fp, " }");
> >
> > --
> > 2.34.0
> >

Re: [RFC PATCH v3 5/5] libsepol: pass avtab to report function

[James Carter]

[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]

On Sat, Dec 4, 2021 at 5:35 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Populate the avtab member before passing as argument to the report
> function. Without the avtab avtab_search_node() is unable to find
> allowxperm rules and this results in false-positive reports, e.g. on:
>
>     allow TATTR1 TATTR1 : CLASS1 ioctl;
>     allowxperm TATTR1 TATTR1 : CLASS1 ioctl 0x9501;
>     neverallowxperm TYPE1 ~self : CLASS1 0x9501;
>
> Reported-by: James Carter <jwcart2@gmail.com>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

No longer getting the false positives, but now I am seeing false negatives.

allow TATTR1 TATTR1 : CLASS4 ioctl;
allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9401;
neverallowxperm TATTR1 self : CLASS4 ioctl 0x9421;

These rules are being caught as they should:
allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9421;
allowxperm TATTR1 TATTR2 : CLASS4 ioctl 0x9421;

These rules are not being caught.
allowxperm TYPE1    self : CLASS4 ioctl 0x9421;
allowxperm TYPE1   TYPE1 : CLASS4 ioctl 0x9421;
allowxperm TYPE1  TATTR1 : CLASS4 ioctl 0x9421;
allowxperm TATTR1   self : CLASS4 ioctl 0x9421;

I've attached the policy.conf that I am testing with.

Thanks,
Jim


> ---
>  libsepol/src/assertion.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
> index 4600be41..a0eebb93 100644
> --- a/libsepol/src/assertion.c
> +++ b/libsepol/src/assertion.c
> @@ -304,10 +304,12 @@ static int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avru
>         args.avrule = avrule;
>         args.errors = 0;
>
> +       args.avtab = &p->te_avtab;
>         rc = avtab_map(&p->te_avtab, report_assertion_avtab_matches, &args);
>         if (rc)
>                 goto oom;
>
> +       args.avtab = &p->te_cond_avtab;
>         rc = avtab_map(&p->te_cond_avtab, report_assertion_avtab_matches, &args);
>         if (rc)
>                 goto oom;
> --
> 2.34.1
>

[-- Attachment #2: policy.conf --]
[-- Type: text/plain, Size: 9397 bytes --]

class CLASS1
class CLASS2
class CLASS3
class CLASS4
class CLASS5
class CLASS6
sid kernel
class CLASS1 { PERM1A PERM1B PERM1C PERM1D }
class CLASS2 { PERM2A PERM2B PERM2C PERM2D }
class CLASS3 { PERM3A PERM3B PERM3C PERM3D }
class CLASS4 { ioctl }
class CLASS5 { ioctl }
class CLASS6 { ioctl }
sensitivity SENS1;
dominance { SENS1 }
category CAT1;
level SENS1:CAT1;
mlsconstrain CLASS1 { PERM1A } (h1 dom h2 and l1 domby h1);
mlsvalidatetrans CLASS1 (l1 == l2 or l1 incomp l2);
attribute TATTR1;
attribute TATTR2;
type TYPE1;
type TYPE2;
type TYPE3;
typeattribute TYPE1 TATTR1, TATTR2;
typeattribute TYPE2 TATTR1, TATTR2;
typeattribute TYPE3 TATTR1;


# Test self neverallow

#allow TYPE1    self : CLASS1 PERM1A; # neverallow violation
#allow TYPE1   TYPE1 : CLASS1 PERM1A; # neverallow violation
#allow TYPE1  TATTR1 : CLASS1 PERM1A; # neverallow violation
#allow TATTR1 TATTR1 : CLASS1 PERM1A; # neverallow violation
#allow TATTR1 TATTR2 : CLASS1 PERM1A; # neverallow violation
neverallow TYPE1 self : CLASS1 PERM1A;

#allow TYPE1    self : CLASS1 PERM1B; # neverallow violation
#allow TYPE1   TYPE1 : CLASS1 PERM1B; # neverallow violation
#allow TYPE1  TATTR1 : CLASS1 PERM1B; # neverallow violation
#allow TATTR1 TATTR1 : CLASS1 PERM1B; # neverallow violation
#allow TATTR1 TATTR2 : CLASS1 PERM1B; # neverallow violation
allow TYPE1 TYPE2 : CLASS1 PERM1B; # NOT a neverallow violation
neverallow TATTR1 self : CLASS1 PERM1B;

# Test allow rule in module, neverallow in base
#allow TYPE1 self : CLASS1 PERM1C; # neverallow violation
neverallow TYPE1 self : CLASS1 PERM1C;

# Test neverallow in module, allow rule in base
#allow TYPE1 self : CLASS1 PERM1D; # neverallow violation
neverallow TYPE1 self : CLASS1 PERM1D;


# Test ~self neverallow

allow TYPE1  self : CLASS2 PERM2A; # Not neverallow violation
allow TYPE1 TYPE1 : CLASS2 PERM2A; # Not neverallow violation
#allow TYPE1   TYPE2 : CLASS2 PERM2A; # neverallow violation
#allow TYPE1  TATTR1 : CLASS2 PERM2A; # neverallow violation
#allow TATTR1 TATTR1 : CLASS2 PERM2A; # neverallow violation
#allow TATTR1 TATTR2 : CLASS2 PERM2A; # neverallow violation
neverallow TYPE1 ~self : CLASS2 PERM2A;

allow TYPE1  self : CLASS2 PERM2B; # Not neverallow violation
allow TYPE2 TYPE2 : CLASS2 PERM2B; # Not neverallow violation
#allow TYPE1   TYPE2  : CLASS2 PERM2B; # neverallow violation
#allow TYPE1  TATTR1  : CLASS2 PERM2B; # neverallow violation
#allow TATTR1 TATTR1  : CLASS2 PERM2B; # neverallow violation
#allow TATTR1 TATTR2  : CLASS2 PERM2B; # neverallow violation
neverallow TATTR1 ~self : CLASS2 PERM2B;

# Test allow rules in module, neverallow in base
allow TYPE1 self : CLASS2 PERM2C;   # Not neverallow violation
#allow TYPE1 TYPE2 : CLASS2 PERM2C; # neverallow violation
neverallow TYPE1 ~self : CLASS2 PERM2C;

# Test neverallow in module, allow rule in base
allow TYPE1 self : CLASS2 PERM2D;   # Not neverallow violation
#allow TYPE1 TYPE2 : CLASS2 PERM2D; # neverallow violation
neverallow TYPE1 ~self : CLASS2 PERM2D;


# Test -self neverallow

allow TYPE1  self : CLASS3 PERM3A; # Not neverallow violation
allow TYPE2 TYPE2 : CLASS3 PERM3A; # Not neverallow violation
#allow TYPE1   TYPE2 : CLASS3 PERM3A; # neverallow violation
#allow TYPE1  TATTR1 : CLASS3 PERM3A; # neverallow violation
#allow TATTR1 TATTR1 : CLASS3 PERM3A; # neverallow violation
#allow TATTR1 TATTR2 : CLASS3 PERM3A; # neverallow violation
neverallow TATTR1 { TATTR1 -self } : CLASS3 PERM3A;

allow TYPE1  self : CLASS3 PERM3B; # Not neverallow violation
allow TYPE2 TYPE2 : CLASS3 PERM3B; # Not neverallow violation
allow TYPE1 TYPE3 : CLASS3 PERM3B; # Not neverallow violation
#allow TYPE1   TYPE2 : CLASS3 PERM3B; # neverallow violation
#allow TYPE1  TATTR1 : CLASS3 PERM3B; # neverallow violation
#allow TATTR1 TATTR1 : CLASS3 PERM3B; # neverallow violation
#allow TATTR1 TATTR2 : CLASS3 PERM3B; # neverallow violation
neverallow TATTR1 { TATTR2 -self } : CLASS3 PERM3B;

# Test allow rules in module, neverallow in base
allow TYPE1 self : CLASS3 PERM3C; # Not neverallow violation
neverallow TATTR1 { TATTR1 -self } : CLASS3 PERM3C;

# Test neverallow in module, allow rule in base
allow TYPE1 self : CLASS3 PERM3D; # Not neverallow violation
#allow TYPE1 TYPE2 : CLASS3 PERM3D; # neverallow violation
neverallow TATTR1 { TATTR1 -self } : CLASS3 PERM3D;


# Test self neverallowxperm

allow TATTR1 TATTR1 : CLASS4 ioctl;
allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9401;

#allowxperm TYPE1    self : CLASS4 ioctl 0x9411; # neverallowxperm violation
#allowxperm TYPE1   TYPE1 : CLASS4 ioctl 0x9411; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS4 ioctl 0x9411; # neverallowxperm violation
#allowxperm TATTR1   self : CLASS4 ioctl 0x9411; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9411; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS4 ioctl 0x9411; # neverallowxperm violation
neverallowxperm TYPE1 self : CLASS4 ioctl 0x9411;

#allowxperm TYPE1    self : CLASS4 ioctl 0x9421; # neverallowxperm violation
#allowxperm TYPE1   TYPE1 : CLASS4 ioctl 0x9421; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS4 ioctl 0x9421; # neverallowxperm violation
#allowxperm TATTR1   self : CLASS4 ioctl 0x9421; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS4 ioctl 0x9421; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS4 ioctl 0x9421; # neverallowxperm violation
allowxperm TYPE1 TYPE2 : CLASS4 ioctl 0x9421; # NOT neverallowxperm violation
neverallowxperm TATTR1 self : CLASS4 ioctl 0x9421;

# Test allow rules in module, neverallowxperm in base
#allowxperm TYPE1  self : CLASS4 ioctl 0x9431; # neverallowxperm violation
neverallowxperm TYPE1 self : CLASS4 ioctl 0x9431;

# Test neverallow in module, allow rule in base
#allowxperm TYPE1 self : CLASS4 ioctl 0x9441; # neverallow violation
neverallowxperm TYPE1 self : CLASS4 ioctl 0x9441;

# Test ~self neverallowxperm

allow TATTR1 TATTR1 : CLASS5 ioctl;
allowxperm TATTR1 TATTR1 : CLASS5 ioctl 0x9501;

allowxperm TYPE1  self : CLASS5 ioctl 0x9511; # Not neverallowxperm violation
allowxperm TYPE1 TYPE1 : CLASS5 ioctl 0x9511; # Not neverallowxperm violation
#allowxperm TYPE1   TYPE2 : CLASS5 ioctl 0x9511; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS5 ioctl 0x9511; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS5 ioctl 0x9511; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS5 ioctl 0x9511; # neverallowxperm violation
neverallowxperm TYPE1 ~self : CLASS5 ioctl 0x9511;

allowxperm TYPE1  self : CLASS5 ioctl 0x9521; # Not neverallowxperm violation
allowxperm TYPE2 TYPE2 : CLASS5 ioctl 0x9521; # Not neverallowxperm violation
#allowxperm TYPE1   TYPE2 : CLASS5 ioctl 0x9521; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS5 ioctl 0x9521; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS5 ioctl 0x9521; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS5 ioctl 0x9521; # neverallowxperm violation
neverallowxperm TATTR1 ~self : CLASS5 ioctl 0x9521;

# Test allow rules in module, neverallowxperm in base
allowxperm TYPE1  self : CLASS5 ioctl 0x9531; # Not neverallow violation
#allowxperm TYPE1 TYPE2 : CLASS5 ioctl 0x9531; # neverallow violation
neverallowxperm TYPE1 ~self : CLASS5 ioctl 0x9531;

# Test neverallow in module, allow rule in base
allowxperm TYPE1 self : CLASS5 ioctl 0x9541; # Not neverallow violation
#allowxperm TYPE1 TYPE2 : CLASS5 ioctl 0x9541; # neverallow violation
neverallowxperm TYPE1 ~self : CLASS5 ioctl 0x9541;


# Test -self neverallowxperm

allow TATTR1 TATTR1 : CLASS6 ioctl;
allowxperm TATTR1 TATTR1 : CLASS6 ioctl 0x9601;

allowxperm TYPE1  self : CLASS6 ioctl 0x9611; # Not neverallowxperm violation
allowxperm TYPE2 TYPE2 : CLASS6 ioctl 0x9611; # Not neverallowxperm violation
#allowxperm TYPE1   TYPE2 : CLASS6 ioctl 0x9611; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS6 ioctl 0x9611; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS6 ioctl 0x9611; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS6 ioctl 0x9611; # neverallowxperm violation
neverallowxperm TATTR1 { TATTR1 -self } : CLASS6 ioctl 0x9611;

allowxperm TYPE1  self : CLASS6 ioctl 0x9621; # Not neverallowxperm violation
allowxperm TYPE2 TYPE2 : CLASS6 ioctl 0x9621; # Not neverallowxperm violation
allowxperm TYPE1 TYPE3 : CLASS6 ioctl 0x9621; # Not neverallowxperm violation
#allowxperm TYPE1   TYPE2 : CLASS6 ioctl 0x9621; # neverallowxperm violation
#allowxperm TYPE1  TATTR1 : CLASS6 ioctl 0x9621; # neverallowxperm violation
#allowxperm TATTR1 TATTR1 : CLASS6 ioctl 0x9621; # neverallowxperm violation
#allowxperm TATTR1 TATTR2 : CLASS6 ioctl 0x9621; # neverallowxperm violation
neverallowxperm TATTR1 { TATTR2 -self } : CLASS6 ioctl 0x9621;

# Test allow rules in module, neverallowxperm in base
allowxperm TYPE1  self : CLASS6 ioctl 0x9631; # Not neverallow violation
#allowxperm TYPE1 TYPE2 : CLASS6 ioctl 0x9631; # neverallow violation
neverallowxperm TYPE1 ~self : CLASS6 ioctl 0x9631;

# Test neverallow in module, allow rule in base
allowxperm TYPE1 self : CLASS6 ioctl 0x9641; # Not neverallow violation
#allowxperm TYPE1 TYPE2 : CLASS6 ioctl 0x9641; # neverallow violation
neverallowxperm TYPE1 ~self : CLASS6 ioctl 0x9641;


role ROLE1;
role ROLE1 types TYPE1;
user USER1 roles ROLE1 level SENS1 range SENS1 - SENS1:CAT1;
sid kernel USER1:ROLE1:TYPE1:SENS1 - SENS1