Merge pull request #28787 from yuwata/credential-next

core: do not leak mount for credentials directory if possible

src/core/credential.h

@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <unistd.h>
+
+#include "hash-funcs.h"
+
+typedef struct ExecContext ExecContext;
+typedef struct ExecParameters ExecParameters;
+typedef struct Unit Unit;
+
+/* A credential configured with LoadCredential= */
+typedef struct ExecLoadCredential {
+        char *id, *path;
+        bool encrypted;
+} ExecLoadCredential;
+
+/* A credential configured with SetCredential= */
+typedef struct ExecSetCredential {
+        char *id;
+        bool encrypted;
+        void *data;
+        size_t size;
+} ExecSetCredential;
+
+ExecSetCredential *exec_set_credential_free(ExecSetCredential *sc);
+DEFINE_TRIVIAL_CLEANUP_FUNC(ExecSetCredential*, exec_set_credential_free);
+
+ExecLoadCredential *exec_load_credential_free(ExecLoadCredential *lc);
+DEFINE_TRIVIAL_CLEANUP_FUNC(ExecLoadCredential*, exec_load_credential_free);
+
+extern const struct hash_ops exec_set_credential_hash_ops;
+extern const struct hash_ops exec_load_credential_hash_ops;
+
+bool exec_context_has_encrypted_credentials(ExecContext *c);
+bool exec_context_has_credentials(const ExecContext *c);
+
+int unit_add_default_credential_dependencies(Unit *u, const ExecContext *c);
+
+int exec_context_destroy_credentials(const ExecContext *c, const char *runtime_root, const char *unit);
+int setup_credentials(
+                const ExecContext *context,
+                const ExecParameters *params,
+                const char *unit,
+                uid_t uid,
+                gid_t gid,
+                char **ret_path,
+                int *ret_mount_fd);

src/core/dbus-execute.c

@@ -9,6 +9,7 @@
 #include "cap-list.h"
 #include "capability-util.h"
 #include "cpu-set-util.h"
+#include "credential.h"
 #include "creds-util.h"
 #include "dbus-execute.h"
 #include "dbus-util.h"

src/core/execute.h

@@ -176,20 +176,6 @@ typedef enum ExecCleanMask {
         _EXEC_CLEAN_MASK_INVALID = -EINVAL,
 } ExecCleanMask;
 
-/* A credential configured with LoadCredential= */
-typedef struct ExecLoadCredential {
-        char *id, *path;
-        bool encrypted;
-} ExecLoadCredential;
-
-/* A credential configured with SetCredential= */
-typedef struct ExecSetCredential {
-        char *id;
-        bool encrypted;
-        void *data;
-        size_t size;
-} ExecSetCredential;
-
 /* Encodes configuration parameters applied to invoked commands. Does not carry runtime data, but only configuration
  * changes sourced from unit files and suchlike. ExecContext objects are usually embedded into Unit objects, and do not
  * change after being loaded. */
@@ -482,15 +468,12 @@ void exec_context_done(ExecContext *c);
 void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix);
 
 int exec_context_destroy_runtime_directory(const ExecContext *c, const char *runtime_root);
-int exec_context_destroy_credentials(const ExecContext *c, const char *runtime_root, const char *unit);
 int exec_context_destroy_mount_ns_dir(Unit *u);
 
 const char* exec_context_fdname(const ExecContext *c, int fd_index);
 
 bool exec_context_may_touch_console(const ExecContext *c);
 bool exec_context_maintains_privileges(const ExecContext *c);
-bool exec_context_has_encrypted_credentials(ExecContext *c);
-bool exec_context_has_credentials(const ExecContext *context);
 
 int exec_context_get_effective_ioprio(const ExecContext *c);
 bool exec_context_get_effective_mount_apivfs(const ExecContext *c);
@@ -526,21 +509,12 @@ void exec_params_clear(ExecParameters *p);
 
 bool exec_context_get_cpu_affinity_from_numa(const ExecContext *c);
 
-ExecSetCredential *exec_set_credential_free(ExecSetCredential *sc);
-DEFINE_TRIVIAL_CLEANUP_FUNC(ExecSetCredential*, exec_set_credential_free);
-
-ExecLoadCredential *exec_load_credential_free(ExecLoadCredential *lc);
-DEFINE_TRIVIAL_CLEANUP_FUNC(ExecLoadCredential*, exec_load_credential_free);
-
 void exec_directory_done(ExecDirectory *d);
 int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink);
 void exec_directory_sort(ExecDirectory *d);
 
 ExecCleanMask exec_clean_mask_from_string(const char *s);
 
