libselinux: grant permissions on deny in permissive mode

This patch duplicates the kernel behavior of granting permissions after a denial to avoid repeated AVC's.

It required moving the avc_update_node static inline function further up in the file and changing its prototype.

---
avc.c | 65 ++++++++++++++++++++++++++++++++++-------------------------------

1 file changed, 34 insertions(+), 31 deletions(-)

Index: libselinux/src/avc.c
===================================================================
@@ -203,6 +203,32 @@

	    & (AVC_CACHE_SLOTS - 1);
}

+static inline void avc_update_node(uint32_t event, struct avc_entry *ae,
+ access_vector_t perms)
+{
+ switch (event) {
+ case AVC_CALLBACK_GRANT:
+ ae->avd.allowed |= perms;
+ break;
+ case AVC_CALLBACK_TRY_REVOKE:
+ case AVC_CALLBACK_REVOKE:
+ ae->avd.allowed &= ~perms;
+ break;
+ case AVC_CALLBACK_AUDITALLOW_ENABLE:
+ ae->avd.auditallow |= perms;
+ break;
+ case AVC_CALLBACK_AUDITALLOW_DISABLE:
+ ae->avd.auditallow &= ~perms;
+ break;
+ case AVC_CALLBACK_AUDITDENY_ENABLE:
+ ae->avd.auditdeny |= perms;
+ break;
+ case AVC_CALLBACK_AUDITDENY_DISABLE:
+ ae->avd.auditdeny &= ~perms;
+ break;
+ }
+}
+
int avc_context_to_sid_raw(security_context_t ctx, security_id_t * sid)

{
	int rc;

@@ -982,9 +1008,12 @@

	denied = requested & ~(ae->avd.allowed);

- if ((!requested || denied) && avc_enforcing) {
- errno = EACCES;
- rc = -1;
+ if (!requested || denied) {
+ if (avc_enforcing) {
+ errno = EACCES;
+ rc = -1;
+ } else

+			avc_update_node(AVC_CALLBACK_GRANT, ae, requested);
	}

      out:
@@ -1066,32 +1095,6 @@
	return (x == y || x == SECSID_WILD || y == SECSID_WILD);
}

-static inline void avc_update_node(uint32_t event, struct avc_node *node,
- access_vector_t perms)
-{
- switch (event) {
- case AVC_CALLBACK_GRANT:
- node->ae.avd.allowed |= perms;
- break;
- case AVC_CALLBACK_TRY_REVOKE:
- case AVC_CALLBACK_REVOKE:
- node->ae.avd.allowed &= ~perms;
- break;
- case AVC_CALLBACK_AUDITALLOW_ENABLE:
- node->ae.avd.auditallow |= perms;
- break;
- case AVC_CALLBACK_AUDITALLOW_DISABLE:
- node->ae.avd.auditallow &= ~perms;
- break;
- case AVC_CALLBACK_AUDITDENY_ENABLE:
- node->ae.avd.auditdeny |= perms;
- break;
- case AVC_CALLBACK_AUDITDENY_DISABLE:
- node->ae.avd.auditdeny &= ~perms;
- break;
- }
-}
-
static int avc_update_cache(uint32_t event, security_id_t ssid,
security_id_t tsid, security_class_t tclass, access_vector_t perms)
@@ -1108,7 +1111,7 @@

				if (avc_sidcmp(ssid, node->ae.ssid) &&
				    avc_sidcmp(tsid, node->ae.tsid) &&
				    tclass == node->ae.tclass) {
-					avc_update_node(event, node, perms);
+					avc_update_node(event, &node->ae, perms);
				}
			}
		}
@@ -1116,7 +1119,7 @@
		/* apply to one node */
		node = avc_search_node(ssid, tsid, tclass, 0);
		if (node) {
-			avc_update_node(event, node, perms);
+			avc_update_node(event, &node->ae, perms);
		}
	}

Eamon Walsh <[email protected]>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

Alternatively, in the userspace AVC case, you could just directly mutate ae->avd.allowed in avc_has_perm_noaudit(), as:

	ae->avd.allowed |= requested;

That is what the kernel AVC used to do before it was converted to RCU (at which point we could no longer mutate entries in-place). Much simpler to do it the above way since you aren't likely to be using RCU in the userspace AVC ;)

You also need to reset the cache upon a setenforce 1 in this case just like the kernel does so that you flush any cached permissions from permissive mode when you enter enforcing mode.

Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

OK, will resubmit using that approach.

Adding James to the cc list:

I'm having trouble with this because avc_ss_reset() takes a "seqno" argument and the selnl_msg_policyload netlink structure provides a seqno field, but the selnl_msg_setenforce does not, it just has the new mode.

Does this field need to be added to the message structure?

Eamon Walsh <[email protected]>
National Security Agency

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

No. Just call avc_ss_reset(0); just like the kernel does. Hacky, but it works ;)

Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

This patch duplicates the kernel behavior of granting permissions after a denial to avoid repeated AVC's, including flushing the cache on a switch to enforcing mode.

---

avc.c | 9 ++++++---
avc_internal.c | 12 ++++++++++++

2 files changed, 18 insertions(+), 3 deletions(-)

Index: src/avc_internal.c
===================================================================
@@ -163,6 +163,12 @@

				    ("%s:  received setenforce notice (enforcing=%d)\n",
				     avc_prefix, msg->val);
				avc_enforcing = msg->val;

+ if (avc_enforcing && (rc = avc_ss_reset(0)) < 0) {
+ avc_log
+ ("%s: cache reset returned %d (errno %d)\n",
+ avc_prefix, rc, errno);
+ goto out;
+ }

				break;
			}

@@ -263,6 +269,12 @@

				    ("%s:  received setenforce notice (enforcing=%d)\n",
				     avc_prefix, msg->val);
				avc_enforcing = msg->val;

+ if (avc_enforcing && (ret = avc_ss_reset(0)) < 0) {
+ avc_log
+ ("%s: cache reset returned %d (errno %d)\n",
+ avc_prefix, ret, errno);
+ goto out;
+ }

				break;
			}

Index: src/avc.c
===================================================================
@@ -982,9 +982,12 @@
 

	denied = requested & ~(ae->avd.allowed);

- if ((!requested || denied) && avc_enforcing) {
- errno = EACCES;
- rc = -1;
+ if (!requested || denied) {
+ if (avc_enforcing) {
+ errno = EACCES;
+ rc = -1;
+ } else

+			ae->avd.allowed |= requested;
	}

      out:

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.