2aacef
From e86a03c4f201745a683cfe1549a202d5ae636b07 Mon Sep 17 00:00:00 2001
5fc298
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
5fc298
Date: Mon, 28 Nov 2022 12:12:55 +0100
5fc298
Subject: [PATCH] coredump: do not allow user to access coredumps with changed
5fc298
 uid/gid/capabilities
5fc298
5fc298
When the user starts a program which elevates its permissions via setuid,
5fc298
setgid, or capabilities set on the file, it may access additional information
5fc298
which would then be visible in the coredump. We shouldn't make the the coredump
5fc298
visible to the user in such cases.
5fc298
5fc298
Reported-by: Matthias Gerstner <mgerstner@suse.de>
5fc298
5fc298
This reads the /proc/<pid>/auxv file and attaches it to the process metadata as
5fc298
PROC_AUXV. Before the coredump is submitted, it is parsed and if either
5fc298
at_secure was set (which the kernel will do for processes that are setuid,
5fc298
setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file
5fc298
is not made accessible to the user. If we can't access this data, we assume the
5fc298
file should not be made accessible either. In principle we could also access
5fc298
the auxv data from a note in the core file, but that is much more complex and
5fc298
it seems better to use the stand-alone file that is provided by the kernel.
5fc298
5fc298
Attaching auxv is both convient for this patch (because this way it's passed
5fc298
between the stages along with other fields), but I think it makes sense to save
5fc298
it in general.
5fc298
5fc298
We use the information early in the core file to figure out if the program was
5fc298
32-bit or 64-bit and its endianness. This way we don't need heuristics to guess
5fc298
whether the format of the auxv structure. This test might reject some cases on
5fc298
fringe architecutes. But the impact would be limited: we just won't grant the
5fc298
user permissions to view the coredump file. If people report that we're missing
5fc298
some cases, we can always enhance this to support more architectures.
5fc298
5fc298
I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and
5fc298
ppc64el, but not the whole coredump handling.
5fc298
5fc298
(cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03)
5fc298
2aacef
Resolves: #2155517
5fc298
---
5fc298
 src/basic/io-util.h     |   9 ++
2aacef
 src/coredump/coredump.c | 196 +++++++++++++++++++++++++++++++++++++---
2aacef
 2 files changed, 192 insertions(+), 13 deletions(-)
5fc298
5fc298
diff --git a/src/basic/io-util.h b/src/basic/io-util.h
5fc298
index 39728e06bc..3afb134266 100644
5fc298
--- a/src/basic/io-util.h
5fc298
+++ b/src/basic/io-util.h
5fc298
@@ -91,7 +91,16 @@ struct iovec_wrapper *iovw_new(void);
5fc298
 struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw);
5fc298
 struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw);
5fc298
 void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors);
5fc298
+
5fc298
 int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len);
5fc298
+static inline int iovw_consume(struct iovec_wrapper *iovw, void *data, size_t len) {
5fc298
+        /* Move data into iovw or free on error */
5fc298
+        int r = iovw_put(iovw, data, len);
5fc298
+        if (r < 0)
5fc298
+                free(data);
5fc298
+        return r;
5fc298
+}
5fc298
+
5fc298
 int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value);
5fc298
 int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value);
5fc298
 void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new);
5fc298
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
2aacef
index 7a181bdeeb..ea3d8c415a 100644
5fc298
--- a/src/coredump/coredump.c
5fc298
+++ b/src/coredump/coredump.c
5fc298
@@ -4,6 +4,7 @@
5fc298
 #include <stdio.h>
5fc298
 #include <sys/prctl.h>
5fc298
 #include <sys/statvfs.h>
5fc298
+#include <sys/auxv.h>
5fc298
 #include <sys/xattr.h>
5fc298
 #include <unistd.h>
5fc298
 
2aacef
@@ -106,6 +107,7 @@ enum {
5fc298
 
5fc298
         META_EXE = _META_MANDATORY_MAX,
5fc298
         META_UNIT,
5fc298
+        META_PROC_AUXV,
5fc298
         _META_MAX
5fc298
 };
5fc298
 
2aacef
@@ -120,10 +122,12 @@ static const char * const meta_field_names[_META_MAX] = {
5fc298
         [META_COMM]           = "COREDUMP_COMM=",
5fc298
         [META_EXE]            = "COREDUMP_EXE=",
5fc298
         [META_UNIT]           = "COREDUMP_UNIT=",
5fc298
+        [META_PROC_AUXV]      = "COREDUMP_PROC_AUXV=",
5fc298
 };
5fc298
 
