diff --git a/src/core/execute.c b/src/core/execute.c index 56e0eec205..664a3b96f7 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2434,7 +2434,9 @@ static int setup_keyring( uid_t uid, gid_t gid) { key_serial_t keyring; - int r; + int r = 0; + uid_t saved_uid; + gid_t saved_gid; assert(u); assert(context); @@ -2453,6 +2455,26 @@ static int setup_keyring( if (context->keyring_mode == EXEC_KEYRING_INHERIT) return 0; + /* Acquiring a reference to the user keyring is nasty. We briefly change identity in order to get things set up + * properly by the kernel. If we don't do that then we can't create it atomically, and that sucks for parallel + * execution. This mimics what pam_keyinit does, too. Setting up session keyring, to be owned by the right user + * & group is just as nasty as acquiring a reference to the user keyring. */ + + saved_uid = getuid(); + saved_gid = getgid(); + + if (gid_is_valid(gid) && gid != saved_gid) { + if (setregid(gid, -1) < 0) + return log_unit_error_errno(u, errno, "Failed to change GID for user keyring: %m"); + } + + if (uid_is_valid(uid) && uid != saved_uid) { + if (setreuid(uid, -1) < 0) { + r = log_unit_error_errno(u, errno, "Failed to change UID for user keyring: %m"); + goto out; + } + } + keyring = keyctl(KEYCTL_JOIN_SESSION_KEYRING, 0, 0, 0, 0); if (keyring == -1) { if (errno == ENOSYS) @@ -2462,12 +2484,36 @@ static int setup_keyring( else if (errno == EDQUOT) log_unit_debug_errno(u, errno, "Out of kernel keyrings to allocate, ignoring."); else - return log_unit_error_errno(u, errno, "Setting up kernel keyring failed: %m"); + r = log_unit_error_errno(u, errno, "Setting up kernel keyring failed: %m"); - return 0; + goto out; } - /* Populate they keyring with the invocation ID by default. */ + /* When requested link the user keyring into the session keyring. */ + if (context->keyring_mode == EXEC_KEYRING_SHARED) { + + if (keyctl(KEYCTL_LINK, + KEY_SPEC_USER_KEYRING, + KEY_SPEC_SESSION_KEYRING, 0, 0) < 0) { + r = log_unit_error_errno(u, errno, "Failed to link user keyring into session keyring: %m"); + goto out; + } + } + + /* Restore uid/gid back */ + if (uid_is_valid(uid) && uid != saved_uid) { + if (setreuid(saved_uid, -1) < 0) { + r = log_unit_error_errno(u, errno, "Failed to change UID back for user keyring: %m"); + goto out; + } + } + + if (gid_is_valid(gid) && gid != saved_gid) { + if (setregid(saved_gid, -1) < 0) + return log_unit_error_errno(u, errno, "Failed to change GID back for user keyring: %m"); + } + + /* Populate they keyring with the invocation ID by default, as original saved_uid. */ if (!sd_id128_is_null(u->invocation_id)) { key_serial_t key; @@ -2478,65 +2524,20 @@ static int setup_keyring( if (keyctl(KEYCTL_SETPERM, key, KEY_POS_VIEW|KEY_POS_READ|KEY_POS_SEARCH| KEY_USR_VIEW|KEY_USR_READ|KEY_USR_SEARCH, 0, 0) < 0) - return log_unit_error_errno(u, errno, "Failed to restrict invocation ID permission: %m"); + r = log_unit_error_errno(u, errno, "Failed to restrict invocation ID permission: %m"); } } - /* And now, make the keyring owned by the service's user */ - if (uid_is_valid(uid) || gid_is_valid(gid)) - if (keyctl(KEYCTL_CHOWN, keyring, uid, gid, 0) < 0) - return log_unit_error_errno(u, errno, "Failed to change ownership of session keyring: %m"); +out: + /* Revert back uid & gid for the the last time, and exit */ + /* no extra logging, as only the first already reported error matters */ + if (getuid() != saved_uid) + (void) setreuid(saved_uid, -1); - /* When requested link the user keyring into the session keyring. */ - if (context->keyring_mode == EXEC_KEYRING_SHARED) { - uid_t saved_uid; - gid_t saved_gid; + if (getgid() != saved_gid) + (void) setregid(saved_gid, -1); - /* Acquiring a reference to the user keyring is nasty. We briefly change identity in order to get things - * set up properly by the kernel. If we don't do that then we can't create it atomically, and that - * sucks for parallel execution. This mimics what pam_keyinit does, too.*/ - - saved_uid = getuid(); - saved_gid = getgid(); - - if (gid_is_valid(gid) && gid != saved_gid) { - if (setregid(gid, -1) < 0) - return log_unit_error_errno(u, errno, "Failed to change GID for user keyring: %m"); - } - - if (uid_is_valid(uid) && uid != saved_uid) { - if (setreuid(uid, -1) < 0) { - (void) setregid(saved_gid, -1); - return log_unit_error_errno(u, errno, "Failed to change UID for user keyring: %m"); - } - } - - if (keyctl(KEYCTL_LINK, - KEY_SPEC_USER_KEYRING, - KEY_SPEC_SESSION_KEYRING, 0, 0) < 0) { - - r = -errno; - - (void) setreuid(saved_uid, -1); - (void) setregid(saved_gid, -1); - - return log_unit_error_errno(u, r, "Failed to link user keyring into session keyring: %m"); - } - - if (uid_is_valid(uid) && uid != saved_uid) { - if (setreuid(saved_uid, -1) < 0) { - (void) setregid(saved_gid, -1); - return log_unit_error_errno(u, errno, "Failed to change UID back for user keyring: %m"); - } - } - - if (gid_is_valid(gid) && gid != saved_gid) { - if (setregid(saved_gid, -1) < 0) - return log_unit_error_errno(u, errno, "Failed to change GID back for user keyring: %m"); - } - } - - return 0; + return r; } static void append_socket_pair(int *array, unsigned *n, const int pair[2]) {