On 8/6/20 5:03 PM, Stephen Smalley wrote: > On Thu, Aug 6, 2020 at 10:51 AM peter enderborg > <peter.enderborg@xxxxxxxx> wrote: >> 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. > I don't think the SIDs are useful because they are dynamically > assigned (aside from the initial SIDs for bootstrapping before policy > load) and are not exported to userspace (userspace has no way to look > them up). It is probably a mistake that we even fall back to logging > them in the existing code and may just be a legacy of when SIDs were > exported to userspace (ancient history, before mainline merge of > SELinux). > > In any event, if you were to include the sample usage and output I > provided as part of the commit message / patch description (or replace > with your own example), that would be helpful I think. Even better > would be to also provide some hint as to how people are expected to > decode the requested/denied/audited fields (I know how to do that but > not everyone will, and ideally one would have a script or something > for doing so). But they are always the same until load off a new policy? And they can be mapped while debugging. You see one event and then you "know" it. This is for developers so it is a different mindset than for the netlink type that is for administrators. Features for speeding things up are useful. Im doing a update on commit message, will send it to Weksteen for a review round first.
