On 2020-04-17 17:53, Paul Moore wrote: > On Wed, Mar 18, 2020 at 5:33 PM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote: > > On 2020-03-18 17:22, Paul Moore wrote: > > > On Wed, Mar 18, 2020 at 9:12 AM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote: > > > > On 2020-03-17 17:30, Richard Guy Briggs wrote: > > > > > Some table unregister actions seem to be initiated by the kernel to > > > > > garbage collect unused tables that are not initiated by any userspace > > > > > actions. It was found to be necessary to add the subject credentials to > > > > > cover this case to reveal the source of these actions. A sample record: > > > > > > > > > > type=NETFILTER_CFG msg=audit(2020-03-11 21:25:21.491:269) : table=nat family=bridge entries=0 op=unregister pid=153 uid=root auid=unset tty=(none) ses=unset subj=system_u:system_r:kernel_t:s0 comm=kworker/u4:2 exe=(null) > > > > > > > > Given the precedent set by bpf unload, I'd really rather drop this patch > > > > that adds subject credentials. > > > > > > > > Similarly with ghak25's subject credentials, but they were already > > > > present and that would change an existing record format, so it isn't > > > > quite as justifiable in that case. > > > > > > Your comments have me confused - do you want this patch (v3 3/3) > > > considered for merging or no? > > > > I would like it considered for merging if you think it will be required > > to provide enough information about the event that happenned. In the > > bpf unload case, there is a program number to provide a link to a > > previous load action. In this case, we won't know for sure what caused > > the table to be unloaded if the number of entries was empty. I'm still > > trying to decide if it matters. For the sake of caution I think it > > should be included. I don't like it, but I think it needs to be > > included. > > I'm in the middle of building patches 1/3 and 2/3, assuming all goes > well I'll merge them into audit/next (expect mail soon), however I'm > going back and forth on this patch. Like you I kinda don't like it, > and with both of us not in love with this patch I have to ask if there > is certification requirement for this? I know about the generic > subj/obj requirements, but in the case where there is no associated > task/syscall/etc. information it isn't like the extra fields supplied > in this patch are going to have much information in that regard; it's > really the *absence* of that information which is telling. Which > brings me to wonder if simply the lack of any associated records in > this event is enough? Before when we weren't associating records into > a single event it would have been a problem, but the way things > currently are, if there are no other records (and you have configured > that) then I think you have everything you need to know. > > Thoughts? I'm good dropping that third patch, but Steve's perspective is more authoritative here. I'll respin patch 1/3 and 2/3 to fix the checkpatch.pl errors if you prefer (was erroneously mentioned in ghak28 feedback). > paul moore - RGB -- Richard Guy Briggs <rgb@xxxxxxxxxx> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635