5fc298
 typedef struct Context {
5fc298
         const char *meta[_META_MAX];
5fc298
+        size_t meta_size[_META_MAX];
5fc298
         pid_t pid;
5fc298
         bool is_pid1;
5fc298
         bool is_journald;
2aacef
@@ -185,13 +189,16 @@ static uint64_t storage_size_max(void) {
5fc298
         return 0;
5fc298
 }
5fc298
 
5fc298
-static int fix_acl(int fd, uid_t uid) {
5fc298
+static int fix_acl(int fd, uid_t uid, bool allow_user) {
5fc298
+        assert(fd >= 0);
5fc298
+        assert(uid_is_valid(uid));
5fc298
 
5fc298
 #if HAVE_ACL
5fc298
         int r;
5fc298
 
5fc298
-        assert(fd >= 0);
5fc298
-        assert(uid_is_valid(uid));
5fc298
+        /* We don't allow users to read coredumps if the uid or capabilities were changed. */
5fc298
+        if (!allow_user)
5fc298
+                return 0;
5fc298
 
5fc298
         if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY)
5fc298
                 return 0;
2aacef
@@ -251,7 +258,8 @@ static int fix_permissions(
5fc298
                 const char *filename,
5fc298
                 const char *target,
5fc298
                 const Context *context,
5fc298
-                uid_t uid) {
5fc298
+                uid_t uid,
5fc298
+                bool allow_user) {
5fc298
 
5fc298
         int r;
5fc298
 
2aacef
@@ -261,7 +269,7 @@ static int fix_permissions(
5fc298
 
5fc298
         /* Ignore errors on these */
5fc298
         (void) fchmod(fd, 0640);
5fc298
-        (void) fix_acl(fd, uid);
5fc298
+        (void) fix_acl(fd, uid, allow_user);
5fc298
         (void) fix_xattr(fd, context);
5fc298
 
5fc298
         r = fsync_full(fd);
2aacef
@@ -331,6 +339,153 @@ static int make_filename(const Context *context, char **ret) {
5fc298
         return 0;
5fc298
 }
5fc298
 
5fc298
+static int parse_auxv64(
5fc298
+                const uint64_t *auxv,
5fc298
+                size_t size_bytes,
5fc298
+                int *at_secure,
5fc298
+                uid_t *uid,
5fc298
+                uid_t *euid,
5fc298
+                gid_t *gid,
5fc298
+                gid_t *egid) {
5fc298
+
5fc298
+        assert(auxv || size_bytes == 0);
5fc298
+
5fc298
+        if (size_bytes % (2 * sizeof(uint64_t)) != 0)
5fc298
+                return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes);
5fc298
+
5fc298
+        size_t words = size_bytes / sizeof(uint64_t);
5fc298
+
5fc298
+        /* Note that we set output variables even on error. */
5fc298
+
5fc298
+        for (size_t i = 0; i + 1 < words; i += 2)
5fc298
+                switch (auxv[i]) {
5fc298
+                case AT_SECURE:
5fc298
+                        *at_secure = auxv[i + 1] != 0;
5fc298
+                        break;
5fc298
+                case AT_UID:
5fc298
+                        *uid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_EUID:
5fc298
+                        *euid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_GID:
5fc298
+                        *gid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_EGID:
5fc298
+                        *egid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_NULL:
5fc298
+                        if (auxv[i + 1] != 0)
5fc298
+                                goto error;
5fc298
+                        return 0;
5fc298
+                }
5fc298
+ error:
5fc298
+        return log_warning_errno(SYNTHETIC_ERRNO(ENODATA),
5fc298
+                                 "AT_NULL terminator not found, cannot parse auxv structure.");
5fc298
+}
5fc298
+
5fc298
+static int parse_auxv32(
5fc298
+                const uint32_t *auxv,
5fc298
+                size_t size_bytes,
5fc298
+                int *at_secure,
5fc298
+                uid_t *uid,
5fc298
+                uid_t *euid,
5fc298
+                gid_t *gid,
5fc298
+                gid_t *egid) {
5fc298
+
5fc298
+        assert(auxv || size_bytes == 0);
5fc298
+
5fc298
+        size_t words = size_bytes / sizeof(uint32_t);
5fc298
+
5fc298
+        if (size_bytes % (2 * sizeof(uint32_t)) != 0)
5fc298
+                return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes);
5fc298
+
5fc298
+        /* Note that we set output variables even on error. */
5fc298
+
5fc298
+        for (size_t i = 0; i + 1 < words; i += 2)
5fc298
+                switch (auxv[i]) {
5fc298
+                case AT_SECURE:
5fc298
+                        *at_secure = auxv[i + 1] != 0;
5fc298
+                        break;
5fc298
+                case AT_UID:
5fc298
+                        *uid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_EUID:
5fc298
+                        *euid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_GID:
5fc298
+                        *gid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_EGID:
5fc298
+                        *egid = auxv[i + 1];
5fc298
+                        break;
5fc298
+                case AT_NULL:
5fc298
+                        if (auxv[i + 1] != 0)
5fc298
+                                goto error;
5fc298
+                        return 0;
5fc298
+                }
5fc298
+ error:
5fc298
+        return log_warning_errno(SYNTHETIC_ERRNO(ENODATA),
5fc298
+                                 "AT_NULL terminator not found, cannot parse auxv structure.");
5fc298
+}
5fc298
+
5fc298
+static int grant_user_access(int core_fd, const Context *context) {
5fc298
+        int at_secure = -1;
5fc298
+        uid_t uid = UID_INVALID, euid = UID_INVALID;
5fc298
+        uid_t gid = GID_INVALID, egid = GID_INVALID;
5fc298
+        int r;
5fc298
+
5fc298
+        assert(core_fd >= 0);
5fc298
+        assert(context);
5fc298
+
5fc298
+        if (!context->meta[META_PROC_AUXV])
5fc298
+                return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), "No auxv data, not adjusting permissions.");
5fc298
+
5fc298
+        uint8_t elf[EI_NIDENT];
5fc298
+        errno = 0;
5fc298
+        if (pread(core_fd, &elf, sizeof(elf), 0) != sizeof(elf))
2aacef
+                return log_warning_errno(errno_or_else(EIO),
2aacef
+                                         "Failed to pread from coredump fd: %s", STRERROR_OR_EOF(errno));
5fc298
+
5fc298
+        if (elf[EI_MAG0] != ELFMAG0 ||
5fc298
+            elf[EI_MAG1] != ELFMAG1 ||
5fc298
+            elf[EI_MAG2] != ELFMAG2 ||
5fc298
+            elf[EI_MAG3] != ELFMAG3 ||
5fc298
+            elf[EI_VERSION] != EV_CURRENT)
5fc298
+                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
5fc298
+                                      "Core file does not have ELF header, not adjusting permissions.");
5fc298
+        if (!IN_SET(elf[EI_CLASS], ELFCLASS32, ELFCLASS64) ||
5fc298
+            !IN_SET(elf[EI_DATA], ELFDATA2LSB, ELFDATA2MSB))
5fc298
+                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
5fc298
+                                      "Core file has strange ELF class, not adjusting permissions.");
5fc298
+
5fc298
+        if ((elf[EI_DATA] == ELFDATA2LSB) != (__BYTE_ORDER == __LITTLE_ENDIAN))
5fc298
+                return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN),
5fc298
+                                      "Core file has non-native endianness, not adjusting permissions.");
5fc298
+
5fc298
+        if (elf[EI_CLASS] == ELFCLASS64)
5fc298
+                r = parse_auxv64((const uint64_t*) context->meta[META_PROC_AUXV],
5fc298
+                                 context->meta_size[META_PROC_AUXV],
5fc298
+                                 &at_secure, &uid, &euid, &gid, &egid);
5fc298
+        else
5fc298
+                r = parse_auxv32((const uint32_t*) context->meta[META_PROC_AUXV],
5fc298
+                                 context->meta_size[META_PROC_AUXV],
5fc298
+                                 &at_secure, &uid, &euid, &gid, &egid);
5fc298
+        if (r < 0)
5fc298
+                return r;
5fc298
+
5fc298
+        /* We allow access if we got all the data and at_secure is not set and
5fc298
+         * the uid/gid matches euid/egid. */
5fc298
+        bool ret =
5fc298
+                at_secure == 0 &&
5fc298
+                uid != UID_INVALID && euid != UID_INVALID && uid == euid &&
5fc298
+                gid != GID_INVALID && egid != GID_INVALID && gid == egid;
5fc298
+        log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)",
5fc298
+                  ret ? "permit" : "restrict",
5fc298
+                  uid, euid, gid, egid, yes_no(at_secure));
5fc298
+        return ret;
5fc298
+}
5fc298
+
5fc298
 static int save_external_coredump(
5fc298
                 const Context *context,
5fc298
                 int input_fd,
2aacef
@@ -453,6 +608,8 @@ static int save_external_coredump(
5fc298
                                 context->meta[META_ARGV_PID], context->meta[META_COMM]);
5fc298
         truncated = r == 1;
5fc298
 
5fc298
+        bool allow_user = grant_user_access(fd, context) > 0;
5fc298
+
5fc298
 #if HAVE_COMPRESSION
5fc298
         if (arg_compress) {
5fc298
                 _cleanup_(unlink_and_freep) char *tmp_compressed = NULL;
2aacef
@@ -490,7 +647,7 @@ static int save_external_coredump(
5fc298
                         uncompressed_size += partial_uncompressed_size;
5fc298
                 }
5fc298
 
5fc298
-                r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid);
5fc298
+                r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid, allow_user);
5fc298
                 if (r < 0)
