From bdeb891378bfcb2dc79de41cfeb6a62807d09b20 Mon Sep 17 00:00:00 2001 From: Robin Ebert Date: Mon, 30 May 2022 11:50:30 +0200 Subject: [PATCH] Use mlock for password buffer --- comm.c | 4 +- include/password-buffer.h | 9 +++++ include/swaylock.h | 3 +- main.c | 10 ++--- meson.build | 1 + pam.c | 9 ++--- password-buffer.c | 81 +++++++++++++++++++++++++++++++++++++++ password.c | 4 +- shadow.c | 11 ++---- 9 files changed, 110 insertions(+), 22 deletions(-) create mode 100644 include/password-buffer.h create mode 100644 password-buffer.c diff --git a/comm.c b/comm.c index 7855a7f..6add38a 100644 --- a/comm.c +++ b/comm.c @@ -5,6 +5,7 @@ #include "comm.h" #include "log.h" #include "swaylock.h" +#include "password-buffer.h" static int comm[2][2] = {{-1, -1}, {-1, -1}}; @@ -19,9 +20,8 @@ ssize_t read_comm_request(char **buf_ptr) { return -1; } swaylock_log(LOG_DEBUG, "received pw check request"); - char *buf = malloc(size); + char *buf = password_buffer_create(size); if (!buf) { - swaylock_log_errno(LOG_ERROR, "failed to malloc pw buffer"); return -1; } size_t offs = 0; diff --git a/include/password-buffer.h b/include/password-buffer.h new file mode 100644 index 0000000..74f35aa --- /dev/null +++ b/include/password-buffer.h @@ -0,0 +1,9 @@ +#ifndef _SWAY_PASSWORD_BUFFER_H +#define _SWAY_PASSWORD_BUFFER_H + +#include + +char *password_buffer_create(size_t size); +void password_buffer_destroy(char *buffer, size_t size); + +#endif diff --git a/include/swaylock.h b/include/swaylock.h index da6e3e9..e49c716 100644 --- a/include/swaylock.h +++ b/include/swaylock.h @@ -67,7 +67,8 @@ struct swaylock_args { struct swaylock_password { size_t len; - char buffer[1024]; + size_t buffer_len; + char *buffer; }; struct swaylock_state { diff --git a/main.c b/main.c index f289b5e..a4279e0 100644 --- a/main.c +++ b/main.c @@ -20,6 +20,7 @@ #include "comm.h" #include "log.h" #include "loop.h" +#include "password-buffer.h" #include "pool-buffer.h" #include "seat.h" #include "swaylock.h" @@ -1187,13 +1188,12 @@ int main(int argc, char **argv) { state.args.colors.line = state.args.colors.ring; } -#ifdef __linux__ - // Most non-linux platforms require root to mlock() - if (mlock(state.password.buffer, sizeof(state.password.buffer)) != 0) { - swaylock_log(LOG_ERROR, "Unable to mlock() password memory."); + state.password.len = 0; + state.password.buffer_len = 1024; + state.password.buffer = password_buffer_create(state.password.buffer_len); + if (!state.password.buffer) { return EXIT_FAILURE; } -#endif wl_list_init(&state.surfaces); state.xkb.context = xkb_context_new(XKB_CONTEXT_NO_FLAGS); diff --git a/meson.build b/meson.build index b2e7944..caab139 100644 --- a/meson.build +++ b/meson.build @@ -127,6 +127,7 @@ sources = [ 'loop.c', 'main.c', 'password.c', + 'password-buffer.c', 'pool-buffer.c', 'render.c', 'seat.c', diff --git a/pam.c b/pam.c index 0adc9c6..0e48fcc 100644 --- a/pam.c +++ b/pam.c @@ -7,6 +7,7 @@ #include #include "comm.h" #include "log.h" +#include "password-buffer.h" #include "swaylock.h" static char *pw_buf = NULL; @@ -96,6 +97,9 @@ void run_pw_backend_child(void) { } int pam_status = pam_authenticate(auth_handle, 0); + password_buffer_destroy(pw_buf, size); + pw_buf = NULL; + bool success = pam_status == PAM_SUCCESS; if (!success) { swaylock_log(LOG_ERROR, "pam_authenticate failed: %s", @@ -103,13 +107,8 @@ void run_pw_backend_child(void) { } if (!write_comm_reply(success)) { - clear_buffer(pw_buf, size); exit(EXIT_FAILURE); } - - clear_buffer(pw_buf, size); - free(pw_buf); - pw_buf = NULL; } pam_setcred(auth_handle, PAM_REFRESH_CRED); diff --git a/password-buffer.c b/password-buffer.c new file mode 100644 index 0000000..b8b7f94 --- /dev/null +++ b/password-buffer.c @@ -0,0 +1,81 @@ +#define _POSIX_C_SOURCE 200809L +#include "password-buffer.h" +#include "log.h" +#include "swaylock.h" +#include +#include +#include +#include +#include + +static bool mlock_supported = true; +static long int page_size = 0; + +static long int get_page_size() { + if (!page_size) { + page_size = sysconf(_SC_PAGESIZE); + } + return page_size; +} + +// password_buffer_lock expects addr to be page alligned +static bool password_buffer_lock(char *addr, size_t size) { + int retries = 5; + while (mlock(addr, size) != 0 && retries > 0) { + switch (errno) { + case EAGAIN: + retries--; + if (retries == 0) { + swaylock_log(LOG_ERROR, "mlock() supported but failed too often."); + return false; + } + break; + case EPERM: + swaylock_log_errno(LOG_ERROR, "Unable to mlock() password memory: Unsupported!"); + mlock_supported = false; + return true; + default: + swaylock_log_errno(LOG_ERROR, "Unable to mlock() password memory."); + return false; + } + return false; + } + + return true; +} + +// password_buffer_unlock expects addr to be page alligned +static bool password_buffer_unlock(char *addr, size_t size) { + if (mlock_supported) { + if (munlock(addr, size) != 0) { + swaylock_log_errno(LOG_ERROR, "Unable to munlock() password memory."); + return false; + } + } + + return true; +} + +char *password_buffer_create(size_t size) { + void *buffer; + int result = posix_memalign(&buffer, get_page_size(), size); + if (result) { + //posix_memalign doesn't set errno according to the man page + errno = result; + swaylock_log_errno(LOG_ERROR, "failed to alloc password buffer"); + return NULL; + } + + if (!password_buffer_lock(buffer, size)) { + free(buffer); + return NULL; + } + + return buffer; +} + +void password_buffer_destroy(char *buffer, size_t size) { + clear_buffer(buffer, size); + password_buffer_unlock(buffer, size); + free(buffer); +} diff --git a/password.c b/password.c index 6880ccb..a3e7674 100644 --- a/password.c +++ b/password.c @@ -22,7 +22,7 @@ void clear_buffer(char *buf, size_t size) { } void clear_password_buffer(struct swaylock_password *pw) { - clear_buffer(pw->buffer, sizeof(pw->buffer)); + clear_buffer(pw->buffer, pw->buffer_len); pw->len = 0; } @@ -37,7 +37,7 @@ static bool backspace(struct swaylock_password *pw) { static void append_ch(struct swaylock_password *pw, uint32_t codepoint) { size_t utf8_size = utf8_chsize(codepoint); - if (pw->len + utf8_size + 1 >= sizeof(pw->buffer)) { + if (pw->len + utf8_size + 1 >= pw->buffer_len) { // TODO: Display error return; } diff --git a/shadow.c b/shadow.c index 0c474e6..c3e9286 100644 --- a/shadow.c +++ b/shadow.c @@ -11,6 +11,7 @@ #endif #include "comm.h" #include "log.h" +#include "password-buffer.h" #include "swaylock.h" void initialize_pw_backend(int argc, char **argv) { @@ -81,23 +82,19 @@ void run_pw_backend_child(void) { } char *c = crypt(buf, encpw); + password_buffer_destroy(buf, size); + buf = NULL; + if (c == NULL) { swaylock_log_errno(LOG_ERROR, "crypt failed"); - clear_buffer(buf, size); exit(EXIT_FAILURE); } bool success = strcmp(c, encpw) == 0; if (!write_comm_reply(success)) { - clear_buffer(buf, size); exit(EXIT_FAILURE); } - // We don't want to keep it in memory longer than necessary, - // so clear *before* sleeping. - clear_buffer(buf, size); - free(buf); - sleep(2); }