Hello Frank and systemd devs,

frank.thalberg at tuta.io [2015-04-12 20:51 +0000]:
> This fixes an issue within journald aborting when running inside
> archlinux container via systemd-nspawn on a debian host with audit
> enabled kernel.

We have exactly the same problem with both standard nspawn as well as
user LXC containers. This completely breaks journalling in containers
and also prints a lot of error messages.

Using audit=0 on the host is not satisfying, though:

 - It's hard to discover
 - There is no reason to disable audit support on the host when all
   you need to do is to unbreak containers and disable auditing there.
 - We don't want admins to create static configs which are (1)
   always the same workaround everywhere, and (2) become obsolete once
   the kernel is fixed.

So I'd rather have a dynamic solution. Your patch works, but I don't
like it that much:

> +                        if (errno == EPERM && detect_container(NULL) > 0) {
> +                                log_debug("Audit not supported in 
> containers.");
> +                                return 0;
> +                        }

The detect_container() check is not really related to the question
"does audit work". It's certainly that way today, but future kernels
might change this.

> --- a/src/journal/journald-server.c
> +++ b/src/journal/journald-server.c
> @@ -1585,9 +1585,11 @@ int server_init(Server *s) {
>           if (r < 0)
>                   return r;
> 
> +#ifdef HAVE_AUDIT
>           r = server_open_audit(s);
>           if (r < 0)
>                   return r;
> +#endif

This would require statically enabling/disabling the complete audit
support in the systemd package, while we can do this check at runtime
without much effort.

Also, with your patch you merely unbreak journald itself, but
systemd-journald-audit.socket and other units which have
ConditionSecurity=audit will still fail.

I created a patch which is a more direct approach which makes the
ConditionSecurity=audit check more thorough and actually working in
containers. With that I can leave audit enabled on the host,
containers will boot fine (including journalling) without audit
support, and as soon as the kernel gets fixed it'll automagically start
working in containers as well.

Lennart, WDYT?

Thanks,

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From 4900d31e22719464ed7208c7013730acb551d961 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Wed, 20 May 2015 13:36:45 +0200
Subject: [PATCH] audit: Fix journal failing on unsupported audit in containers

Commit cfb1f5df introduced ConditionSecurity=audit via use_audit(). However,
use_audit() is still true in namespaces like nspawn containers as there
creating an audit socket succeeds. What fails in journald is binding to it
("Permission denied"). So make the check more thorough to only declare that
audit is supported when bind() works as well. This is now exactly the same
check as journald does.

In journald, check use_audit() to see if auditing is supported before we
actually try to bind to the audit socket (as that will fail hard in
namespaces).

This avoids a failing journald and systemd-journald-audit.socket in nspawn and
similar environments. If/once the kernel gets proper auditing support for
namespaces, this will magically start to work without further changes.

Adjust README accordingly.
---
 README                       | 18 ++++--------------
 src/journal/journald-audit.c |  5 +++++
 src/shared/audit.c           |  9 ++++++++-
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/README b/README
index 039110e..da7a86a 100644
--- a/README
+++ b/README
@@ -97,20 +97,6 @@ REQUIREMENTS:
           CONFIG_EFIVAR_FS
           CONFIG_EFI_PARTITION
 
-        Note that kernel auditing is broken when used with systemd's
-        container code. When using systemd in conjunction with
-        containers, please make sure to either turn off auditing at
-        runtime using the kernel command line option "audit=0", or
-        turn it off at kernel compile time using:
-          CONFIG_AUDIT=n
-        If systemd is compiled with libseccomp support on
-        architectures which do not use socketcall() and where seccomp
-        is supported (this effectively means x86-64 and ARM, but
-        excludes 32-bit x86!), then nspawn will now install a
-        work-around seccomp filter that makes containers boot even
-        with audit being enabled. This works correctly only on kernels
-        3.14 and newer though. TL;DR: turn audit off, still.
-
         glibc >= 2.16
         libcap
         libmount >= 2.20 (from util-linux)
@@ -249,6 +235,10 @@ WARNINGS:
         false positives will be triggered by code which violates
         some rules but is actually safe.
 
+        Kernel auditing is broken when used with systemd's container
+        code. journal support for audit messages will not be available
+        in e. g. nspawn.
+
 ENGINEERING AND CONSULTING SERVICES:
         ENDOCODE <https://endocode.com/> offers professional
         engineering and consulting services for systemd. Please
diff --git a/src/journal/journald-audit.c b/src/journal/journald-audit.c
index 64395e1..e1dbce9 100644
--- a/src/journal/journald-audit.c
+++ b/src/journal/journald-audit.c
@@ -517,6 +517,11 @@ int server_open_audit(Server *s) {
         static const int one = 1;
         int r;
 
+        if (!use_audit()) {
+                log_debug("Audit not supported.");
+                return 0;
+        }
+
         if (s->audit_fd < 0) {
                 static const union sockaddr_union sa = {
                         .nl.nl_family = AF_NETLINK,
diff --git a/src/shared/audit.c b/src/shared/audit.c
index 54148fc..fb5d41e 100644
--- a/src/shared/audit.c
+++ b/src/shared/audit.c
@@ -26,6 +26,7 @@
 #include "audit.h"
 #include "util.h"
 #include "process-util.h"
+#include "socket-util.h"
 #include "fileio.h"
 
 int audit_session_from_pid(pid_t pid, uint32_t *id) {
@@ -85,7 +86,13 @@ bool use_audit(void) {
                 if (fd < 0)
                         cached_use = errno != EAFNOSUPPORT && errno != EPROTONOSUPPORT;
                 else {
-                        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);
                 }
         }
-- 
2.1.4

Attachment: signature.asc
Description: Digital signature

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to