Bug#864189: unblock: systemd/232-25

Michael Biebl biebl at debian.org
Mon Jun 5 01:05:04 BST 2017


Package: release.debian.org
Severity: normal
User: release.debian.org at packages.debian.org
Usertags: unblock

Hi,

please consider unblocking systemd.

The changes include two fixes for selinux, a fix for a dist-upgrade
failure and an important performance regression.

None of those should affect the udev/libudev1 udeb, i.e. the installer.

That said, I've CCed debian-boot for a d-i/KiBi ack.

Here's an annotated changelog


systemd (232-25) unstable; urgency=medium

  * hwdb: Use path_join() to generate the hwdb_bin path.
    This ensures /lib/udev/hwdb.bin gets the correct SELinux context. Having
    double slashes in the path makes selabel_lookup_raw() return the wrong
    context. (Closes: #851933)

https://anonscm.debian.org/cgit/pkg-systemd/systemd.git/commit/?h=stretch&id=16508bf

I was asked by the SELinux maintainers to fix this for stretch. In the
end, it turned out to be a bug in libselinux (#863854). But the fix for
libselinux is rather invasive so will likely not make it into stretch
and it's easy to avoid triggering the bug, so I've decided to fix/work
around this in systemd.

  * selinux: Enable labeling and access checks for unprivileged users.
    Revert commit that inadvertently broke a lot of SELinux related
    functionality for both unprivileged users and systemd instances running
    as MANAGER_USER and instead deal with the auditd issue by checking for
    the CAP_AUDIT_WRITE capability before opening an audit netlink socket.
    (Closes: #863800)

https://anonscm.debian.org/cgit/pkg-systemd/systemd.git/commit/?h=stretch&id=5088d0

Laurent Bigonville, one of the SELinux maintainers, asked me to pull
those fixes for stretch. He tested the patches and confirmed that they
work. The patches are from upstream.

  * Revert "systemd-sysv: Add Conflicts: systemd-shim"
    Under certain conditions this confuses Jessies's apt which then tries to
    remove systemd while being the active init system, resulting in a failed
    dist-upgrade. While this turned out to be a bug in apt, avoid this
    situation by dropping the Conflicts. (Closes: #854041)

https://anonscm.debian.org/cgit/pkg-systemd/systemd.git/commit/?h=stretch&id=a99075

This is bug which imho is the most important one to get fixed for r0.
It was (sometimes) causing dist-upgrade failures, if prior to the upgrade
systemd-shim was installed. David Kalnischkies identified this as a bug
in apt, but since we can't retroactively fix apt in jessie, I decided to
drop this Conflicts again to avoid this situation.

  * link: Fix offload features initialization.
    This fixes a regression introduced in v232 which caused TCP
    segmentation offloads being disabled by default, resulting in
    significant performance issues under certain conditions. (Closes: #864073)

https://anonscm.debian.org/cgit/pkg-systemd/systemd.git/commit/?h=stretch&id=551b79

This seemed like a rather straightforward fix which was unfortuantely
only reported the other day. Otherwise I would have pulled it earlier.
The patch is from upstream.

Full debdiff is attached as well.

Regards,
Michael

unblock systemd/232-25

-- System Information:
Debian Release: 9.0
  APT prefers unstable
  APT policy: (500, 'unstable'), (200, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=de_DE.utf8, LC_CTYPE=de_DE.utf8 (charmap=UTF-8), LANGUAGE=de_DE.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
-------------- next part --------------
diff --git a/debian/changelog b/debian/changelog
index 68276b7..d3789db 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,27 @@
+systemd (232-25) unstable; urgency=medium
+
+  * hwdb: Use path_join() to generate the hwdb_bin path.
+    This ensures /lib/udev/hwdb.bin gets the correct SELinux context. Having
+    double slashes in the path makes selabel_lookup_raw() return the wrong
+    context. (Closes: #851933)
+  * selinux: Enable labeling and access checks for unprivileged users.
+    Revert commit that inadvertently broke a lot of SELinux related
+    functionality for both unprivileged users and systemd instances running
+    as MANAGER_USER and instead deal with the auditd issue by checking for
+    the CAP_AUDIT_WRITE capability before opening an audit netlink socket.
+    (Closes: #863800)
+  * Revert "systemd-sysv: Add Conflicts: systemd-shim"
+    Under certain conditions this confuses Jessies's apt which then tries to
+    remove systemd while being the active init system, resulting in a failed
+    dist-upgrade. While this turned out to be a bug in apt, avoid this
+    situation by dropping the Conflicts. (Closes: #854041)
+  * link: Fix offload features initialization.
+    This fixes a regression introduced in v232 which caused TCP
+    segmentation offloads being disabled by default, resulting in
+    significant performance issues under certain conditions. (Closes: #864073)
+
+ -- Michael Biebl <biebl at debian.org>  Sun, 04 Jun 2017 22:58:32 +0200
+
 systemd (232-24) unstable; urgency=medium
 
   [ Felipe Sateler ]
diff --git a/debian/control b/debian/control
index c4e7db1..15df6f7 100644
--- a/debian/control
+++ b/debian/control
@@ -106,7 +106,6 @@ Conflicts: sysvinit-core,
            upstart-sysv,
            openrc (<< 0.20.4-2.1),
            file-rc,
-           systemd-shim,
 Replaces: sysvinit (<< 2.88dsf-44~),
           sysvinit-core,
           upstart (<< 1.13.2-0ubuntu10~),
diff --git a/debian/patches/Revert-selinux-split-up-mac_selinux_have-from-mac_selinux.patch b/debian/patches/Revert-selinux-split-up-mac_selinux_have-from-mac_selinux.patch
new file mode 100644
index 0000000..4ceb643
--- /dev/null
+++ b/debian/patches/Revert-selinux-split-up-mac_selinux_have-from-mac_selinux.patch
@@ -0,0 +1,225 @@
+From: Gary Tierney <gary.tierney at gmx.com>
+Date: Tue, 2 May 2017 17:42:19 +0100
+Subject: Revert "selinux: split up mac_selinux_have() from mac_selinux_use()"
+
+This reverts commit 6355e75610a8d47fc3ba5ab8bd442172a2cfe574.
+
+The previously mentioned commit inadvertently broke a lot of SELinux related
+functionality for both unprivileged users and systemd instances running as
+MANAGER_USER.  In particular, setting the correct SELinux context after a User=
+directive is used would fail to work since we attempt to set the security
+context after changing UID.  Additionally, it causes activated socket units to
+be mislabeled for systemd --user processes since setsockcreatecon() would never
+be called.
+
+Reverting this fixes the issues with labeling outlined above, and reinstates
+SELinux access checks on unprivileged user services.
+
+(cherry picked from commit 6d395665e5ce7b64f3de4c9550c0779843e6cc44)
+---
+ src/basic/selinux-util.c           | 20 +++++---------------
+ src/basic/selinux-util.h           |  1 -
+ src/journal/journald-native.c      |  2 +-
+ src/journal/journald-server.c      |  2 +-
+ src/journal/journald-stream.c      |  2 +-
+ src/journal/journald-syslog.c      |  2 +-
+ src/libsystemd/sd-bus/bus-socket.c |  2 +-
+ src/shared/condition.c             |  2 +-
+ src/test/test-condition.c          |  2 +-
+ src/test/test-selinux.c            |  8 ++++----
+ 10 files changed, 16 insertions(+), 27 deletions(-)
+
+diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c
+index bc07654..380285d 100644
+--- a/src/basic/selinux-util.c
++++ b/src/basic/selinux-util.c
+@@ -53,7 +53,7 @@ static struct selabel_handle *label_hnd = NULL;
+ #define log_enforcing(...) log_full(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, __VA_ARGS__)
+ #endif
+ 
+-bool mac_selinux_have(void) {
++bool mac_selinux_use(void) {
+ #ifdef HAVE_SELINUX
+         if (cached_use < 0)
+                 cached_use = is_selinux_enabled() > 0;
+@@ -64,16 +64,6 @@ bool mac_selinux_have(void) {
+ #endif
+ }
+ 
+-bool mac_selinux_use(void) {
+-        if (!mac_selinux_have())
+-                return false;
+-
+-        /* Never try to configure SELinux features if we aren't
+-         * root */
+-
+-        return getuid() == 0;
+-}
+-
+ void mac_selinux_retest(void) {
+ #ifdef HAVE_SELINUX
+         cached_use = -1;
+@@ -205,7 +195,7 @@ int mac_selinux_get_create_label_from_exe(const char *exe, char **label) {
+         assert(exe);
+         assert(label);
+ 
+-        if (!mac_selinux_have())
++        if (!mac_selinux_use())
+                 return -EOPNOTSUPP;
+ 
+         r = getcon_raw(&mycon);
+@@ -231,7 +221,7 @@ int mac_selinux_get_our_label(char **label) {
+         assert(label);
+ 
+ #ifdef HAVE_SELINUX
+-        if (!mac_selinux_have())
++        if (!mac_selinux_use())
+                 return -EOPNOTSUPP;
+ 
+         r = getcon_raw(label);
+@@ -255,7 +245,7 @@ int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char *
+         assert(exe);
+         assert(label);
+ 
+-        if (!mac_selinux_have())
++        if (!mac_selinux_use())
+                 return -EOPNOTSUPP;
+ 
+         r = getcon_raw(&mycon);
+@@ -310,7 +300,7 @@ char* mac_selinux_free(char *label) {
+         if (!label)
+                 return NULL;
+ 
+-        if (!mac_selinux_have())
++        if (!mac_selinux_use())
+                 return NULL;
+ 
+ 
+diff --git a/src/basic/selinux-util.h b/src/basic/selinux-util.h
+index ce6bc8e..5bf7236 100644
+--- a/src/basic/selinux-util.h
++++ b/src/basic/selinux-util.h
+@@ -26,7 +26,6 @@
+ #include "macro.h"
+ 
+ bool mac_selinux_use(void);
+-bool mac_selinux_have(void);
+ void mac_selinux_retest(void);
+ 
+ int mac_selinux_init(void);
+diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
+index bba75b6..128e951 100644
+--- a/src/journal/journald-native.c
++++ b/src/journal/journald-native.c
+@@ -478,7 +478,7 @@ int server_open_native_socket(Server*s) {
+                 return log_error_errno(errno, "SO_PASSCRED failed: %m");
+ 
+ #ifdef HAVE_SELINUX
+-        if (mac_selinux_have()) {
++        if (mac_selinux_use()) {
+                 r = setsockopt(s->native_fd, SOL_SOCKET, SO_PASSSEC, &one, sizeof(one));
+                 if (r < 0)
+                         log_warning_errno(errno, "SO_PASSSEC failed: %m");
+diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
+index 2e8954a..239b099 100644
+--- a/src/journal/journald-server.c
++++ b/src/journal/journald-server.c
+@@ -911,7 +911,7 @@ static void dispatch_message_real(
+                 }
+ 
+ #ifdef HAVE_SELINUX
+-                if (mac_selinux_have()) {
++                if (mac_selinux_use()) {
+                         if (label) {
+                                 x = alloca(strlen("_SELINUX_CONTEXT=") + label_len + 1);
+ 
+diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
+index bc092f3..77551dc 100644
+--- a/src/journal/journald-stream.c
++++ b/src/journal/journald-stream.c
+@@ -494,7 +494,7 @@ static int stdout_stream_install(Server *s, int fd, StdoutStream **ret) {
+         if (r < 0)
+                 return log_error_errno(r, "Failed to determine peer credentials: %m");
+ 
+-        if (mac_selinux_have()) {
++        if (mac_selinux_use()) {
+                 r = getpeersec(fd, &stream->label);
+                 if (r < 0 && r != -EOPNOTSUPP)
+                         (void) log_warning_errno(r, "Failed to determine peer security context: %m");
+diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c
+index 0609b4b..8856cc8 100644
+--- a/src/journal/journald-syslog.c
++++ b/src/journal/journald-syslog.c
+@@ -410,7 +410,7 @@ int server_open_syslog_socket(Server *s) {
+                 return log_error_errno(errno, "SO_PASSCRED failed: %m");
+ 
+ #ifdef HAVE_SELINUX
+-        if (mac_selinux_have()) {
++        if (mac_selinux_use()) {
+                 r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_PASSSEC, &one, sizeof(one));
+                 if (r < 0)
+                         log_warning_errno(errno, "SO_PASSSEC failed: %m");
+diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
+index cfd7753..83f82be 100644
+--- a/src/libsystemd/sd-bus/bus-socket.c
++++ b/src/libsystemd/sd-bus/bus-socket.c
+@@ -607,7 +607,7 @@ static void bus_get_peercred(sd_bus *b) {
+         b->ucred_valid = getpeercred(b->input_fd, &b->ucred) >= 0;
+ 
+         /* Get the SELinux context of the peer */
+-        if (mac_selinux_have()) {
++        if (mac_selinux_use()) {
+                 r = getpeersec(b->input_fd, &b->label);
+                 if (r < 0 && r != -EOPNOTSUPP)
+                         log_debug_errno(r, "Failed to determine peer security context: %m");
+diff --git a/src/shared/condition.c b/src/shared/condition.c
+index 8bd6a51..b22f7f6 100644
+--- a/src/shared/condition.c
++++ b/src/shared/condition.c
+@@ -236,7 +236,7 @@ static int condition_test_security(Condition *c) {
+         assert(c->type == CONDITION_SECURITY);
+ 
+         if (streq(c->parameter, "selinux"))
+-                return mac_selinux_have();
++                return mac_selinux_use();
+         if (streq(c->parameter, "smack"))
+                 return mac_smack_use();
+         if (streq(c->parameter, "apparmor"))
+diff --git a/src/test/test-condition.c b/src/test/test-condition.c
+index dd985f5..3bb7f92 100644
+--- a/src/test/test-condition.c
++++ b/src/test/test-condition.c
+@@ -243,7 +243,7 @@ static void test_condition_test_security(void) {
+ 
+         condition = condition_new(CONDITION_SECURITY, "selinux", false, true);
+         assert_se(condition);
+-        assert_se(condition_test(condition) != mac_selinux_have());
++        assert_se(condition_test(condition) != mac_selinux_use());
+         condition_free(condition);
+ 
+         condition = condition_new(CONDITION_SECURITY, "ima", false, false);
+diff --git a/src/test/test-selinux.c b/src/test/test-selinux.c
+index 7545ad3..d60b984 100644
+--- a/src/test/test-selinux.c
++++ b/src/test/test-selinux.c
+@@ -35,16 +35,16 @@ static void test_testing(void) {
+         b = mac_selinux_use();
+         log_info("mac_selinux_use ? %s", yes_no(b));
+ 
+-        b = mac_selinux_have();
+-        log_info("mac_selinux_have ? %s", yes_no(b));
++        b = mac_selinux_use();
++        log_info("mac_selinux_use ? %s", yes_no(b));
+ 
+         mac_selinux_retest();
+ 
+         b = mac_selinux_use();
+         log_info("mac_selinux_use ? %s", yes_no(b));
+ 
+-        b = mac_selinux_have();
+-        log_info("mac_selinux_have ? %s", yes_no(b));
++        b = mac_selinux_use();
++        log_info("mac_selinux_use ? %s", yes_no(b));
+ }
+ 
+ static void test_loading(void) {
diff --git a/debian/patches/audit-fd-check-for-CAP_AUDIT_WRITE-before-opening-an-audi.patch b/debian/patches/audit-fd-check-for-CAP_AUDIT_WRITE-before-opening-an-audi.patch
new file mode 100644
index 0000000..fdbf491
--- /dev/null
+++ b/debian/patches/audit-fd-check-for-CAP_AUDIT_WRITE-before-opening-an-audi.patch
@@ -0,0 +1,41 @@
+From: Gary Tierney <gary.tierney at gmx.com>
+Date: Tue, 2 May 2017 21:05:32 +0100
+Subject: audit-fd: check for CAP_AUDIT_WRITE before opening an audit socket
+
+Adds a check to audit-fd.c to ensure that CAP_AUDIT_WRITE is present in
+the set of effective capabilities before opening an audit netlink
+socket.  This ensures that unprivileged systemd instances (MANAGER_USER)
+don't try to log AVC permission checks with the audit subsystem when
+CAP_AUDIT_WRITE is not present.
+
+(cherry picked from commit b3fb3c01eeedd1995c74dbe30fe33f189bc828e2)
+---
+ src/core/audit-fd.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/src/core/audit-fd.c b/src/core/audit-fd.c
+index 76afe3f..a91906b 100644
+--- a/src/core/audit-fd.c
++++ b/src/core/audit-fd.c
+@@ -27,6 +27,7 @@
+ #include <libaudit.h>
+ #include <stdbool.h>
+ 
++#include "capability-util.h"
+ #include "fd-util.h"
+ #include "log.h"
+ #include "util.h"
+@@ -37,6 +38,13 @@ static int audit_fd;
+ int get_audit_fd(void) {
+ 
+         if (!initialized) {
++                if (have_effective_cap(CAP_AUDIT_WRITE) == 0) {
++                        audit_fd = -EPERM;
++                        initialized = true;
++
++                        return audit_fd;
++                }
++
+                 audit_fd = audit_open();
+ 
+                 if (audit_fd < 0) {
diff --git a/debian/patches/debian/Re-enable-journal-forwarding-to-syslog.patch b/debian/patches/debian/Re-enable-journal-forwarding-to-syslog.patch
index 9dcbce8..2178269 100644
--- a/debian/patches/debian/Re-enable-journal-forwarding-to-syslog.patch
+++ b/debian/patches/debian/Re-enable-journal-forwarding-to-syslog.patch
@@ -30,7 +30,7 @@ index 9daa964..e829d63 100644
          <literal>systemd.journald.forward_to_syslog</literal>,
          <literal>systemd.journald.forward_to_kmsg</literal>,
 diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
-index 2e8954a..cfefe9d 100644
+index 239b099..29119a2 100644
 --- a/src/journal/journald-server.c
 +++ b/src/journal/journald-server.c
 @@ -1885,6 +1885,7 @@ int server_init(Server *s) {
diff --git a/debian/patches/hwdb-use-path_join-to-generate-the-hwdb_bin-path-6063.patch b/debian/patches/hwdb-use-path_join-to-generate-the-hwdb_bin-path-6063.patch
new file mode 100644
index 0000000..00f5b1b
--- /dev/null
+++ b/debian/patches/hwdb-use-path_join-to-generate-the-hwdb_bin-path-6063.patch
@@ -0,0 +1,31 @@
+From: Michael Biebl <mbiebl at gmail.com>
+Date: Thu, 1 Jun 2017 03:21:11 +0200
+Subject: hwdb: use path_join() to generate the hwdb_bin path (#6063)
+
+This avoids having double slashes which can confuse selinux.
+(cherry picked from commit 0aac506b64dd6102374635290ca979b080d1192f)
+---
+ src/hwdb/hwdb.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/src/hwdb/hwdb.c b/src/hwdb/hwdb.c
+index 1d16d9f..de2f403 100644
+--- a/src/hwdb/hwdb.c
++++ b/src/hwdb/hwdb.c
+@@ -31,6 +31,7 @@
+ #include "hwdb-util.h"
+ #include "label.h"
+ #include "mkdir.h"
++#include "path-util.h"
+ #include "selinux-util.h"
+ #include "strbuf.h"
+ #include "string-util.h"
+@@ -669,7 +670,7 @@ static int hwdb_update(int argc, char *argv[], void *userdata) {
+         log_debug("strings dedup'ed: %8zu bytes (%8zu)",
+                   trie->strings->dedup_len, trie->strings->dedup_count);
+ 
+-        hwdb_bin = strjoin(arg_root, "/", arg_hwdb_bin_dir, "/hwdb.bin");
++        hwdb_bin = path_join(arg_root, arg_hwdb_bin_dir, "hwdb.bin");
+         if (!hwdb_bin)
+                 return -ENOMEM;
+ 
diff --git a/debian/patches/link-fix-offload-features-initialization-4639.patch b/debian/patches/link-fix-offload-features-initialization-4639.patch
new file mode 100644
index 0000000..cf98585
--- /dev/null
+++ b/debian/patches/link-fix-offload-features-initialization-4639.patch
@@ -0,0 +1,22 @@
+From: Maks Naumov <maksqwe1 at ukr.net>
+Date: Mon, 14 Nov 2016 12:33:43 +0200
+Subject: link: fix offload features initialization (#4639)
+
+(cherry picked from commit 45d34fa7fb9f587f09e8cca882966050b4cbb5be)
+---
+ src/udev/net/link-config.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
+index ece9248..fa8fe1f 100644
+--- a/src/udev/net/link-config.c
++++ b/src/udev/net/link-config.c
+@@ -168,7 +168,7 @@ static int load_link(link_config_ctx *ctx, const char *filename) {
+         link->wol = _WOL_INVALID;
+         link->duplex = _DUP_INVALID;
+ 
+-        memset(&link->features, -1, _NET_DEV_FEAT_MAX);
++        memset(&link->features, -1, sizeof(link->features));
+ 
+         r = config_parse(NULL, filename, file,
+                          "Match\0Link\0Ethernet\0",
diff --git a/debian/patches/series b/debian/patches/series
index adc86a7..621b176 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -63,6 +63,10 @@ journal-fix-up-syslog-facility-when-forwarding-native-mes.patch
 machinectl-make-sure-that-inability-to-get-OS-version-isn.patch
 nspawn-support-ephemeral-boots-from-images.patch
 resolved-bugfix-of-null-pointer-p-question-dereferencing-.patch
+hwdb-use-path_join-to-generate-the-hwdb_bin-path-6063.patch
+Revert-selinux-split-up-mac_selinux_have-from-mac_selinux.patch
+audit-fd-check-for-CAP_AUDIT_WRITE-before-opening-an-audi.patch
+link-fix-offload-features-initialization-4639.patch
 debian/Use-Debian-specific-config-files.patch
 debian/don-t-try-to-start-autovt-units-when-not-running-wit.patch
 debian/Make-logind-hostnamed-localed-timedated-D-Bus-activa.patch


More information about the Pkg-systemd-maintainers mailing list