Re: [RFC][PATCH] selinux: Report result in avc messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Attached patch switches to reporting permissive=0|1 and only does it
for avc: denied messages.

On Wed, Apr 30, 2014 at 8:18 AM, Stephen Smalley
<stephen.smalley@xxxxxxxxx> wrote:
> I could make it permissive=0 or permissive=1 if that is less
> confusing.  It doesn't necessarily correspond to the result of the
> system call, just the avc_has_perm call, as e.g. the kernel checks
> CAP_DAC_OVERRIDE and falls back to CAP_DAC_READ_SEARCH if only
> read/search access was requested, and there are other cases where a
> permission denial has a side effect rather than preventing the system
> call (e.g. CAP_FSETID).
>
> On Wed, Apr 30, 2014 at 6:34 AM, Daniel J Walsh <dwalsh@xxxxxxxxxx> wrote:
>>
>> On 04/30/2014 09:29 AM, Steve Grubb wrote:
>>> On Wednesday, April 30, 2014 08:59:50 AM Daniel J Walsh wrote:
>>>> How about permitted rather then allowed.
>>> I think permitted is already in an AVC.
>> Not sure where.
>>>
>>>> On 04/29/2014 10:59 PM, Eric Paris wrote:
>>>>> On Tue, 2014-04-29 at 16:54 -0700, Stephen Smalley wrote:
>>>>>> Requested for Android in order to distinguish denials that are not in
>>>>>> fact breaking anything yet due to permissive domains versus denials
>>>>>> that are being enforced, but seems generally useful.  result field was
>>>>>> already in the selinux audit data structure and was being passed to
>>>>>> avc_audit() but wasn't being used.  Seems to cause no harm to ausearch
>>>>>> or audit2allow to add it as a field.  Comments?
>>>>> I think it's a great idea, but I'm worried that Steve is going to get
>>>>> grumpy because an AVC record is going to have a result= field which is
>>>>> similar, but not necessarily related to the res= field of a SYSCALL
>>>>> record.
>>> I think that I'll have to parse this field no matter what. Its probably that
>>> important. In the syscall, we use success= to be the final determination.
>>>
>>>
>>>>> Seems easily confused (although probably 9999 times out of
>>>>> 10000 they will be the same)
>>> Why would this ever not be correct? Are there times when we get an AVC with a
>>> denial _and_ the syscall completes successfully?
>>>
>>> I'd suggest using res= since its in the audit dictionary and means exactly
>>> what you are wanting to use it for. In it, 1 is success, 0 is failure.
>>>
>> I have seen AVC's where the success=yes in enforcing mode.  Basically
>> the kernel takes a different code path and the syscall succeeds.  Most
>> of these end up as dontaudits.
>>>>> So while I wholeheartedly think we should take the idea, I wonder if
>>>>> someone can dream up a name that isn't confusingly similar...
>>>>>
>>>>> I can't think of anything...
>>> There is thesaurus.com. :-)
>>>
>>> consequence, outcome, effect, reaction,  conclusion, verdict, decision,
>>> judgement, finding, ruling, answer, solution, recommendation, order,  ...
>>>
>>> -Steve
>>
From 26e103995ce90378ca9a80a86f11bf3a25c96c37 Mon Sep 17 00:00:00 2001
From: Stephen Smalley <sds@xxxxxxxxxxxxx>
Date: Tue, 29 Apr 2014 11:29:04 -0700
Subject: [PATCH] selinux:  Report permissive mode in avc: denied messages.

We cannot presently tell from an avc: denied message whether access was in
fact denied or was allowed due to global or per-domain permissive mode.
Add a permissive= field to the avc message to reflect this information.

Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
---
 security/selinux/avc.c         | 7 ++++++-
 security/selinux/hooks.c       | 5 +++--
 security/selinux/include/avc.h | 4 ++--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index fc3e662..a18f1fa 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -444,11 +444,15 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
 	avc_dump_query(ab, ad->selinux_audit_data->ssid,
 			   ad->selinux_audit_data->tsid,
 			   ad->selinux_audit_data->tclass);
+	if (ad->selinux_audit_data->denied) {
+		audit_log_format(ab, " permissive=%u",
+				 ad->selinux_audit_data->result ? 0 : 1);
+	}
 }
 
 /* This is the slow part of avc audit with big stack footprint */
 noinline int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass,
-		u32 requested, u32 audited, u32 denied,
+		u32 requested, u32 audited, u32 denied, int result,
 		struct common_audit_data *a,
 		unsigned flags)
 {
@@ -477,6 +481,7 @@ noinline int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass,
 	sad.tsid = tsid;
 	sad.audited = audited;
 	sad.denied = denied;
+	sad.result = result;
 
 	a->selinux_audit_data = &sad;
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b4beb77..e156b5f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2770,6 +2770,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na
 
 static noinline int audit_inode_permission(struct inode *inode,
 					   u32 perms, u32 audited, u32 denied,
+					   int result,
 					   unsigned flags)
 {
 	struct common_audit_data ad;
@@ -2780,7 +2781,7 @@ static noinline int audit_inode_permission(struct inode *inode,
 	ad.u.inode = inode;
 
 	rc = slow_avc_audit(current_sid(), isec->sid, isec->sclass, perms,
-			    audited, denied, &ad, flags);
+			    audited, denied, result, &ad, flags);
 	if (rc)
 		return rc;
 	return 0;
@@ -2822,7 +2823,7 @@ static int selinux_inode_permission(struct inode *inode, int mask)
 	if (likely(!audited))
 		return rc;
 
-	rc2 = audit_inode_permission(inode, perms, audited, denied, flags);
+	rc2 = audit_inode_permission(inode, perms, audited, denied, rc, flags);
 	if (rc2)
 		return rc2;
 	return rc;
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index f53ee3c..ddf8eec 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -102,7 +102,7 @@ static inline u32 avc_audit_required(u32 requested,
 }
 
 int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass,
-		   u32 requested, u32 audited, u32 denied,
+		   u32 requested, u32 audited, u32 denied, int result,
 		   struct common_audit_data *a,
 		   unsigned flags);
 
@@ -137,7 +137,7 @@ static inline int avc_audit(u32 ssid, u32 tsid,
 	if (likely(!audited))
 		return 0;
 	return slow_avc_audit(ssid, tsid, tclass,
-			      requested, audited, denied,
+			      requested, audited, denied, result,
 			      a, 0);
 }
 
-- 
1.8.3.1

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux