diff options
author | Geoff Greer <geoff@greer.fm> | 2018-04-12 17:38:24 -0700 |
---|---|---|
committer | Geoff Greer <geoff@greer.fm> | 2018-04-12 17:49:21 -0700 |
commit | ad6aa21c43bb87c917e21416f3ba448b634a98f8 (patch) | |
tree | 2e2eea866e21d7f3484ef8efde82b5a6a6a2bfb0 | |
parent | Merge branch 'master' of github.com:swaywm/sway (diff) | |
download | sway-ad6aa21c43bb87c917e21416f3ba448b634a98f8.tar.gz sway-ad6aa21c43bb87c917e21416f3ba448b634a98f8.tar.zst sway-ad6aa21c43bb87c917e21416f3ba448b634a98f8.zip |
swaylock: Securely zero-out password.
- Replace char* with static array. Any chars > 1024 will be discarded.
- mlock() password buffer so it can't be written to swap.
- Clear password buffer after auth succeeds or fails.
This is basically the same treatment I gave the 0.15 branch in https://github.com/swaywm/sway/pull/1519
-rw-r--r-- | include/swaylock/swaylock.h | 3 | ||||
-rw-r--r-- | swaylock/main.c | 13 | ||||
-rw-r--r-- | swaylock/password.c | 44 |
3 files changed, 34 insertions, 26 deletions
diff --git a/include/swaylock/swaylock.h b/include/swaylock/swaylock.h index 173e8b12..ed9fea19 100644 --- a/include/swaylock/swaylock.h +++ b/include/swaylock/swaylock.h | |||
@@ -24,9 +24,8 @@ struct swaylock_args { | |||
24 | }; | 24 | }; |
25 | 25 | ||
26 | struct swaylock_password { | 26 | struct swaylock_password { |
27 | size_t size; | ||
28 | size_t len; | 27 | size_t len; |
29 | char *buffer; | 28 | char buffer[1024]; |
30 | }; | 29 | }; |
31 | 30 | ||
32 | struct swaylock_state { | 31 | struct swaylock_state { |
diff --git a/swaylock/main.c b/swaylock/main.c index 4c6b44c6..200c1b5f 100644 --- a/swaylock/main.c +++ b/swaylock/main.c | |||
@@ -8,6 +8,7 @@ | |||
8 | #include <stdio.h> | 8 | #include <stdio.h> |
9 | #include <stdlib.h> | 9 | #include <stdlib.h> |
10 | #include <string.h> | 10 | #include <string.h> |
11 | #include <sys/mman.h> | ||
11 | #include <sys/stat.h> | 12 | #include <sys/stat.h> |
12 | #include <time.h> | 13 | #include <time.h> |
13 | #include <unistd.h> | 14 | #include <unistd.h> |
@@ -18,10 +19,15 @@ | |||
18 | #include "background-image.h" | 19 | #include "background-image.h" |
19 | #include "pool-buffer.h" | 20 | #include "pool-buffer.h" |
20 | #include "cairo.h" | 21 | #include "cairo.h" |
22 | #include "log.h" | ||
21 | #include "util.h" | 23 | #include "util.h" |
22 | #include "wlr-input-inhibitor-unstable-v1-client-protocol.h" | 24 | #include "wlr-input-inhibitor-unstable-v1-client-protocol.h" |
23 | #include "wlr-layer-shell-unstable-v1-client-protocol.h" | 25 | #include "wlr-layer-shell-unstable-v1-client-protocol.h" |
24 | 26 | ||
27 | void sway_terminate(int exit_code) { | ||
28 | exit(exit_code); | ||
29 | } | ||
30 | |||
25 | static void daemonize() { | 31 | static void daemonize() { |
26 | int fds[2]; | 32 | int fds[2]; |
27 | if (pipe(fds) != 0) { | 33 | if (pipe(fds) != 0) { |
@@ -236,6 +242,13 @@ int main(int argc, char **argv) { | |||
236 | } | 242 | } |
237 | } | 243 | } |
238 | 244 | ||
245 | #ifdef __linux__ | ||
246 | // Most non-linux platforms require root to mlock() | ||
247 | if (mlock(state.password.buffer, sizeof(state.password.buffer)) != 0) { | ||
248 | sway_abort("Unable to mlock() password memory."); | ||
249 | } | ||
250 | #endif | ||
251 | |||
239 | wl_list_init(&state.surfaces); | 252 | wl_list_init(&state.surfaces); |
240 | state.xkb.context = xkb_context_new(XKB_CONTEXT_NO_FLAGS); | 253 | state.xkb.context = xkb_context_new(XKB_CONTEXT_NO_FLAGS); |
241 | state.display = wl_display_connect(NULL); | 254 | state.display = wl_display_connect(NULL); |
diff --git a/swaylock/password.c b/swaylock/password.c index 1839f991..c8df3de8 100644 --- a/swaylock/password.c +++ b/swaylock/password.c | |||
@@ -1,7 +1,9 @@ | |||
1 | #define _XOPEN_SOURCE 500 | ||
1 | #include <assert.h> | 2 | #include <assert.h> |
2 | #include <pwd.h> | 3 | #include <pwd.h> |
3 | #include <security/pam_appl.h> | 4 | #include <security/pam_appl.h> |
4 | #include <stdlib.h> | 5 | #include <stdlib.h> |
6 | #include <string.h> | ||
5 | #include <unistd.h> | 7 | #include <unistd.h> |
6 | #include <wlr/util/log.h> | 8 | #include <wlr/util/log.h> |
7 | #include <xkbcommon/xkbcommon.h> | 9 | #include <xkbcommon/xkbcommon.h> |
@@ -20,7 +22,7 @@ static int function_conversation(int num_msg, const struct pam_message **msg, | |||
20 | switch (msg[i]->msg_style) { | 22 | switch (msg[i]->msg_style) { |
21 | case PAM_PROMPT_ECHO_OFF: | 23 | case PAM_PROMPT_ECHO_OFF: |
22 | case PAM_PROMPT_ECHO_ON: | 24 | case PAM_PROMPT_ECHO_ON: |
23 | pam_reply[i].resp = pw->buffer; | 25 | pam_reply[i].resp = strdup(pw->buffer); // PAM clears and frees this |
24 | break; | 26 | break; |
25 | case PAM_ERROR_MSG: | 27 | case PAM_ERROR_MSG: |
26 | case PAM_TEXT_INFO: | 28 | case PAM_TEXT_INFO: |
@@ -30,6 +32,16 @@ static int function_conversation(int num_msg, const struct pam_message **msg, | |||
30 | return PAM_SUCCESS; | 32 | return PAM_SUCCESS; |
31 | } | 33 | } |
32 | 34 | ||
35 | void clear_password_buffer(struct swaylock_password *pw) { | ||
36 | // Use volatile keyword so so compiler can't optimize this out. | ||
37 | volatile char *buffer = pw->buffer; | ||
38 | volatile char zero = '\0'; | ||
39 | for (size_t i = 0; i < sizeof(buffer); ++i) { | ||
40 | buffer[i] = zero; | ||
41 | } | ||
42 | pw->len = 0; | ||
43 | } | ||
44 | |||
33 | static bool attempt_password(struct swaylock_password *pw) { | 45 | static bool attempt_password(struct swaylock_password *pw) { |
34 | struct passwd *passwd = getpwuid(getuid()); | 46 | struct passwd *passwd = getpwuid(getuid()); |
35 | char *username = passwd->pw_name; | 47 | char *username = passwd->pw_name; |
@@ -38,6 +50,7 @@ static bool attempt_password(struct swaylock_password *pw) { | |||
38 | }; | 50 | }; |
39 | pam_handle_t *local_auth_handle = NULL; | 51 | pam_handle_t *local_auth_handle = NULL; |
40 | int pam_err; | 52 | int pam_err; |
53 | // TODO: only call pam_start once. keep the same handle the whole time | ||
41 | if ((pam_err = pam_start("swaylock", username, | 54 | if ((pam_err = pam_start("swaylock", username, |
42 | &local_conversation, &local_auth_handle)) != PAM_SUCCESS) { | 55 | &local_conversation, &local_auth_handle)) != PAM_SUCCESS) { |
43 | wlr_log(L_ERROR, "PAM returned error %d", pam_err); | 56 | wlr_log(L_ERROR, "PAM returned error %d", pam_err); |
@@ -46,18 +59,15 @@ static bool attempt_password(struct swaylock_password *pw) { | |||
46 | wlr_log(L_ERROR, "pam_authenticate failed"); | 59 | wlr_log(L_ERROR, "pam_authenticate failed"); |
47 | goto fail; | 60 | goto fail; |
48 | } | 61 | } |
62 | // TODO: only call pam_end once we succeed at authing. refresh tokens beforehand | ||
49 | if ((pam_err = pam_end(local_auth_handle, pam_err)) != PAM_SUCCESS) { | 63 | if ((pam_err = pam_end(local_auth_handle, pam_err)) != PAM_SUCCESS) { |
50 | wlr_log(L_ERROR, "pam_end failed"); | 64 | wlr_log(L_ERROR, "pam_end failed"); |
51 | goto fail; | 65 | goto fail; |
52 | } | 66 | } |
53 | // PAM frees this | 67 | clear_password_buffer(pw); |
54 | pw->buffer = NULL; | ||
55 | pw->len = pw->size = 0; | ||
56 | return true; | 68 | return true; |
57 | fail: | 69 | fail: |
58 | // PAM frees this | 70 | clear_password_buffer(pw); |
59 | pw->buffer = NULL; | ||
60 | pw->len = pw->size = 0; | ||
61 | return false; | 71 | return false; |
62 | } | 72 | } |
63 | 73 | ||
@@ -70,24 +80,10 @@ static bool backspace(struct swaylock_password *pw) { | |||
70 | } | 80 | } |
71 | 81 | ||
72 | static void append_ch(struct swaylock_password *pw, uint32_t codepoint) { | 82 | static void append_ch(struct swaylock_password *pw, uint32_t codepoint) { |
73 | if (!pw->buffer) { | ||
74 | pw->size = 8; | ||
75 | if (!(pw->buffer = malloc(pw->size))) { | ||
76 | // TODO: Display error | ||
77 | return; | ||
78 | } | ||
79 | pw->buffer[0] = 0; | ||
80 | } | ||
81 | size_t utf8_size = utf8_chsize(codepoint); | 83 | size_t utf8_size = utf8_chsize(codepoint); |
82 | if (pw->len + utf8_size + 1 >= pw->size) { | 84 | if (pw->len + utf8_size + 1 >= sizeof(pw->buffer)) { |
83 | size_t size = pw->size * 2; | 85 | // TODO: Display error |
84 | char *buffer = realloc(pw->buffer, size); | 86 | return; |
85 | if (!buffer) { | ||
86 | // TODO: Display error | ||
87 | return; | ||
88 | } | ||
89 | pw->size = size; | ||
90 | pw->buffer = buffer; | ||
91 | } | 87 | } |
92 | utf8_encode(&pw->buffer[pw->len], codepoint); | 88 | utf8_encode(&pw->buffer[pw->len], codepoint); |
93 | pw->buffer[pw->len + utf8_size] = 0; | 89 | pw->buffer[pw->len + utf8_size] = 0; |