Re: [PATCH 2/2] selinux: add attributes to avc tracepoint

[peter enderborg]

On 8/6/20 3:49 PM, Stephen Smalley wrote:
> On Thu, Aug 6, 2020 at 9:45 AM Stephen Smalley
> <stephen.smalley.work@xxxxxxxxx> wrote:
>> On 8/6/20 8:32 AM, Stephen Smalley wrote:
>>
>>> On 8/6/20 8:24 AM, peter enderborg wrote:
>>>
>>>> On 8/6/20 2:11 PM, Stephen Smalley wrote:
>>>>> On 8/6/20 4:03 AM, Thiébaud Weksteen wrote:
>>>>>
>>>>>> From: Peter Enderborg <peter.enderborg@xxxxxxxx>
>>>>>>
>>>>>> Add further attributes to filter the trace events from AVC.
>>>>> Please include sample usage and output in the description.
>>>>>
>>>>>
>>>> Im not sure where you want it to be.
>>>>
>>>> In the commit message or in a Documentation/trace/events-avc.rst ?
>>> I was just asking for it in the commit message / patch description.  I
>>> don't know what is typical for Documentation/trace.
>> For example, I just took the patches for a spin, running the
>> selinux-testsuite under perf like so:
>>
>> sudo perf record -e avc:selinux_audited -g make test
>>
>> and then ran:
>>
>> sudo perf report -g
>>
>> and a snippet of sample output included:
>>
>>       6.40%     6.40%  requested=0x800000 denied=0x800000
>> audited=0x800000 result=-13 ssid=922 tsid=922
>> scontext=unconfined_u:unconfined_r:test_binder_mgr_t:s0-s0:c0.c1023
>> tcontext=unconfined_u:unconfined_r:test_binder_mgr_t:s0-s0:c0.c1023
>> tclass=capability
> So then the question becomes how do you use the above information,
> e.g. is that sufficient to correlate it to an actual avc: denied
> message, how do you decode the requested/denied/audited fields (or
> should the code do that for you and just report the string name(s) of
> the permission(s), do you need all three of those fields separately,
> is it useful to log the ssid/tsid at all given that you have the
> contexts and sids are dynamically assigned, etc.
>
>>              |
>>              ---0x495641000028933d
>>                 __libc_start_main
>>                 |
>>                 |--4.60%--__GI___ioctl
>>                 |          entry_SYSCALL_64
>>                 |          do_syscall_64
>>                 |          __x64_sys_ioctl
>>                 |          ksys_ioctl
>>                 |          binder_ioctl
>>                 |          binder_set_nice
>>                 |          can_nice
>>                 |          capable
>>                 |          security_capable
>>                 |          cred_has_capability.isra.0
>>                 |          slow_avc_audit
>>                 |          common_lsm_audit
>>                 |          avc_audit_post_callback
>>                 |          avc_audit_post_callback

The real cool thing happen when you enable "user-stack-trace" too.

           <...>-4820  [007] .... 85878.897553: selinux_audited: requested=0x4000000 denied=0x4000000 audited=0x4000000 result=-13 ssid=341 tsid=61 scontext=system_u:system_r:ntpd_t:s0 tcontext=system_u:object_r:bin_t:s0 tclass=file
           <...>-4820  [007] .... 85878.897572: <user stack trace>
 =>  <00007f07d99bb45b>
 =>  <0000555ecd89ca57>

The fields are useful for filter what you what to see and what you can ignore.  Having the ssid and text was from the part where it is called.
The numeric can be used for two things. When you dont have any context. Same same reason as in post_callback. We need to be static
in format so it need  be there if it ever can happen. And it is also useful for faster filtering.

You can do "ssid!=42 && ssid!=43 && tsid==666".  From my view it would be good to have all fields there. But they need to right typed to be able
to use the filter mechanism. There must me some trade-off too where the argument filtering get bigger than the processing, but I think we can
add a lot more before we reach that threshold.