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.
Having partly overlapping traces is not unheard off. Check
compaction.c where we have a trace_mm_compaction_begin
and a more detailed trace_mm_compaction_migratepages.
(And a trace_mm_compaction_end)
> redundant in my mind, this new event does do the permission lookup in
> the kernel so that the contexts/class/permissions are all available as
> a string which is a good thing.
>
> Without going into the details, would the tracing folks be okay with
> doing something similar with the existing selinux_audited tracepoint?
> It's extra work in the kernel, but since it would only be triggered
> when the tracepoint was active it seems bearable to me.
I think the method for expanding lists is what we tried first on
suggestion from Steven Rostedt. Maybe we can do a trace_event
from a TP_prink but that would be recursive.
>> diff --git a/include/trace/events/avc.h b/include/trace/events/avc.h
>> index 94bca8bef8d2..9a28559956de 100644
>> --- a/include/trace/events/avc.h
>> +++ b/include/trace/events/avc.h
>> @@ -54,6 +54,43 @@ TRACE_EVENT(selinux_audited,
>> )
>> );
>>
>> +TRACE_EVENT(selinux_denied,
>> +
>> + TP_PROTO(struct selinux_audit_data *sad,
>> + char *scontext,
>> + char *tcontext,
>> + const char *tclass,
>> + const char *permission,
>> + int64_t seq
>> + ),
>> +
>> + TP_ARGS(sad, scontext, tcontext, tclass, permission, seq),
>> +
>> + TP_STRUCT__entry(
>> + __field(int, result)
>> + __string(scontext, scontext)
>> + __string(tcontext, tcontext)
>> + __string(permission, permission)
>> + __string(tclass, tclass)
>> + __field(u64, seq)
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->result = sad->result;
>> + __entry->seq = seq;
>> + __assign_str(tcontext, tcontext);
>> + __assign_str(scontext, scontext);
>> + __assign_str(permission, permission);
>> + __assign_str(tclass, tclass);
>> + ),
>> +
>> + TP_printk("trace_seq=%lld result=%d scontext=%s tcontext=%s tclass=%s permission=%s",
>> + __entry->seq, __entry->result, __get_str(scontext), __get_str(tcontext),
>> + __get_str(tclass), __get_str(permission)
>> +
>> + )
>> +);
>> +
>> #endif
>>
>> /* This part must be outside protection */
>> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
>> index 1debddfb5af9..ca53baca15e1 100644
>> --- a/security/selinux/avc.c
>> +++ b/security/selinux/avc.c
>> @@ -92,6 +92,7 @@ struct selinux_avc {
>> };
>>
>> static struct selinux_avc selinux_avc;
>> +static atomic64_t trace_seqno;
>>
>> void selinux_avc_init(struct selinux_avc **avc)
>> {
>> @@ -731,9 +732,31 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
>> tclass = secclass_map[sad->tclass-1].name;
>> audit_log_format(ab, " tclass=%s", tclass);
>>
>> - if (sad->denied)
>> + if (sad->denied) {
>> audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
>> -
>> + if (trace_selinux_denied_enabled()) {
>> + int i, perm;
>> + const char **perms;
>> + u32 denied = sad->denied;
>> + int64_t seq;
>> +
>> + seq = atomic_long_inc_return(&trace_seqno);
>> + perms = secclass_map[sad->tclass-1].perms;
>> + i = 0;
>> + perm = 1;
>> + while (i < (sizeof(denied) * 8)) {
>> + if ((perm & denied & sad->requested) && perms[i]) {
>> + trace_selinux_denied(sad, scontext, tcontext,
>> + tclass, perms[i], seq);
>> + denied &= ~perm;
>> + if (!denied)
>> + break;
>> + }
>> + i++;
>> + perm <<= 1;
>> + }
>> + }
>> + }
>> trace_selinux_audited(sad, scontext, tcontext, tclass);
>> kfree(tcontext);
>> kfree(scontext);
>> --
>> 2.17.1
