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

[Paul Moore]

On Wed, Aug 26, 2020 at 11:06 AM peter enderborg
<peter.enderborg@xxxxxxxx> wrote:
> On 8/26/20 4:45 PM, Paul Moore wrote:
> > On Wed, Aug 26, 2020 at 10:34 AM peter enderborg
> > <peter.enderborg@xxxxxxxx> wrote:
> >> On 8/26/20 3:42 PM, Paul Moore wrote:
> >>> On Mon, Aug 24, 2020 at 9:23 AM Peter Enderborg
> >>> <peter.enderborg@xxxxxxxx> 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@xxxxxxxx>
> >>>> ---
> >>>>  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://lkml.org/lkml/2020/8/18/842
>
> https://lkml.org/lkml/2020/8/21/526
>
> and the "single list" version
>
> https://lkml.org/lkml/2020/8/17/1346
>
> 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 :)

-- 
paul moore
www.paul-moore.com