5fc298
                         return r;
5fc298
 
2aacef
@@ -517,7 +674,7 @@ static int save_external_coredump(
2aacef
                            "SIZE_LIMIT=%"PRIu64, max_size,
5fc298
                            "MESSAGE_ID=" SD_MESSAGE_TRUNCATED_CORE_STR);
5fc298
 
5fc298
-        r = fix_permissions(fd, tmp, fn, context, uid);
5fc298
+        r = fix_permissions(fd, tmp, fn, context, uid, allow_user);
5fc298
         if (r < 0)
5fc298
                 return log_error_errno(r, "Failed to fix permissions and finalize coredump %s into %s: %m", coredump_tmpfile_name(tmp), fn);
5fc298
 
2aacef
@@ -765,7 +922,7 @@ static int change_uid_gid(const Context *context) {
5fc298
 }
5fc298
 
5fc298
 static int submit_coredump(
5fc298
-                Context *context,
5fc298
+                const Context *context,
5fc298
                 struct iovec_wrapper *iovw,
5fc298
                 int input_fd) {
5fc298
 
2aacef
@@ -944,16 +1101,15 @@ static int save_context(Context *context, const struct iovec_wrapper *iovw) {
5fc298
                 struct iovec *iovec = iovw->iovec + n;
5fc298
 
5fc298
                 for (size_t i = 0; i < ELEMENTSOF(meta_field_names); i++) {
5fc298
-                        char *p;
5fc298
-
5fc298
                         /* Note that these strings are NUL terminated, because we made sure that a
5fc298
                          * trailing NUL byte is in the buffer, though not included in the iov_len
5fc298
                          * count (see process_socket() and gather_pid_metadata_*()) */
5fc298
                         assert(((char*) iovec->iov_base)[iovec->iov_len] == 0);
5fc298
 
5fc298
-                        p = startswith(iovec->iov_base, meta_field_names[i]);
5fc298
+                        const char *p = startswith(iovec->iov_base, meta_field_names[i]);
5fc298
                         if (p) {
5fc298
                                 context->meta[i] = p;
5fc298
+                                context->meta_size[i] = iovec->iov_len - strlen(meta_field_names[i]);
5fc298
                                 break;
5fc298
                         }
2aacef
                 }
2aacef
@@ -1190,6 +1346,7 @@ static int gather_pid_metadata(struct iovec_wrapper *iovw, Context *context) {
5fc298
         uid_t owner_uid;
5fc298
         pid_t pid;
5fc298
         char *t;
5fc298
+        size_t size;
5fc298
         const char *p;
5fc298
         int r;
5fc298
 
2aacef
@@ -1254,13 +1411,26 @@ static int gather_pid_metadata(struct iovec_wrapper *iovw, Context *context) {
5fc298
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t);
5fc298
 
5fc298
         p = procfs_file_alloca(pid, "cgroup");
5fc298
-        if (read_full_virtual_file(p, &t, NULL) >=0)
5fc298
+        if (read_full_virtual_file(p, &t, NULL) >= 0)
5fc298
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t);
5fc298
 
5fc298
         p = procfs_file_alloca(pid, "mountinfo");
5fc298
-        if (read_full_virtual_file(p, &t, NULL) >=0)
5fc298
+        if (read_full_virtual_file(p, &t, NULL) >= 0)
5fc298
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t);
5fc298
 
5fc298
+        /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */
5fc298
+        p = procfs_file_alloca(pid, "auxv");
5fc298
+        if (read_full_virtual_file(p, &t, &size) >= 0) {
5fc298
+                char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1);
5fc298
+                if (buf) {
5fc298
+                        /* Add a dummy terminator to make save_context() happy. */
5fc298
+                        *((uint8_t*) mempcpy(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size)) = '\0';
5fc298
+                        (void) iovw_consume(iovw, buf, size + strlen("COREDUMP_PROC_AUXV="));
5fc298
+                }
5fc298
+
5fc298
+                free(t);
5fc298
+        }
5fc298
+
5fc298
         if (get_process_cwd(pid, &t) >= 0)
5fc298
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t);
5fc298