Re: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context

On 2018-07-23 17:00, Paul Moore wrote: > On Mon, Jul 23, 2018 at 12:43 PM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > > On 2018-07-12 17:46, Richard Guy Briggs wrote: > > > On 2018-06-28 18:11, Paul Moore wrote: > > > > On Thu, Jun 14, 2018 at 4:23 PM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > > > > > Since the function audit_log_common_recv_msg() is shared by a number of > > > > > AUDIT_CONFIG_CHANGE and the entire range of AUDIT_USER_* record types, > > > > > and since the AUDIT_CONFIG_CHANGE message type has been converted to a > > > > > syscall accompanied record type, special-case the AUDIT_USER_* range of > > > > > messages so they remain standalone records. > > > > > > > > > > See: https://github.com/linux-audit/audit-kernel/issues/59 > > > > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > > > > > --- > > > > > kernel/audit.c | 12 +++++++++--- > > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > I think this is fine, but see my previous comment about combining 2/6 > > > > and 3/6 as a safety measure. > > > > > > This one I left as a seperate patch for discussion. We'd previously > > > talked about connecting all possible records with syscall records if > > > they exist, but this one I'm unsure about, since we don't really care > > > what userspace process is issuing this message. It is just the message > > > content itself that is important. Or is it? Are we concerned about > > > CAP_AUDIT_WRITE holders/abusers and want as much info about them as we > > > can get in case they go rogue or pear-shaped? > > > > I'm waiting on re-spinning this patchset because of this open question. > > > > Is connecting AUDIT_USER* records desirable or a liability? > > Like I said, I think special casing the AUDIT_USER* records so they > are *not* associated with other records is okay, and perhaps even the > right thing to do. The problem is that we don't have the necessary > context (har har) to match any kernel events (and there is the > possibility that there are none) to the userspace generated > AUDIT_USER* event. Further, I don't think this is something we would > ever be able to solve in a reasonable manner. Ok, having said that, I think I'd still prefer to keep this patch seperate, partly to retain the simplicity of the previous patch and make very clear what each one is doing, and partly if we decide to change our mind in the future that these AUDIT_USER* records should be autonomous.