Merge ~enr0n/ubuntu/+source/systemd:ubuntu-focal-sru into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal

[Nick Rosbrook]

Re-upload these changes with a new version number due to the recent security update. Also address an SRU review comment to tweak the workaround for LP: #1991285.

[Lukas Märdian (slyon)]

Thanks. Security matches what we have in the archive (git-ubuntu).
The is_container check looks good to me after reading the SRU discussion.

Nice job of keeping a clean fast-forward git history, with using this squashed revert commit!

review: Approve

[Lukas Märdian (slyon)]

I pushed the corresponding branch & tags to our git repo and sponsored the upload:
https://launchpad.net/ubuntu/focal/+queue?queue_state=1&queue_text=systemd

diff --git a/debian/changelog b/debian/changelog
index 0edd425..9185ef8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,4 +1,4 @@
-systemd (245.4-4ubuntu3.20) focal; urgency=medium
+systemd (245.4-4ubuntu3.21) focal; urgency=medium
 
   * udev: avoid NIC renaming race with kernel (LP: #2002445)
     Files:
@@ -7,16 +7,28 @@ systemd (245.4-4ubuntu3.20) focal; urgency=medium
     - debian/patches/lp2002445-sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch
     - debian/patches/lp2002445-udev-attempt-device-rename-even-if-interface-is-up.patch
     - debian/patches/lp2002445-udev-net-allow-new-link-name-as-an-altname-before-renamin.patch
-    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=e34409f9d68a15220aa36cbae219d4f4987e5207
+    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=69ab4a02e828e20ea0ddbd75179324df7a8d1175
   * test-seccomp: accept ENOSYS from sysctl(2) too (LP: #1933090)
     Thanks to Roxana Nicolescu
     File: debian/patches/lp1933090-test-seccomp-accept-ENOSYS-from-sysctl-2-too.patch
-    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=706f2e0c787cade9609d0f2b37634d7eb4517beb
-  * debian/test: ignore systemd-remount-fs.service failure on armhf (LP: #1991285)
+    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=adaddd1441370ebcdb8bc33d7406b95d85b744f9
+  * debian/test: ignore systemd-remount-fs.service failure in containers (LP: #1991285)
     File: debian/tests/boot-and-services
-    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=cddc66ef6ecfb555dfd38c5f5fe8572efb424114
+    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=264bdc86f1e4dcd10e8d914d095581c54c33199a
 
- -- Nick Rosbrook <[e-mail]>  Mon, 27 Feb 2023 10:11:13 -0500
+ -- Nick Rosbrook <[e-mail]>  Wed, 15 Mar 2023 11:04:15 -0400
+
+systemd (245.4-4ubuntu3.20) focal-security; urgency=medium
+
+  * SECURITY UPDATE: buffer overrun vulnerability in format_timespan()
+    - debian/patches/CVE-2022-3821.patch: time-util: fix buffer-over-run
+    - CVE-2022-3821
+  * SECURITY UPDATE: information leak vulnerability in systemd-coredump
+    - debian/patches/CVE-2022-4415.patch: do not allow user to access
+      coredumps with changed uid/gid/capabilities
+    - CVE-2022-4415
+
+ -- Nishit Majithia <[e-mail]>  Thu, 02 Mar 2023 18:28:02 +0530
 
 systemd (245.4-4ubuntu3.19) focal; urgency=medium
 
diff --git a/debian/patches/CVE-2022-3821.patch b/debian/patches/CVE-2022-3821.patch
 new file mode 100644
index 0000000..e7ee561
--- /dev/null
+++ b/debian/patches/CVE-2022-3821.patch
@@ -0,0 +1,37 @@
+ Origin: bakcport, https://github.com/systemd/systemd/commit/9102c625a673a3246d7e73d8737f3494446bad4e
+
+From 9102c625a673a3246d7e73d8737f3494446bad4e Mon Sep 17 00:00:00 2001
+From: Yu Watanabe <[e-mail]>
+Date: Thu, 7 Jul 2022 18:27:02 +0900
+Subject: [PATCH] time-util: fix buffer-over-run
+
+Fixes #23928.
+---
+ src/basic/time-util.c     | 2 +-
+ src/test/test-time-util.c | 5 +++++
+ 2 files changed, 6 insertions(+), 1 deletion(-)
+
+--- systemd-245.4.orig/src/basic/time-util.c
++++ systemd-245.4/src/basic/time-util.c
+@@ -514,7 +514,7 @@ char *format_timespan(char *buf, size_t
+                         t = b;
+                 }
+ 
+-                n = MIN((size_t) k, l);
++                n = MIN((size_t) k, l-1);
+ 
+                 l -= n;
+                 p += n;
+--- systemd-245.4.orig/src/test/test-time-util.c
++++ systemd-245.4/src/test/test-time-util.c
+@@ -520,5 +520,10 @@ int main(int argc, char *argv[]) {
+         x++;
+         assert((time_t) x < 0);
+ 
++        /* See issue #23928. */
++        _cleanup_free_ char *buf;
++        assert_se(buf = new(char, 5));
++        assert_se(buf == format_timespan(buf, 5, 100005, 1000));
++
+         return 0;
+ }
diff --git a/debian/patches/CVE-2022-4415.patch b/debian/patches/CVE-2022-4415.patch
 new file mode 100644
index 0000000..527b156
--- /dev/null
+++ b/debian/patches/CVE-2022-4415.patch
@@ -0,0 +1,386 @@
+ Origin: backport, https://github.com/systemd/systemd-stable/commit/bb47600aeb38c68c857fbf0ee5f66c3144dd81ce
+
+From bb47600aeb38c68c857fbf0ee5f66c3144dd81ce Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <[e-mail]>
+Date: Mon, 28 Nov 2022 12:12:55 +0100
+Subject: [PATCH] coredump: do not allow user to access coredumps with changed
+ uid/gid/capabilities
+
+When the user starts a program which elevates its permissions via setuid,
+setgid, or capabilities set on the file, it may access additional information
+which would then be visible in the coredump. We shouldn't make the the coredump
+visible to the user in such cases.
+
+Reported-by: Matthias Gerstner <[e-mail]>
+
+This reads the /proc/<pid>/auxv file and attaches it to the process metadata as
+PROC_AUXV. Before the coredump is submitted, it is parsed and if either
+at_secure was set (which the kernel will do for processes that are setuid,
+setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file
+is not made accessible to the user. If we can't access this data, we assume the
+file should not be made accessible either. In principle we could also access
+the auxv data from a note in the core file, but that is much more complex and
+it seems better to use the stand-alone file that is provided by the kernel.
+
+Attaching auxv is both convient for this patch (because this way it's passed
+between the stages along with other fields), but I think it makes sense to save
+it in general.
+
+We use the information early in the core file to figure out if the program was
+32-bit or 64-bit and its endianness. This way we don't need heuristics to guess
+whether the format of the auxv structure. This test might reject some cases on
+fringe architecutes. But the impact would be limited: we just won't grant the
+user permissions to view the coredump file. If people report that we're missing
+some cases, we can always enhance this to support more architectures.
+
+I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and
+ppc64el, but not the whole coredump handling.
+
+(cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03)
+(cherry picked from commit 9b75a3d0502d6741c8ecb7175794345f8eb3827c)
+(cherry picked from commit efca5283dc791a07171f80eef84e14fdb58fad57)
+(cherry picked from commit 1d5e0e9910500f3c3584485f77bfc35e601036e3)
+(cherry picked from commit 8215e1527d859e77dd1378fd7e42bbd32130edb3)
+(cherry picked from commit 786df410b1cb3a2294c9a5d118c958525e7439e6)
+---
+ src/basic/io-util.h     |   9 ++
+ src/coredump/coredump.c | 196 +++++++++++++++++++++++++++++++++++++---
+ 2 files changed, 192 insertions(+), 13 deletions(-)
+
+--- systemd-245.4.orig/src/basic/io-util.h
++++ systemd-245.4/src/basic/io-util.h
+@@ -85,7 +85,16 @@ struct iovec_wrapper *iovw_new(void);
+ struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw);
+ struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw);
+ void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors);
++
+ int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len);
++static inline int iovw_consume(struct iovec_wrapper *iovw, void *data, size_t len) {
++        /* Move data into iovw or free on error */
++        int r = iovw_put(iovw, data, len);
++        if (r < 0)
++                free(data);
++        return r;
++}
++
+ int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value);
+ int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value);
+ void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new);
+--- systemd-245.4.orig/src/coredump/coredump.c
++++ systemd-245.4/src/coredump/coredump.c
+@@ -3,6 +3,7 @@
+ #include <errno.h>
+ #include <stdio.h>
+ #include <sys/prctl.h>
++#include <sys/auxv.h>
+ #include <sys/xattr.h>
+ #include <unistd.h>
+ 
+@@ -95,6 +96,7 @@ enum {
+ 
+         META_EXE = _META_MANDATORY_MAX,
+         META_UNIT,
++        META_PROC_AUXV,
+         _META_MAX
+ };
+ 
+@@ -109,10 +111,12 @@ static const char * const meta_field_nam
+         [META_COMM]              = "COREDUMP_COMM=",
+         [META_EXE]               = "COREDUMP_EXE=",
+         [META_UNIT]              = "COREDUMP_UNIT=",
++        [META_PROC_AUXV]         = "COREDUMP_PROC_AUXV=",
+ };
+ 
+ typedef struct Context {
+         const char *meta[_META_MAX];
++        size_t meta_size[_META_MAX];
+         pid_t pid;
+         bool is_pid1;
+         bool is_journald;
+@@ -171,7 +175,9 @@ static uint64_t storage_size_max(void) {
+         return 0;
+ }
+ 
+-static int fix_acl(int fd, uid_t uid) {
++static int fix_acl(int fd, uid_t uid, bool allow_user) {
++        assert(fd >= 0);
++        assert(uid_is_valid(uid));
+ 
+ #if HAVE_ACL
+         _cleanup_(acl_freep) acl_t acl = NULL;
+@@ -179,7 +185,9 @@ static int fix_acl(int fd, uid_t uid) {
+         acl_permset_t permset;
+         int r;
+ 
+-        assert(fd >= 0);
++        /* We don't allow users to read coredumps if the uid or capabilities were changed. */
++        if (!allow_user)
++                return 0;
+ 
+         if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY)
+                 return 0;
+@@ -258,7 +266,8 @@ static int fix_permissions(
+                 const char *filename,
+                 const char *target,
+                 const Context *context,
+-                uid_t uid) {
++                uid_t uid,
++                bool allow_user) {
+ 
+         int r;
+ 
+@@ -268,7 +277,7 @@ static int fix_permissions(
+ 
+         /* Ignore errors on these */
+         (void) fchmod(fd, 0640);
+-        (void) fix_acl(fd, uid);
++        (void) fix_acl(fd, uid, allow_user);
+         (void) fix_xattr(fd, context);
+ 
+         if (fsync(fd) < 0)
+@@ -339,6 +348,153 @@ static int make_filename(const Context *
+         return 0;
+ }
+ 
++static int parse_auxv64(
++                const uint64_t *auxv,
++                size_t size_bytes,
++                int *at_secure,
++                uid_t *uid,
++                uid_t *euid,
++                gid_t *gid,
++                gid_t *egid) {
++
++        assert(auxv || size_bytes == 0);
++
++        if (size_bytes % (2 * sizeof(uint64_t)) != 0)
++                return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes);
++
++        size_t words = size_bytes / sizeof(uint64_t);
++
++        /* Note that we set output variables even on error. */
++
++        for (size_t i = 0; i + 1 < words; i += 2)
++                switch (auxv[i]) {
++                case AT_SECURE:
++                        *at_secure = auxv[i + 1] != 0;
++                        break;
++                case AT_UID:
++                        *uid = auxv[i + 1];
++                        break;
++                case AT_EUID:
++                        *euid = auxv[i + 1];
++                        break;
++                case AT_GID:
++                        *gid = auxv[i + 1];
++                        break;
++                case AT_EGID:
++                        *egid = auxv[i + 1];
++                        break;
++                case AT_NULL:
++                        if (auxv[i + 1] != 0)
++                                goto error;
++                        return 0;
++                }
++ error:
++        return log_warning_errno(SYNTHETIC_ERRNO(ENODATA),
++                                 "AT_NULL terminator not found, cannot parse auxv structure.");
++}
++
++static int parse_auxv32(
++                const uint32_t *auxv,
++                size_t size_bytes,
++                int *at_secure,
++                uid_t *uid,
++                uid_t *euid,
++                gid_t *gid,
++                gid_t *egid) {
++
++        assert(auxv || size_bytes == 0);
++
++        size_t words = size_bytes / sizeof(uint32_t);
++
++        if (size_bytes % (2 * sizeof(uint32_t)) != 0)
++                return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes);
++
++        /* Note that we set output variables even on error. */
++
++        for (size_t i = 0; i + 1 < words; i += 2)
++                switch (auxv[i]) {
++                case AT_SECURE:
++                        *at_secure = auxv[i + 1] != 0;
++                        break;
++                case AT_UID:
++                        *uid = auxv[i + 1];
++                        break;
++                case AT_EUID:
++                        *euid = auxv[i + 1];
++                        break;
++                case AT_GID:
++                        *gid = auxv[i + 1];
++                        break;
++                case AT_EGID:
++                        *egid = auxv[i + 1];
++                        break;
++                case AT_NULL:
++                        if (auxv[i + 1] != 0)
++                                goto error;
++                        return 0;
++                }
++ error:
++        return log_warning_errno(SYNTHETIC_ERRNO(ENODATA),
++                                 "AT_NULL terminator not found, cannot parse auxv structure.");
++}
++
++static int grant_user_access(int core_fd, const Context *context) {
++        int at_secure = -1;
++        uid_t uid = UID_INVALID, euid = UID_INVALID;
++        uid_t gid = GID_INVALID, egid = GID_INVALID;
++        int r;
++
++        assert(core_fd >= 0);
++        assert(context);
++
++        if (!context->meta[META_PROC_AUXV])
++                return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), "No auxv data, not adjusting permissions.");
++
++        uint8_t elf[EI_NIDENT];
++        errno = 0;
++        if (pread(core_fd, &elf, sizeof(elf), 0) != sizeof(elf))
++                return log_warning_errno(errno_or_else(EIO),
++                                         "Failed to pread from coredump fd: %s", errno != 0 ? strerror_safe(errno) : "Unexpected EOF");
++
++        if (elf[EI_MAG0] != ELFMAG0 ||
++            elf[EI_MAG1] != ELFMAG1 ||
++            elf[EI_MAG2] != ELFMAG2 ||
++            elf[EI_MAG3] != ELFMAG3 ||
++            elf[EI_VERSION] != EV_CURRENT)
++                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
++                                      "Core file does not have ELF header, not adjusting permissions.");
++        if (!IN_SET(elf[EI_CLASS], ELFCLASS32, ELFCLASS64) ||
++            !IN_SET(elf[EI_DATA], ELFDATA2LSB, ELFDATA2MSB))
++                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
++                                      "Core file has strange ELF class, not adjusting permissions.");
++
++        if ((elf[EI_DATA] == ELFDATA2LSB) != (__BYTE_ORDER == __LITTLE_ENDIAN))
++                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
++                                      "Core file has non-native endianness, not adjusting permissions.");
++
++        if (elf[EI_CLASS] == ELFCLASS64)
++                r = parse_auxv64((const uint64_t*) context->meta[META_PROC_AUXV],
++                                 context->meta_size[META_PROC_AUXV],
++                                 &at_secure, &uid, &euid, &gid, &egid);
++        else
++                r = parse_auxv32((const uint32_t*) context->meta[META_PROC_AUXV],
++                                 context->meta_size[META_PROC_AUXV],
++                                 &at_secure, &uid, &euid, &gid, &egid);
++        if (r < 0)
++                return r;
++
++        /* We allow access if we got all the data and at_secure is not set and
++         * the uid/gid matches euid/egid. */
++        bool ret =
++                at_secure == 0 &&
++                uid != UID_INVALID && euid != UID_INVALID && uid == euid &&
++                gid != GID_INVALID && egid != GID_INVALID && gid == egid;
++        log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)",
++                  ret ? "permit" : "restrict",
++                  uid, euid, gid, egid, yes_no(at_secure));
++        return ret;
++}
++
+ static int save_external_coredump(
+                 const Context *context,
+                 int input_fd,
+@@ -404,6 +560,8 @@ static int save_external_coredump(
+                 goto fail;
+         }
+         *ret_truncated = r == 1;
++        bool allow_user = grant_user_access(fd, context) > 0;
++
+         if (*ret_truncated)
+                 log_struct(LOG_INFO,
+                            LOG_MESSAGE("Core file was truncated to %zu bytes.", max_size),
+@@ -445,7 +603,7 @@ static int save_external_coredump(
+                         goto fail_compressed;
+                 }
+ 
+-                r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid);
++                r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid, allow_user);
+                 if (r < 0)
+                         goto fail_compressed;
+ 
+@@ -468,7 +626,7 @@ static int save_external_coredump(
+ uncompressed:
+ #endif
+ 
+-        r = fix_permissions(fd, tmp, fn, context, uid);
++        r = fix_permissions(fd, tmp, fn, context, uid, allow_user);
+         if (r < 0)
+                 goto fail;
+ 
+@@ -719,7 +877,7 @@ static int change_uid_gid(const Context
+ }
+ 
+ static int submit_coredump(
+-                Context *context,
++                const Context *context,
+                 struct iovec_wrapper *iovw,
+                 int input_fd) {
+ 
+@@ -841,16 +999,15 @@ static int save_context(Context *context
+                 struct iovec *iovec = iovw->iovec + n;
+ 
+                 for (i = 0; i < ELEMENTSOF(meta_field_names); i++) {
+-                        char *p;
+-
+                         /* Note that these strings are NUL terminated, because we made sure that a
+                          * trailing NUL byte is in the buffer, though not included in the iov_len
+                          * count (see process_socket() and gather_pid_metadata_*()) */
+                         assert(((char*) iovec->iov_base)[iovec->iov_len] == 0);
+ 
+-                        p = startswith(iovec->iov_base, meta_field_names[i]);
++                        const char *p = startswith(iovec->iov_base, meta_field_names[i]);
+                         if (p) {
+                                 context->meta[i] = p;
++                                context->meta_size[i] = iovec->iov_len - strlen(meta_field_names[i]);
+                                 count++;
+                                 break;
+                         }
+@@ -1098,6 +1255,7 @@ static int gather_pid_metadata(struct io
+         uid_t owner_uid;
+         pid_t pid;
+         char *t;
++        size_t size;
+         const char *p;
+         int r;
+ 
+@@ -1163,13 +1321,26 @@ static int gather_pid_metadata(struct io
+                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t);
+ 
+         p = procfs_file_alloca(pid, "cgroup");
+-        if (read_full_file(p, &t, NULL) >=0)
++        if (read_full_file(p, &t, NULL) >= 0)
+                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t);
+ 
+         p = procfs_file_alloca(pid, "mountinfo");
+-        if (read_full_file(p, &t, NULL) >=0)
++        if (read_full_file(p, &t, NULL) >= 0)
+                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t);
+ 
++        /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */
++        p = procfs_file_alloca(pid, "auxv");
++        if (read_full_virtual_file(p, &t, &size) >= 0) {
++                char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1);
++                if (buf) {
++                        /* Add a dummy terminator to make save_context() happy. */
++                        *((uint8_t*) mempcpy(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size)) = '\0';
++                        (void) iovw_consume(iovw, buf, size + strlen("COREDUMP_PROC_AUXV="));
++                }
++
++                free(t);
++        }
++
+         if (get_process_cwd(pid, &t) >= 0)
+                 (void) iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t);
+ 
diff --git a/debian/patches/series b/debian/patches/series
index bae1736..49f6f2e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -185,6 +185,8 @@ lp1945225/0001-udev-net_id-parse-_SUN-ACPI-index-as-a-signed-intege.patch
 lp1945225/0002-udev-net_id-don-t-generate-slot-based-names-if-multi.patch
 lp1945225/0003-net_id-fix-newly-added-naming-scheme-name.patch
 lp1945225/0004-Add-remaining-supported-schemes-as-options-for-defau.patch
+CVE-2022-3821.patch
+CVE-2022-4415.patch
 lp2002445-udev-net-allow-new-link-name-as-an-altname-before-renamin.patch
 lp2002445-netlink-introduce-rtnl_get-delete_link_alternative_names.patch
 lp2002445-netlink-do-not-fail-when-new-interface-name-is-already-us.patch
diff --git a/debian/tests/boot-and-services b/debian/tests/boot-and-services
index 6090569..6c832be 100755
--- a/debian/tests/boot-and-services
+++ b/debian/tests/boot-and-services
@@ -59,10 +59,7 @@ class ServicesTest(unittest.TestCase):
         # https://bugs.debian.org/926138
         if is_container:
             failed = [f for f in failed if 'e2scrub_reap.service' not in f]
-        # LP: #1991285
-        arch = subprocess.check_output(['dpkg', '--print-architecture'])
-        arch = arch.decode('utf-8').strip()
-        if arch == 'armhf':
+            # LP: #1991285
             failed = [f for f in failed if 'systemd-remount-fs.service' not in f]
         if failed:
             for f in failed: