RE: [RFC PATCH] selinux: Add denied trace with permssion filter

[Robert Judy <rjudy@sfasu.edu>]

I would like to unsubscribe from this group. I have sent "unsubscribe" requests to selinux-owner@vger.kernel.org as subject and in the body of the e-mail but that has not worked. Please advise me how to unsubscribe.

Thank you and respectfully,

rmj

-----Original Message-----
From: selinux-owner@vger.kernel.org <selinux-owner@vger.kernel.org> On Behalf Of Paul Moore
Sent: Monday, August 31, 2020 9:16 AM
To: peter enderborg <peter.enderborg@sony.com>
Cc: linux-kernel@vger.kernel.org; SElinux list <selinux@vger.kernel.org>; Steven Rostedt <rostedt@goodmis.org>; Stephen Smalley <stephen.smalley.work@gmail.com>
Subject: Re: [RFC PATCH] selinux: Add denied trace with permssion filter

On Thu, Aug 27, 2020 at 10:04 AM peter enderborg <peter.enderborg@sony.com> wrote:
>
> On 8/27/20 3:30 PM, Paul Moore wrote:
> > On Wed, Aug 26, 2020 at 11:06 AM peter enderborg 
> > <peter.enderborg@sony.com> wrote:
> >> On 8/26/20 4:45 PM, Paul Moore wrote:
> >>> On Wed, Aug 26, 2020 at 10:34 AM peter enderborg 
> >>> <peter.enderborg@sony.com> wrote:
> >>>> On 8/26/20 3:42 PM, Paul Moore wrote:
> >>>>> On Mon, Aug 24, 2020 at 9:23 AM Peter Enderborg 
> >>>>> <peter.enderborg@sony.com> wrote:
> >>>>>> This adds tracing of all denies. They are grouped with 
> >>>>>> trace_seq for each audit.
> >>>>>>
> >>>>>> A filter can be inserted with a write to it's filter section.
> >>>>>>
> >>>>>> echo "permission==\"entrypoint\"" > 
> >>>>>> events/avc/selinux_denied/filter
> >>>>>>
> >>>>>> A output will be like:
> >>>>>>           runcon-1046  [002] .N..   156.351738: selinux_denied:
> >>>>>>           trace_seq=2 result=-13
> >>>>>>           scontext=system_u:system_r:cupsd_t:s0-s0:c0.
> >>>>>>           c1023 tcontext=system_u:object_r:bin_t:s0
> >>>>>>           tclass=file permission=entrypoint
> >>>>>>
> >>>>>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> >>>>>> ---
> >>>>>>  include/trace/events/avc.h | 37 +++++++++++++++++++++++++++++++++++++
> >>>>>>  security/selinux/avc.c     | 27 +++++++++++++++++++++++++--
> >>>>>>  2 files changed, 62 insertions(+), 2 deletions(-)
> >>>>> My most significant comment is that I don't think we want, or 
> >>>>> need, two trace points in the avc_audit_post_callback() 
> >>>>> function.  Yes, I understand they are triggered slightly 
> >>>>> differently, but from my perspective there isn't enough 
> >>>>> difference between the two tracepoints to warrant including 
> >>>>> both.  However, while the tracepoints may be
> >>>> We tried that but that was problematic too.
> >>> My apologies if I was on that thread, but can you remind me why it 
> >>> was a problem?  Why can't we use a single tracepoint to capture 
> >>> the AVC information?
> >> The problem is parsing the event.
> >>
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_
> >> 2020_8_18_842&d=DwIBaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4c
> >> c&r=oO5HuGEGxznA2F3djiiYxmxxWQonw0h6Sks-BEoB4ys&m=qmi2ROWsLC_0mLLhH
> >> kpb71j1YoicydLh-7l4cOsLYcY&s=iS3eZr3TFrN5I7BbnvPFYOKd6DfW1FHTFcwI7j
> >> oS_fk&e=
> >>
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_
> >> 2020_8_21_526&d=DwIBaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4c
> >> c&r=oO5HuGEGxznA2F3djiiYxmxxWQonw0h6Sks-BEoB4ys&m=qmi2ROWsLC_0mLLhH
> >> kpb71j1YoicydLh-7l4cOsLYcY&s=9OsLN0Y5mUWxEAAqUE6K4PS57Pn1XyZz7GXak6
> >> uc_Ls&e=
> >>
> >> and the "single list" version
> >>
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_
> >> 2020_8_17_1346&d=DwIBaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4
> >> cc&r=oO5HuGEGxznA2F3djiiYxmxxWQonw0h6Sks-BEoB4ys&m=qmi2ROWsLC_0mLLh
> >> Hkpb71j1YoicydLh-7l4cOsLYcY&s=tWSY2ry2IT6RcT5BIUwMuqBL_yPObDE1VljbL
> >> qI1zrA&e=
> >>
> >> With this patch we follow standard message format so no plugin should be needed.
> > I'm evidently missing something very fundamental (likely), and/or 
> > I'm just not communicating very clearly (also likely), because the 
> > above links don't appear to make any sense with respect to my question.
> >
> > Let me try a reset ... Why can't we basically take the 
> > "selinux_denied" TRACE_EVENT implementation in your patch and use it 
> > to replace the "selinux_audited" TRACE_EVENT in the selinux/next 
> > tree (of course with the necessary changes to the AVC callback code)?
> >
> > If the "selinux_denied" implementation is valid from a tracing point 
> > of view, why can we not do this?  Of course if the "selinux_denied"
> > implementation is not a valid TRACE_EVENT then I'm not sure why this 
> > was suggested for SELinux :)
>
> Im happly fine with replacing the selinux_audited with selinux_denied.  However it is the case where there are more than one denial at the same time. Im not sure how and when it might happen.

One thing I wondered about was why not build up a single string with all of the permissions instead of generating multiple trace events?
In the previous discussion it was implied that this was due to limitations in the tracing subsystem's filtering, and based on the discussion thus far I'm guessing there is little desire for this information if it can't be filtered on?

If that's the case then I think we are stuck with the tracing code that currently lives in selinux/next, as I currently have little desire to add more than one tracepoint in the SELinux permission checking codepath.

> When that happen we got more than one event. I have no problems with that, but im not sure if the debug tools and perf can make sense of that.
>
> A other feature with the selinux_audited event it might be inserted on other places in the code too.  A denial is sort of final.

--
paul moore
www.paul-moore.com