All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Carter <jwcart2@gmail.com>
To: selinux@vger.kernel.org
Cc: cgzones@googlemail.com, James Carter <jwcart2@gmail.com>
Subject: [PATCH 16/16 v2] libsepol: Fix two problems with neverallowxperm reporting
Date: Tue, 11 Jan 2022 16:54:46 -0500	[thread overview]
Message-ID: <20220111215446.595516-17-jwcart2@gmail.com> (raw)
In-Reply-To: <20220111215446.595516-1-jwcart2@gmail.com>

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.

Example:
 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 --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))
 					continue;
-
+				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
 					continue;
 				rc = check_extended_permissions(neverallow_xperms, xperms);
 				if (rc)
-					break;
+					return rc;
 			}
 		}
 	}
-- 
2.31.1


  parent reply	other threads:[~2022-01-11 21:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-11 21:54 [PATCH 00/16 v2] Refactor and fix assertion checking James Carter
2022-01-11 21:54 ` [PATCH 01/16 v2] libsepol: Return an error if check_assertion() returns an error James Carter
2022-01-11 21:54 ` [PATCH 02/16 v2] libsepol: Change label in check_assertion_avtab_match() James Carter
2022-01-11 21:54 ` [PATCH 03/16 v2] libsepol: Remove uneeded error messages in assertion checking James Carter
2022-01-11 21:54 ` [PATCH 04/16 v2] libsepol: Check for error from check_assertion_extended_permissions() James Carter
2022-01-11 21:54 ` [PATCH 05/16 v2] libsepol: Use consistent return checking style James Carter
2022-01-11 21:54 ` [PATCH 06/16 v2] libsepol: Move check of target types to before check for self James Carter
2022-01-11 21:54 ` [PATCH 07/16 v2] libsepol: Create function check_assertion_self_match() and use it James Carter
2022-01-11 21:54 ` [PATCH 08/16 v2] libsepol: Use (rc < 0) instead of (rc) when calling ebitmap functions James Carter
2022-01-11 21:54 ` [PATCH 09/16 v2] libsepol: Remove unnessesary check for matching class James Carter
2022-01-11 21:54 ` [PATCH 10/16 v2] libsepol: Move assigning outer loop index out of inner loop James Carter
2022-01-11 21:54 ` [PATCH 11/16 v2] libsepol: Make use of previously created ebitmap when checking self James Carter
2022-01-11 21:54 ` [PATCH 12/16 v2] libsepol: Refactor match_any_class_permissions() to be clearer James Carter
2022-01-11 21:54 ` [PATCH 13/16 v2] libsepol: Make return value clearer when reporting neverallowx errors James Carter
2022-01-11 21:54 ` [PATCH 14/16 v2] libsepol: The src and tgt must be the same if neverallow uses self James Carter
2022-01-11 21:54 ` [PATCH 15/16 v2] libsepol: Set args avtab pointer when reporting assertion violations James Carter
2022-01-11 21:54 ` James Carter [this message]
2022-02-18 21:16 ` [PATCH 00/16 v2] Refactor and fix assertion checking James Carter
2022-02-24 21:07   ` James Carter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220111215446.595516-17-jwcart2@gmail.com \
    --to=jwcart2@gmail.com \
    --cc=cgzones@googlemail.com \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.