-extern const struct hash_ops exec_set_credential_hash_ops;
-extern const struct hash_ops exec_load_credential_hash_ops;
-
 const char* exec_output_to_string(ExecOutput i) _const_;
 ExecOutput exec_output_from_string(const char *s) _pure_;
 

src/core/load-fragment.c

@@ -28,6 +28,7 @@
 #include "conf-parser.h"
 #include "core-varlink.h"
 #include "cpu-set-util.h"
+#include "credential.h"
 #include "creds-util.h"
 #include "env-util.h"
 #include "errno-list.h"

src/core/meson.build

@@ -11,6 +11,7 @@ libcore_sources = files(
         'bpf-socket-bind.c',
         'cgroup.c',
         'core-varlink.c',
+        'credential.c',
         'dbus-automount.c',
         'dbus-cgroup.c',
         'dbus-device.c',

src/core/namespace.c

@@ -74,7 +74,8 @@ typedef enum MountMode {
         EXTENSION_DIRECTORIES, /* Bind-mounted outside the root directory, and used by subsequent mounts */
         EXTENSION_IMAGES, /* Mounted outside the root directory, and used by subsequent mounts */
         MQUEUEFS,
-        READWRITE_IMPLICIT, /* Should have the lowest priority. */
+        READWRITE_IMPLICIT, /* Should have the 2nd lowest priority. */
+        MKDIR,              /* Should have the lowest priority. */
         _MOUNT_MODE_MAX,
 } MountMode;
 
@@ -227,6 +228,7 @@ static const char * const mount_mode_table[_MOUNT_MODE_MAX] = {
         [EXEC]                 = "exec",
         [NOEXEC]               = "noexec",
         [MQUEUEFS]             = "mqueuefs",
+        [MKDIR]                = "mkdir",
 };
 
 /* Helper struct for naming simplicity and reusability */
@@ -1551,6 +1553,12 @@ static int apply_one_mount(
         case OVERLAY_MOUNT:
                 return mount_overlay(m);
 
+        case MKDIR:
+                r = mkdir_p_label(mount_entry_path(m), 0755);
+                if (r < 0)
+                        return r;
+                return 1;
+
         default:
                 assert_not_reached();
         }
@@ -2017,6 +2025,7 @@ int setup_namespace(
                 const char* tmp_dir,
                 const char* var_tmp_dir,
                 const char *creds_path,
+                int creds_fd,
                 const char *log_namespace,
                 unsigned long mount_propagation_flag,
                 VeritySettings *verity,
@@ -2339,13 +2348,22 @@ int setup_namespace(
                                 .flags = MS_NODEV|MS_STRICTATIME|MS_NOSUID|MS_NOEXEC,
                         };
 
-                        *(m++) = (MountEntry) {
-                                .path_const = creds_path,
-                                .mode = BIND_MOUNT,
-                                .read_only = true,
-                                .source_const = creds_path,
-                                .ignore = true,
-                        };
+                        /* If we have mount fd for credentials directory, then it will be mounted after
+                         * namespace is set up. So, here we only create the mount point. */
+
+                        if (creds_fd < 0)
+                                *(m++) = (MountEntry) {
+                                        .path_const = creds_path,
+                                        .mode = BIND_MOUNT,
+                                        .read_only = true,
+                                        .source_const = creds_path,
+                                        .ignore = true,
+                                };
+                        else
+                                *(m++) = (MountEntry) {
+                                        .path_const = creds_path,
+                                        .mode = MKDIR,
+                                };
                 } else {
                         /* If our service has no credentials store configured, then make the whole
                          * credentials tree inaccessible wholesale. */

src/core/namespace.h

@@ -122,6 +122,7 @@ int setup_namespace(
                 const char *tmp_dir,
                 const char *var_tmp_dir,
                 const char *creds_path,
+                int creds_fd,
                 const char *log_namespace,
                 unsigned long mount_propagation_flag,
                 VeritySettings *verity,

src/core/unit.c

@@ -20,6 +20,7 @@
 #include "cgroup-util.h"
 #include "chase.h"
 #include "core-varlink.h"
+#include "credential.h"
 #include "dbus-unit.h"
 #include "dbus.h"
 #include "dropin.h"
@@ -1375,31 +1376,16 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) {
                 r = unit_add_two_dependencies_by_name(u, UNIT_AFTER, UNIT_REQUIRES, varlink_socket_unit, true, UNIT_DEPENDENCY_FILE);
                 if (r < 0)
                         return r;
-        } else
+        } else {
                 r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_JOURNALD_SOCKET, true, UNIT_DEPENDENCY_FILE);
-        if (r < 0)
-                return r;
-
-        if (exec_context_has_credentials(c) && u->manager->prefix[EXEC_DIRECTORY_RUNTIME]) {
-                _cleanup_free_ char *p = NULL, *m = NULL;
-
-                /* Let's make sure the credentials directory of this service is unmounted *after* the service
-                 * itself shuts down. This only matters if mount namespacing is not used for the service, and
-                 * hence the credentials mount appears on the host. */
-
-                p = path_join(u->manager->prefix[EXEC_DIRECTORY_RUNTIME], "credentials", u->id);
-                if (!p)
-                        return -ENOMEM;
-
-                r = unit_name_from_path(p, ".mount", &m);
-                if (r < 0)
-                        return r;
-
-                r = unit_add_dependency_by_name(u, UNIT_AFTER, m, /* add_reference= */ true, UNIT_DEPENDENCY_FILE);
                 if (r < 0)
                         return r;
         }
 
+        r = unit_add_default_credential_dependencies(u, c);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 

src/test/test-execute.c

@@ -282,7 +282,11 @@ static void test_exec_cpuaffinity(Manager *m) {
 
 static void test_exec_credentials(Manager *m) {
         test(m, "exec-set-credential.service", 0, CLD_EXITED);
+        test(m, "exec-set-credential-with-mount-namespace.service", 0, CLD_EXITED);
+        test(m, "exec-set-credential-with-seccomp.service", 0, CLD_EXITED);
         test(m, "exec-load-credential.service", MANAGER_IS_SYSTEM(m) ? 0 : EXIT_CREDENTIALS, CLD_EXITED);
+        test(m, "exec-load-credential-with-mount-namespace.service", MANAGER_IS_SYSTEM(m) ? 0 : EXIT_CREDENTIALS, CLD_EXITED);
+        test(m, "exec-load-credential-with-seccomp.service", MANAGER_IS_SYSTEM(m) ? 0 : EXIT_CREDENTIALS, CLD_EXITED);
         test(m, "exec-credentials-dir-specifier.service", MANAGER_IS_SYSTEM(m) ? 0 : EXIT_CREDENTIALS, CLD_EXITED);
 }
 

src/test/test-namespace.c

@@ -194,6 +194,7 @@ TEST(protect_kernel_logs) {
                                     NULL,
                                     NULL,
                                     NULL,
+                                    -EBADF,
                                     NULL,
                                     0,
                                     NULL,

src/test/test-ns.c

@@ -96,6 +96,7 @@ int main(int argc, char *argv[]) {
                             tmp_dir,
                             var_tmp_dir,
                             NULL,
+                            -EBADF,
                             NULL,
                             0,
                             NULL,

test/test-execute/exec-load-credential-with-mount-namespace.service

@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for LoadCredential=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(cat %d/test-execute.load-credential)" = "foo"'
+Type=oneshot
+LoadCredential=test-execute.load-credential
+PrivateMounts=yes

test/test-execute/exec-load-credential-with-seccomp.service

@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for LoadCredential=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(cat %d/test-execute.load-credential)" = "foo"'
+Type=oneshot
+LoadCredential=test-execute.load-credential
+SystemCallFilter=~open_tree move_mount

test/test-execute/exec-set-credential-with-mount-namespace.service

@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for SetCredential=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(cat %d/test-execute.set-credential)" = "hoge"'
+Type=oneshot
+SetCredential=test-execute.set-credential:hoge
+PrivateMounts=yes

test/test-execute/exec-set-credential-with-seccomp.service

@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for SetCredential=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(cat %d/test-execute.set-credential)" = "hoge"'
+Type=oneshot
+SetCredential=test-execute.set-credential:hoge
+SystemCallFilter=~open_tree move_mount