From: Mike Yuan Date: Wed, 4 Feb 2026 01:41:07 +0000 (+0100) Subject: ask-password-api: several modernizations for ask_password_agent() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4153a2a6f410458d7812c3b8f2d2de12d7a06170;p=thirdparty%2Fsystemd.git ask-password-api: several modernizations for ask_password_agent() * Replace goto cleanup with block_signals_reset + CLEANUP_TMPFILE_AT * Use RENAME_NOREPLACE to make sure we don't overwrite any ongoing request * Reword log messages a bit --- diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 57536688005..8882995351a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -831,10 +831,6 @@ int ask_password_agent( _cleanup_close_ int socket_fd = -EBADF, signal_fd = -EBADF, inotify_fd = -EBADF, dfd = -EBADF; _cleanup_(unlink_and_freep) char *socket_name = NULL; - _cleanup_free_ char *temp = NULL, *final = NULL; - _cleanup_strv_free_erase_ char **l = NULL; - _cleanup_fclose_ FILE *f = NULL; - sigset_t mask, oldmask; int r; assert(req); @@ -850,64 +846,59 @@ int ask_password_agent( if (req->flag_file) return -EOPNOTSUPP; - assert_se(sigemptyset(&mask) >= 0); - assert_se(sigset_add_many(&mask, SIGINT, SIGTERM) >= 0); - assert_se(sigprocmask(SIG_BLOCK, &mask, &oldmask) >= 0); - _cleanup_free_ char *askpwdir = NULL; r = get_ask_password_directory_for_flags(flags, &askpwdir); if (r < 0) - goto finish; - if (r == 0) { - r = -ENXIO; - goto finish; - } + return r; + if (r == 0) + return -ENXIO; - dfd = open_mkdir(askpwdir, O_RDONLY|O_CLOEXEC, 0755); - if (dfd < 0) { - r = log_debug_errno(dfd, "Failed to open directory '%s': %m", askpwdir); - goto finish; - } + dfd = open_mkdir(askpwdir, O_CLOEXEC, 0755); + if (dfd < 0) + return log_debug_errno(dfd, "Failed to open directory '%s': %m", askpwdir); if (FLAGS_SET(flags, ASK_PASSWORD_ACCEPT_CACHED) && req->keyring) { r = ask_password_keyring(req, flags, ret); - if (r >= 0) { - r = 0; - goto finish; - } else if (r != -ENOKEY) - goto finish; + if (r != -ENOKEY) + return r; inotify_fd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK); - if (inotify_fd < 0) { - r = -errno; - goto finish; - } + if (inotify_fd < 0) + return -errno; r = inotify_add_watch_fd(inotify_fd, dfd, IN_ONLYDIR|IN_ATTRIB /* for mtime */); if (r < 0) - goto finish; + return r; } - if (asprintf(&final, "ask.%" PRIu64, random_u64()) < 0) { - r = -ENOMEM; - goto finish; - } + sigset_t mask, oldmask; + _unused_ _cleanup_(block_signals_reset) sigset_t *saved_ssp = NULL; - r = fopen_temporary_at(dfd, final, &f, &temp); + assert_se(sigemptyset(&mask) >= 0); + assert_se(sigset_add_many(&mask, SIGINT, SIGTERM) >= 0); + assert_se(sigprocmask(SIG_BLOCK, &mask, &oldmask) >= 0); + + saved_ssp = &oldmask; + + _cleanup_free_ char *fname = NULL, *final = NULL; + _cleanup_fclose_ FILE *f = NULL; + + if (asprintf(&final, "ask.%" PRIu64, random_u64()) < 0) + return -ENOMEM; + + r = fopen_temporary_at(dfd, final, &f, &fname); if (r < 0) - goto finish; + return r; + + CLEANUP_TMPFILE_AT(dfd, fname); signal_fd = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC); - if (signal_fd < 0) { - r = -errno; - goto finish; - } + if (signal_fd < 0) + return -errno; socket_fd = create_socket(askpwdir, &socket_name); - if (socket_fd < 0) { - r = socket_fd; - goto finish; - } + if (socket_fd < 0) + return socket_fd; fprintf(f, "[Ask]\n" @@ -933,21 +924,19 @@ int ask_password_agent( if (req->id) fprintf(f, "Id=%s\n", req->id); - if (fchmod(fileno(f), 0644) < 0) { - r = -errno; - goto finish; - } + if (fchmod(fileno(f), 0644) < 0) + return -errno; r = fflush_and_check(f); if (r < 0) - goto finish; + return r; - if (renameat(dfd, temp, dfd, final) < 0) { - r = -errno; - goto finish; - } + /* Paranoia: never clobber an ongoing request */ + r = rename_noreplace(dfd, fname, dfd, final); + if (r < 0) + return r; - temp = mfree(temp); + free_and_replace(fname, final); enum { POLL_SOCKET, @@ -969,20 +958,17 @@ int ask_password_agent( .events = POLLIN, }; if (req->hup_fd >= 0) - pollfd[hup_fd_idx = n_pollfd ++] = (struct pollfd) { + pollfd[hup_fd_idx = n_pollfd++] = (struct pollfd) { .fd = req->hup_fd, .events = POLLHUP, }; assert(n_pollfd <= _POLL_MAX); + _cleanup_strv_free_erase_ char **l = NULL; + for (;;) { - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; - char passphrase[LINE_MAX+1]; - struct iovec iovec; - struct ucred *ucred; usec_t timeout; - ssize_t n; if (req->until > 0) timeout = usec_sub_unsigned(req->until, now(CLOCK_MONOTONIC)); @@ -993,45 +979,34 @@ int ask_password_agent( if (r == -EINTR) continue; if (r < 0) - goto finish; - if (r == 0) { - r = -ETIME; - goto finish; - } + return r; + if (r == 0) + return -ETIME; - if (pollfd[POLL_SIGNAL].revents & POLLIN) { - r = -EINTR; - goto finish; - } + if (pollfd[POLL_SIGNAL].revents & POLLIN) + return -EINTR; - if (req->hup_fd >= 0 && pollfd[hup_fd_idx].revents & POLLHUP) { - r = -ECONNRESET; - goto finish; - } + if (req->hup_fd >= 0 && pollfd[hup_fd_idx].revents & POLLHUP) + return -ECONNRESET; if (inotify_fd >= 0 && pollfd[inotify_idx].revents != 0) { (void) flush_fd(inotify_fd); if (req->keyring) { r = ask_password_keyring(req, flags, ret); - if (r >= 0) { - r = 0; - goto finish; - } else if (r != -ENOKEY) - goto finish; + if (r != -ENOKEY) + return r; } } if (pollfd[POLL_SOCKET].revents == 0) continue; + if (pollfd[POLL_SOCKET].revents != POLLIN) + return -EIO; - if (pollfd[POLL_SOCKET].revents != POLLIN) { - r = -EIO; - goto finish; - } - - iovec = IOVEC_MAKE(passphrase, sizeof(passphrase)); - + char passphrase[LINE_MAX+1]; + struct iovec iovec = IOVEC_MAKE(passphrase, sizeof(passphrase)); + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, @@ -1039,7 +1014,7 @@ int ask_password_agent( .msg_controllen = sizeof(control), }; - n = recvmsg_safe(socket_fd, &msghdr, 0); + ssize_t n = recvmsg_safe(socket_fd, &msghdr, 0); if (ERRNO_IS_NEG_TRANSIENT(n)) continue; if (n == -ECHRNG) { @@ -1050,76 +1025,54 @@ int ask_password_agent( log_debug_errno(n, "Got message with truncated payload data, ignoring."); continue; } - if (n < 0) { - r = (int) n; - goto finish; - } + if (n < 0) + return n; CLEANUP_ERASE(passphrase); cmsg_close_all(&msghdr); if (n == 0) { - log_debug("Message too short"); + log_debug("Got empty reply from ask-password agent, ignoring."); continue; } - ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); + struct ucred *ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); if (!ucred) { - log_debug("Received message without credentials. Ignoring."); + log_debug("Received message without credentials, ignoring."); continue; } - if (ucred->uid != getuid() && ucred->uid != 0) { - log_debug("Got response from bad user. Ignoring."); + log_debug("Got password response from bad user, ignoring."); continue; } + if (passphrase[0] == '-') + return -ECANCELED; + if (passphrase[0] == '+') { /* An empty message refers to the empty password */ if (n == 1) l = strv_new(""); else l = strv_parse_nulstr(passphrase+1, n-1); - if (!l) { - r = -ENOMEM; - goto finish; - } - - if (strv_isempty(l)) { - l = strv_free(l); - log_debug("Invalid packet"); - continue; - } + if (!l) + return -ENOMEM; - break; - } + if (!strv_isempty(l)) + break; - if (passphrase[0] == '-') { - r = -ECANCELED; - goto finish; + l = strv_free(l); } - log_debug("Invalid packet"); + log_debug("Got invalid response from ask-password agent, ignoring."); } if (req->keyring) (void) add_to_keyring_and_log(req->keyring, flags, l); *ret = TAKE_PTR(l); - r = 0; - -finish: - if (temp) { - assert(dfd >= 0); - (void) unlinkat(dfd, temp, 0); - } else if (final) { - assert(dfd >= 0); - (void) unlinkat(dfd, final, 0); - } - - assert_se(sigprocmask(SIG_SETMASK, &oldmask, NULL) == 0); - return r; + return 0; } static int ask_password_credential(const AskPasswordRequest *req, AskPasswordFlags flags, char ***ret) {