[systemd-devel] [PATCH] audit: Fix journal failing on unsupported audit in containers [was: journal: don't complain about audit socket errors in a container.]

[Martin Pitt]

Lennart Poettering [2015-05-20 14:57 +0200]:
> On Wed, 20.05.15 13:53, Martin Pitt (martin.pitt at ubuntu.com) wrote:
> 
> > -                        cached_use = true;
> > +                        /* bind() fails in namespaces (containers), so check that too */
> > +                        static const union sockaddr_union sa = {
> > +                                .nl.nl_family = AF_NETLINK,
> > +                                .nl.nl_pid    = 0,
> > +                                .nl.nl_groups = AUDIT_NLGRP_READLOG,
> > +                        };
> > +                        cached_use = (bind(fd, &sa.sa, sizeof(sa.nl)) >= 0);
> >                          safe_close(fd);
> 
> This check is simply not right. With that you check whether the
> multicast audit API is available. But given that it has been added
> only one or two kernel releases ago, and is protected by its own
> capabality the check is definitely too broad.

OK. I thought the intention of ConditionSecurity=audit was exactly
that, as this condition was added together with adding it to
systemd-journald-audit.socket.

> The fact is simply that the kernel audit subsystem is borked in the
> kernel when it comes to namespacing, and there's no nice way to detect
> whether it is borked I am aware of. 

Right, hence my thinking was that we check for the things we actually
want to do with it.

> We could of course add a detect_container() check now to journald. But
> I think that would be a big mistake, since there was work on fixing
> audit in the kernel for containers (by adding audit namespacing or
> so).

Right, fully agreed. That's why I wrote that I didn't like Frank's original
patch.

> Anyway, I think people are mostly concerned about bind() failing here,
> hence I have now added some code to handle that gracefully.
> 
> Right now it will still log a message with LOG_WARNING. I'd be willing
> to downgrade this to LOG_DEBUG for select error codes, if you tell me
> the ones you run into. EINVAL?

bind(7, {sa_family=AF_NETLINK, pid=0, groups=00000001}, 12) = -1 EPERM (Operation not permitted)

With commit 417a7fdc journald now starts working, but
systemd-journald-audit.socket still fails:

   Active: failed (Result: resources)
   systemd[1]: systemd-journald-audit.socket: Socket service systemd-journald.service already active, refusing.
   systemd[1]: Failed to listen on Journal Audit Socket.

That's why I thought tightening the ConditionSecurity=audit check
would make more sense, as systemd-journald-audit.socket is the only
unit which actually uses it. We could add
ConditionVirtualization=!container to it as a distro-level workaround,
but I don't like that for the reasons above. I don't just want to
leave it like that as it makes the system stay in "degraded" mode.

> Also, please convince your distro to enable seccomp support!

Fair enough, but that hack doesn't work on all platforms we support
(i386, powerpc, ppc64el, etc.), and quite frankly that's an even worse
hack: You'd need to disable that filter once the kernel gets fixed,
and/or conditionalize it based on the running kernel version. I'd like
the same code to work everywhere :-)

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20150520/7512476b/attachment.sig>