Messages in this thread | | | From | Paul Moore <> | Date | Thu, 23 Jan 2020 09:32:16 -0500 | Subject | Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events |
| |
On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2020-01-22 17:40, Paul Moore wrote: > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
...
> > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index 17b0d523afb3..478259f3fa53 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff *skb) > > > audit_ctl_unlock(); > > > } > > > > > > +/* Log information about who is connecting to the audit multicast socket */ > > > +static void audit_log_multicast_bind(int group, const char *op, int err) > > > +{ > > > + const struct cred *cred; > > > + struct tty_struct *tty; > > > + char comm[sizeof(current->comm)]; > > > + struct audit_buffer *ab; > > > + > > > + if (!audit_enabled) > > > + return; > > > + > > > + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER); > > > + if (!ab) > > > + return; > > > + > > > + cred = current_cred(); > > > + tty = audit_get_tty(); > > > + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", > > > + task_pid_nr(current), > > > + from_kuid(&init_user_ns, cred->uid), > > > + from_kuid(&init_user_ns, audit_get_loginuid(current)), > > > + tty ? tty_name(tty) : "(none)", > > > + audit_get_sessionid(current)); > > > > Don't we already get all of that information as part of the syscall record? > > Yes. However, the syscall record isn't always present. One example is > systemd, shown above.
Assuming that the system supports syscall auditing, the absence of a syscall record is a configuration choice made by the admin. If the system doesn't support syscall auditing the obvious "fix" is to do the work to enable syscall auditing on that platform ... but now we're starting to get off topic.
> The other is the disconnect record, shown above, > which may be asynchronous, or an unmonitored syscall (It could only be > setsockopt, close, shutdown.).
An unmonitored syscall still falls under the category of a configuration choice so I'm not too concerned about that, but the async disconnect record is legitimate. Can you provide more information about when this occurs? I'm guessing this is pretty much just an abrupt/abnormal program exit?
> > I'm pretty sure these are the same arguments I made when Steve posted > > a prior version of this patch. > > You did. I would really like to have dropped them, but they aren't > reliably available.
Personally I'm not too worried if we have duplicate information spread across records in a single event, as long as they are consistent. However, I remember Steve complaining rather loudly about duplicated fields across records in a single event some time back; perhaps that is not a concern of his anymore (perhaps it was a narrow case at the time), I don't know.
Here is the deal, either duplicated information is something we are okay with, or it is something to avoid; we need to pick one. As mentioned above, I don't really care that much either way (I have a slight preference, but I don't feel strongly enough to fight for it), so let's hear the arguments both for and against and decide - whatever we pick I'll enforce so long as we are stuck with this string format.
-- paul moore www.paul-moore.com
| |