diff mbox series

[16/16,v2] libsepol: Fix two problems with neverallowxperm reporting

Message ID 20220111215446.595516-17-jwcart2@gmail.com (mailing list archive)
State Accepted
Commit 71291385cf14
Headers show
Series Refactor and fix assertion checking | expand

Commit Message

James Carter Jan. 11, 2022, 9:54 p.m. UTC
Not all violations of neverallowxperm rules were being reported.
In check_assertion_extended_permissions_avtab(), a break was
performed after finding a match rather than just returning right
away. This means that if other src and tgt pairs were checked
afterward that did not match, then no match would be reported.

 allow attr attr:CLASS ioctl;
 allowxperm attr attr:CLASS ioctl 0x9401;
 allowxperm t1 self:CLASS ioctl 0x9421;
 neverallowxperm attr self:CLASS ioctl 0x9421;
Would result in no assertion violations being found.

Another problem was that the reporting function did not properly
recognize when there was a valid allowxperm rule and falsely
reported additional violations that did not exist. (There had
to be at least one legitimate violation.)

Using the same example as above (and assuming t1 and t2 both have
attribute attr), the following would be reported as:
  neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
  violated by
  allowxperm t1 t1:CLASS ioctl { 0x9421 };

  neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
  violated by
  allow t2 t2:CLASS4 { ioctl };

There is no violation for t2 because there is a valid allowxperm
rule for it.

With this patch, only the first error message (which is the correct
one) is printed.

Signed-off-by: James Carter <jwcart2@gmail.com>
 libsepol/src/assertion.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff mbox series


diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index b21c83ba..44c20362 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -149,6 +149,7 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 	ebitmap_node_t *snode, *tnode;
 	unsigned int i, j;
 	int rc;
+	int found_xperm = 0;
 	int errors = 0;
 	memcpy(&tmp_key, k, sizeof(avtab_key_t));
@@ -165,7 +166,7 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 				if ((xperms->specified != AVTAB_XPERMS_IOCTLFUNCTION)
 						&& (xperms->specified != AVTAB_XPERMS_IOCTLDRIVER))
+				found_xperm = 1;
 				rc = check_extended_permissions(avrule->xperms, xperms);
 				/* failure on the extended permission check_extended_permissions */
 				if (rc) {
@@ -185,7 +186,7 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 	/* failure on the regular permissions */
-	if (!errors) {
+	if (!found_xperm) {
 		ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
 				"allow %s %s:%s {%s };",
 				avrule->source_line, avrule->source_filename, avrule->line,
@@ -343,7 +344,7 @@  static int check_assertion_extended_permissions_avtab(avrule_t *avrule, avtab_t
 				rc = check_extended_permissions(neverallow_xperms, xperms);
 				if (rc)
-					break;
+					return rc;