linux-kernel - Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Jan 2020 09:32:16 -0500
From:   Paul Moore <paul@...l-moore.com>
To:     Richard Guy Briggs <rgb@...hat.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>, sgrubb@...hat.com,
        omosnace@...hat.com, nhorman@...hat.com,
        Eric Paris <eparis@...isplace.org>
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@...hat.com> wrote:
> On 2020-01-22 17:40, Paul Moore wrote:
> > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@...hat.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