From 1602b008531ba6e0c704588cb2643daef26b71d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 14 Oct 2015 22:40:23 +0200 Subject: tree-wide: whenever we deal with passwords, erase them from memory after use A bit snake-oilish, but can't hurt. --- src/ask-password/ask-password.c | 2 + src/basic/util.c | 18 ++++ src/basic/util.h | 4 + src/cryptsetup/cryptsetup.c | 71 +++++++++------- src/firstboot/firstboot.c | 19 ++--- src/reply-password/reply-password.c | 19 ++--- src/shared/ask-password-api.c | 16 +++- .../tty-ask-password-agent.c | 97 +++++++++++++++------- 8 files changed, 162 insertions(+), 84 deletions(-) (limited to 'src') diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 1a69d15908..89a49c2e86 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -174,6 +174,8 @@ int main(int argc, char *argv[]) { break; } + strv_erase(l); + finish: free(arg_message); diff --git a/src/basic/util.c b/src/basic/util.c index 2565b0f547..f24db9796e 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -6805,3 +6805,21 @@ bool fdname_is_valid(const char *s) { bool oom_score_adjust_is_valid(int oa) { return oa >= OOM_SCORE_ADJ_MIN && oa <= OOM_SCORE_ADJ_MAX; } + +void string_erase(char *x) { + + if (!x) + return; + + /* A delicious drop of snake-oil! To be called on memory where + * we stored passphrases or so, after we used them. */ + + memory_erase(x, strlen(x)); +} + +void strv_erase(char **l) { + char **i; + + STRV_FOREACH(i, l) + string_erase(*i); +} diff --git a/src/basic/util.h b/src/basic/util.h index 6c63bc221f..b1c64675e0 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -943,3 +943,7 @@ int version(void); bool fdname_is_valid(const char *s); bool oom_score_adjust_is_valid(int oa); + +#define memory_erase(p, l) memset((p), 'x', (l)) +void string_erase(char *x); +void strv_erase(char **l); diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index cc03ad3ca8..c9be17446b 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -312,15 +312,16 @@ static char *disk_mount_point(const char *label) { return NULL; } -static int get_password(const char *vol, const char *src, usec_t until, bool accept_cached, char ***passwords) { +static int get_password(const char *vol, const char *src, usec_t until, bool accept_cached, char ***ret) { _cleanup_free_ char *description = NULL, *name_buffer = NULL, *mount_point = NULL, *maj_min = NULL, *text = NULL, *escaped_name = NULL; + _cleanup_strv_free_ char **passwords = NULL, **passwords2 = NULL; const char *name = NULL; char **p, *id; int r = 0; assert(vol); assert(src); - assert(passwords); + assert(ret); description = disk_description(src); mount_point = disk_mount_point(vol); @@ -360,57 +361,74 @@ static int get_password(const char *vol, const char *src, usec_t until, bool acc id = strjoina("cryptsetup:", escaped_name); - r = ask_password_auto(text, "drive-harddisk", id, "cryptsetup", until, ASK_PASSWORD_PUSH_CACHE|(accept_cached ? ASK_PASSWORD_ACCEPT_CACHED : 0), passwords); + r = ask_password_auto(text, "drive-harddisk", id, "cryptsetup", until, ASK_PASSWORD_PUSH_CACHE|(accept_cached ? ASK_PASSWORD_ACCEPT_CACHED : 0), &passwords); if (r < 0) return log_error_errno(r, "Failed to query password: %m"); if (arg_verify) { - _cleanup_strv_free_ char **passwords2 = NULL; + assert(strv_length(passwords) == 1); - assert(strv_length(*passwords) == 1); - - if (asprintf(&text, "Please enter passphrase for disk %s! (verification)", name) < 0) - return log_oom(); + if (asprintf(&text, "Please enter passphrase for disk %s! (verification)", name) < 0) { + r = log_oom(); + goto finish; + } id = strjoina("cryptsetup-verification:", escaped_name); r = ask_password_auto(text, "drive-harddisk", id, "cryptsetup", until, ASK_PASSWORD_PUSH_CACHE, &passwords2); - if (r < 0) - return log_error_errno(r, "Failed to query verification password: %m"); + if (r < 0) { + log_error_errno(r, "Failed to query verification password: %m"); + goto finish; + } assert(strv_length(passwords2) == 1); - if (!streq(*passwords[0], passwords2[0])) { + if (!streq(passwords[0], passwords2[0])) { log_warning("Passwords did not match, retrying."); - return -EAGAIN; + r = -EAGAIN; + goto finish; } } - strv_uniq(*passwords); + strv_uniq(passwords); - STRV_FOREACH(p, *passwords) { + STRV_FOREACH(p, passwords) { char *c; if (strlen(*p)+1 >= arg_key_size) continue; /* Pad password if necessary */ - if (!(c = new(char, arg_key_size))) - return log_oom(); + c = new(char, arg_key_size); + if (!c) { + r = -ENOMEM; + goto finish; + } strncpy(c, *p, arg_key_size); free(*p); *p = c; } - return 0; + *ret = passwords; + passwords = NULL; + + r = 0; + +finish: + strv_erase(passwords); + strv_erase(passwords2); + + return r; } -static int attach_tcrypt(struct crypt_device *cd, - const char *name, - const char *key_file, - char **passwords, - uint32_t flags) { +static int attach_tcrypt( + struct crypt_device *cd, + const char *name, + const char *key_file, + char **passwords, + uint32_t flags) { + int r = 0; _cleanup_free_ char *passphrase = NULL; struct crypt_params_tcrypt params = { @@ -520,8 +538,7 @@ static int attach_luks_or_plain(struct crypt_device *cd, * it just configures encryption * parameters when used for plain * mode. */ - r = crypt_format(cd, CRYPT_PLAIN, cipher, cipher_mode, - NULL, NULL, arg_keyfile_size, ¶ms); + r = crypt_format(cd, CRYPT_PLAIN, cipher, cipher_mode, NULL, NULL, arg_keyfile_size, ¶ms); /* hash == NULL implies the user passed "plain" */ pass_volume_key = (params.hash == NULL); @@ -537,9 +554,7 @@ static int attach_luks_or_plain(struct crypt_device *cd, crypt_get_device_name(cd)); if (key_file) { - r = crypt_activate_by_keyfile_offset(cd, name, arg_key_slot, - key_file, arg_keyfile_size, - arg_keyfile_offset, flags); + r = crypt_activate_by_keyfile_offset(cd, name, arg_key_slot, key_file, arg_keyfile_size, arg_keyfile_offset, flags); if (r < 0) { log_error_errno(r, "Failed to activate with key file '%s': %m", key_file); return -EAGAIN; @@ -631,7 +646,6 @@ int main(int argc, char *argv[]) { k = crypt_init(&cd, arg_header); } else k = crypt_init(&cd, argv[3]); - if (k) { log_error_errno(k, "crypt_init() failed: %m"); goto finish; @@ -688,6 +702,7 @@ int main(int argc, char *argv[]) { arg_header ? argv[3] : NULL, passwords, flags); + strv_erase(passwords); if (k >= 0) break; else if (k == -EAGAIN) { diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 1562ccf0d7..da247fbef8 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -51,15 +51,6 @@ static bool arg_copy_locale = false; static bool arg_copy_timezone = false; static bool arg_copy_root_password = false; -static void clear_string(char *x) { - - if (!x) - return; - - /* A delicious drop of snake-oil! */ - memset(x, 'x', strlen(x)); -} - static bool press_any_key(void) { char k = 0; bool need_nl = true; @@ -477,18 +468,18 @@ static int prompt_root_password(void) { r = ask_password_tty(msg2, NULL, 0, 0, NULL, &b); if (r < 0) { - clear_string(a); + string_erase(a); return log_error_errno(r, "Failed to query root password: %m"); } if (!streq(a, b)) { log_error("Entered passwords did not match, please try again."); - clear_string(a); - clear_string(b); + string_erase(a); + string_erase(b); continue; } - clear_string(b); + string_erase(b); arg_root_password = a; a = NULL; break; @@ -881,7 +872,7 @@ finish: free(arg_locale_messages); free(arg_timezone); free(arg_hostname); - clear_string(arg_root_password); + string_erase(arg_root_password); free(arg_root_password); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/src/reply-password/reply-password.c b/src/reply-password/reply-password.c index d0d61b98ed..534cf729b9 100644 --- a/src/reply-password/reply-password.c +++ b/src/reply-password/reply-password.c @@ -50,9 +50,10 @@ static int send_on_socket(int fd, const char *socket_name, const void *packet, s } int main(int argc, char *argv[]) { - int fd = -1, r = EXIT_FAILURE; + _cleanup_close_ int fd = -1; char packet[LINE_MAX]; size_t length; + int r; log_set_target(LOG_TARGET_AUTO); log_parse_environment(); @@ -60,14 +61,14 @@ int main(int argc, char *argv[]) { if (argc != 3) { log_error("Wrong number of arguments."); - goto finish; + return EXIT_FAILURE; } if (streq(argv[1], "1")) { packet[0] = '+'; if (!fgets(packet+1, sizeof(packet)-1, stdin)) { - log_error_errno(errno, "Failed to read password: %m"); + r = log_error_errno(errno, "Failed to read password: %m"); goto finish; } @@ -78,22 +79,20 @@ int main(int argc, char *argv[]) { length = 1; } else { log_error("Invalid first argument %s", argv[1]); + r = -EINVAL; goto finish; } fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); if (fd < 0) { - log_error_errno(errno, "socket() failed: %m"); + r = log_error_errno(errno, "socket() failed: %m"); goto finish; } - if (send_on_socket(fd, argv[2], packet, length) < 0) - goto finish; - - r = EXIT_SUCCESS; + r = send_on_socket(fd, argv[2], packet, length); finish: - safe_close(fd); + memory_erase(packet, sizeof(packet)); - return r; + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index f8cf11b297..e35594a5df 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -78,6 +78,7 @@ static int retrieve_key(key_serial_t serial, char ***ret) { if (n < m) break; + memory_erase(p, n); free(p); m *= 2; } @@ -86,6 +87,8 @@ static int retrieve_key(key_serial_t serial, char ***ret) { if (!l) return -ENOMEM; + memory_erase(p, n); + *ret = l; return 0; } @@ -116,6 +119,7 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa return r; r = strv_make_nulstr(l, &p, &n); + strv_erase(l); if (r < 0) return r; @@ -124,6 +128,7 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa assert(p[n-1] == 0); serial = add_key("user", keyname, p, n-1, KEY_SPEC_USER_KEYRING); + memory_erase(p, n); if (serial == -1) return -errno; @@ -361,9 +366,12 @@ int ask_password_tty( dirty = true; } + + c = 'x'; } x = strndup(passphrase, p); + memory_erase(passphrase, p); if (!x) { r = -ENOMEM; goto finish; @@ -620,6 +628,7 @@ int ask_password_agent( l = strv_new("", NULL); else l = strv_parse_nulstr(passphrase+1, n-1); + memory_erase(passphrase, n); if (!l) { r = -ENOMEM; goto finish; @@ -688,9 +697,12 @@ int ask_password_auto( if (r < 0) return r; - r = strv_consume(&l, s); - if (r < 0) + r = strv_push(&l, s); + if (r < 0) { + string_erase(s); + free(s); return -ENOMEM; + } *ret = l; return 0; diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 53986babae..7a5ac9fa9c 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -120,23 +120,30 @@ static int ask_password_plymouth( y = now(CLOCK_MONOTONIC); - if (y > until) - return -ETIME; + if (y > until) { + r = -ETIME; + goto finish; + } sleep_for = (int) ((until - y) / USEC_PER_MSEC); } - if (flag_file && access(flag_file, F_OK) < 0) - return -errno; + if (flag_file && access(flag_file, F_OK) < 0) { + r = -errno; + goto finish; + } j = poll(pollfd, notify >= 0 ? 2 : 1, sleep_for); if (j < 0) { if (errno == EINTR) continue; - return -errno; - } else if (j == 0) - return -ETIME; + r = -errno; + goto finish; + } else if (j == 0) { + r = -ETIME; + goto finish; + } if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) flush_fd(notify); @@ -149,9 +156,12 @@ static int ask_password_plymouth( if (errno == EINTR || errno == EAGAIN) continue; - return -errno; - } else if (k == 0) - return -EIO; + r = -errno; + goto finish; + } else if (k == 0) { + r = -EIO; + goto finish; + } p += k; @@ -166,12 +176,14 @@ static int ask_password_plymouth( * with a normal password request */ packet = mfree(packet); - if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0) - return -ENOMEM; + if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0) { + r = -ENOMEM; + goto finish; + } r = loop_write(fd, packet, n+1, true); if (r < 0) - return r; + goto finish; flags &= ~ASK_PASSWORD_ACCEPT_CACHED; p = 0; @@ -179,7 +191,8 @@ static int ask_password_plymouth( } /* No password, because UI not shown */ - return -ENOENT; + r = -ENOENT; + goto finish; } else if (buffer[0] == 2 || buffer[0] == 9) { uint32_t size; @@ -191,32 +204,43 @@ static int ask_password_plymouth( memcpy(&size, buffer+1, sizeof(size)); size = le32toh(size); - if (size + 5 > sizeof(buffer)) - return -EIO; + if (size + 5 > sizeof(buffer)) { + r = -EIO; + goto finish; + } if (p-5 < size) continue; l = strv_parse_nulstr(buffer + 5, size); - if (!l) - return -ENOMEM; + if (!l) { + r = -ENOMEM; + goto finish; + } *ret = l; break; - } else + } else { /* Unknown packet */ - return -EIO; + r = -EIO; + goto finish; + } } - return 0; + r = 0; + +finish: + memory_erase(buffer, sizeof(buffer)); + return r; } static int parse_password(const char *filename, char **wall) { _cleanup_free_ char *socket_name = NULL, *message = NULL, *packet = NULL; + bool accept_cached = false, echo = false; + size_t packet_length = 0; uint64_t not_after = 0; unsigned pid = 0; - bool accept_cached = false, echo = false; const ConfigTableItem items[] = { { "Ask", "Socket", config_parse_string, 0, &socket_name }, @@ -270,7 +294,6 @@ static int parse_password(const char *filename, char **wall) { } else { union sockaddr_union sa = {}; - size_t packet_length = 0; _cleanup_close_ int socket_fd = -1; assert(arg_action == ACTION_QUERY || @@ -307,6 +330,8 @@ static int parse_password(const char *filename, char **wall) { } } + strv_erase(passwords); + } else { _cleanup_free_ char *password = NULL; int tty_fd = -1; @@ -338,28 +363,40 @@ static int parse_password(const char *filename, char **wall) { strcpy(packet + 1, password); } } + + string_erase(password); } - if (IN_SET(r, -ETIME, -ENOENT)) + if (IN_SET(r, -ETIME, -ENOENT)) { /* If the query went away, that's OK */ - return 0; - - if (r < 0) - return log_error_errno(r, "Failed to query password: %m"); + r = 0; + goto finish; + } + if (r < 0) { + log_error_errno(r, "Failed to query password: %m"); + goto finish; + } socket_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0); - if (socket_fd < 0) - return log_error_errno(errno, "socket(): %m"); + if (socket_fd < 0) { + r = log_error_errno(errno, "socket(): %m"); + goto finish; + } sa.un.sun_family = AF_UNIX; strncpy(sa.un.sun_path, socket_name, sizeof(sa.un.sun_path)); r = sendto(socket_fd, packet, packet_length, MSG_NOSIGNAL, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(socket_name)); + memory_erase(packet, packet_length); if (r < 0) return log_error_errno(errno, "Failed to send: %m"); } return 0; + +finish: + memory_erase(packet, packet_length); + return r; } static int wall_tty_block(void) { -- cgit v1.2.3-54-g00ecf