From c1de1178254c786d9cd4fe2b36f72e6e26778f57 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse Date: Sat, 15 Apr 2017 09:39:13 +0200 Subject: [PATCH 1/3] Rename all PAM-independant variables/comments. there is nothing PAM-specific about pam_state or pam_state_t. therefore rename them to be authenticator independant. --- i3lock.c | 34 +++++++++++++++++----------------- unlock_indicator.c | 34 +++++++++++++++++----------------- unlock_indicator.h | 10 +++++----- xcb.c | 4 ++-- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/i3lock.c b/i3lock.c index 761ed32..2110fef 100644 --- a/i3lock.c +++ b/i3lock.c @@ -59,11 +59,11 @@ bool unlock_indicator = true; char *modifier_string = NULL; static bool dont_fork = false; struct ev_loop *main_loop; -static struct ev_timer *clear_pam_wrong_timeout; +static struct ev_timer *clear_auth_wrong_timeout; static struct ev_timer *clear_indicator_timeout; static struct ev_timer *discard_passwd_timeout; extern unlock_state_t unlock_state; -extern pam_state_t pam_state; +extern auth_state_t auth_state; int failed_attempts = 0; bool show_failed_attempts = false; bool retry_verification = false; @@ -206,13 +206,13 @@ static void finish_input(void) { } /* - * Resets pam_state to STATE_PAM_IDLE 2 seconds after an unsuccessful + * Resets auth_state to STATE_AUTH_IDLE 2 seconds after an unsuccessful * authentication event. * */ -static void clear_pam_wrong(EV_P_ ev_timer *w, int revents) { - DEBUG("clearing pam wrong\n"); - pam_state = STATE_PAM_IDLE; +static void clear_auth_wrong(EV_P_ ev_timer *w, int revents) { + DEBUG("clearing auth wrong\n"); + auth_state = STATE_AUTH_IDLE; redraw_screen(); /* Clear modifier string. */ @@ -222,9 +222,9 @@ static void clear_pam_wrong(EV_P_ ev_timer *w, int revents) { } /* Now free this timeout. */ - STOP_TIMER(clear_pam_wrong_timeout); + STOP_TIMER(clear_auth_wrong_timeout); - /* retry with input done during pam verification */ + /* retry with input done during auth verification */ if (retry_verification) { retry_verification = false; finish_input(); @@ -248,8 +248,8 @@ static void discard_passwd_cb(EV_P_ ev_timer *w, int revents) { } static void input_done(void) { - STOP_TIMER(clear_pam_wrong_timeout); - pam_state = STATE_PAM_VERIFY; + STOP_TIMER(clear_auth_wrong_timeout); + auth_state = STATE_AUTH_VERIFY; unlock_state = STATE_STARTED; redraw_screen(); @@ -271,7 +271,7 @@ static void input_done(void) { fprintf(stderr, "Authentication failure\n"); /* Get state of Caps and Num lock modifiers, to be displayed in - * STATE_PAM_WRONG state */ + * STATE_AUTH_WRONG state */ xkb_mod_index_t idx, num_mods; const char *mod_name; @@ -305,7 +305,7 @@ static void input_done(void) { } } - pam_state = STATE_PAM_WRONG; + auth_state = STATE_AUTH_WRONG; failed_attempts += 1; clear_input(); if (unlock_indicator) @@ -314,7 +314,7 @@ static void input_done(void) { /* Clear this state after 2 seconds (unless the user enters another * password during that time). */ ev_now_update(main_loop); - START_TIMER(clear_pam_wrong_timeout, TSTAMP_N_SECS(2), clear_pam_wrong); + START_TIMER(clear_auth_wrong_timeout, TSTAMP_N_SECS(2), clear_auth_wrong); /* Cancel the clear_indicator_timeout, it would hide the unlock indicator * too early. */ @@ -393,7 +393,7 @@ static void handle_key_press(xcb_key_press_event_t *event) { if ((ksym == XKB_KEY_j || ksym == XKB_KEY_m) && !ctrl) break; - if (pam_state == STATE_PAM_WRONG) { + if (auth_state == STATE_AUTH_WRONG) { retry_verification = true; return; } @@ -725,7 +725,7 @@ static void xcb_check_cb(EV_P_ ev_check *w, int revents) { /* * This function is called from a fork()ed child and will raise the i3lock * window when the window is obscured, even when the main i3lock process is - * blocked due to PAM. + * blocked due to the authentication backend. * */ static void raise_loop(xcb_window_t window) { @@ -985,7 +985,7 @@ int main(int argc, char *argv[]) { cursor = create_cursor(conn, screen, win, curs_choice); /* Display the "locking…" message while trying to grab the pointer/keyboard. */ - pam_state = STATE_PAM_LOCK; + auth_state = STATE_AUTH_LOCK; grab_pointer_and_keyboard(conn, screen, cursor); pid_t pid = fork(); @@ -1012,7 +1012,7 @@ int main(int argc, char *argv[]) { errx(EXIT_FAILURE, "Could not initialize libev. Bad LIBEV_FLAGS?\n"); /* Explicitly call the screen redraw in case "locking…" message was displayed */ - pam_state = STATE_PAM_IDLE; + auth_state = STATE_AUTH_IDLE; redraw_screen(); struct ev_io *xcb_watcher = calloc(sizeof(struct ev_io), 1); diff --git a/unlock_indicator.c b/unlock_indicator.c index ddad957..4c7d0e9 100644 --- a/unlock_indicator.c +++ b/unlock_indicator.c @@ -78,7 +78,7 @@ static xcb_visualtype_t *vistype; /* Maintain the current unlock/PAM state to draw the appropriate unlock * indicator. */ unlock_state_t unlock_state; -pam_state_t pam_state; +auth_state_t auth_state; /* * Returns the scaling factor of the current screen. E.g., on a 227 DPI MacBook @@ -141,7 +141,7 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { } if (unlock_indicator && - (unlock_state >= STATE_KEY_PRESSED || pam_state > STATE_PAM_IDLE)) { + (unlock_state >= STATE_KEY_PRESSED || auth_state > STATE_AUTH_IDLE)) { cairo_scale(ctx, scaling_factor(), scaling_factor()); /* Draw a (centered) circle with transparent background. */ cairo_set_line_width(ctx, 10.0); @@ -154,12 +154,12 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { /* Use the appropriate color for the different PAM states * (currently verifying, wrong password, or default) */ - switch (pam_state) { - case STATE_PAM_VERIFY: - case STATE_PAM_LOCK: + switch (auth_state) { + case STATE_AUTH_VERIFY: + case STATE_AUTH_LOCK: cairo_set_source_rgba(ctx, 0, 114.0 / 255, 255.0 / 255, 0.75); break; - case STATE_PAM_WRONG: + case STATE_AUTH_WRONG: case STATE_I3LOCK_LOCK_FAILED: cairo_set_source_rgba(ctx, 250.0 / 255, 0, 0, 0.75); break; @@ -169,16 +169,16 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { } cairo_fill_preserve(ctx); - switch (pam_state) { - case STATE_PAM_VERIFY: - case STATE_PAM_LOCK: + switch (auth_state) { + case STATE_AUTH_VERIFY: + case STATE_AUTH_LOCK: cairo_set_source_rgb(ctx, 51.0 / 255, 0, 250.0 / 255); break; - case STATE_PAM_WRONG: + case STATE_AUTH_WRONG: case STATE_I3LOCK_LOCK_FAILED: cairo_set_source_rgb(ctx, 125.0 / 255, 51.0 / 255, 0); break; - case STATE_PAM_IDLE: + case STATE_AUTH_IDLE: cairo_set_source_rgb(ctx, 51.0 / 255, 125.0 / 255, 0); break; } @@ -205,14 +205,14 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { cairo_set_source_rgb(ctx, 0, 0, 0); cairo_select_font_face(ctx, "sans-serif", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_NORMAL); cairo_set_font_size(ctx, 28.0); - switch (pam_state) { - case STATE_PAM_VERIFY: + switch (auth_state) { + case STATE_AUTH_VERIFY: text = "verifying…"; break; - case STATE_PAM_LOCK: + case STATE_AUTH_LOCK: text = "locking…"; break; - case STATE_PAM_WRONG: + case STATE_AUTH_WRONG: text = "wrong!"; break; case STATE_I3LOCK_LOCK_FAILED: @@ -245,7 +245,7 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { cairo_close_path(ctx); } - if (pam_state == STATE_PAM_WRONG && (modifier_string != NULL)) { + if (auth_state == STATE_AUTH_WRONG && (modifier_string != NULL)) { cairo_text_extents_t extents; double x, y; @@ -334,7 +334,7 @@ xcb_pixmap_t draw_image(uint32_t *resolution) { * */ void redraw_screen(void) { - DEBUG("redraw_screen(unlock_state = %d, pam_state = %d)\n", unlock_state, pam_state); + DEBUG("redraw_screen(unlock_state = %d, auth_state = %d)\n", unlock_state, auth_state); xcb_pixmap_t bg_pixmap = draw_image(last_resolution); xcb_change_window_attributes(conn, win, XCB_CW_BACK_PIXMAP, (uint32_t[1]){bg_pixmap}); /* XXX: Possible optimization: Only update the area in the middle of the diff --git a/unlock_indicator.h b/unlock_indicator.h index acfe768..e4a8d8e 100644 --- a/unlock_indicator.h +++ b/unlock_indicator.h @@ -11,12 +11,12 @@ typedef enum { } unlock_state_t; typedef enum { - STATE_PAM_IDLE = 0, /* no PAM interaction at the moment */ - STATE_PAM_VERIFY = 1, /* currently verifying the password via PAM */ - STATE_PAM_LOCK = 2, /* currently locking the screen */ - STATE_PAM_WRONG = 3, /* the password was wrong */ + STATE_AUTH_IDLE = 0, /* no authenticator interaction at the moment */ + STATE_AUTH_VERIFY = 1, /* currently verifying the password via authenticator */ + STATE_AUTH_LOCK = 2, /* currently locking the screen */ + STATE_AUTH_WRONG = 3, /* the password was wrong */ STATE_I3LOCK_LOCK_FAILED = 4 /* i3lock failed to load */ -} pam_state_t; +} auth_state_t; xcb_pixmap_t draw_image(uint32_t* resolution); void redraw_screen(void); diff --git a/xcb.c b/xcb.c index 73ebb7e..078ddb9 100644 --- a/xcb.c +++ b/xcb.c @@ -24,7 +24,7 @@ #include "cursors.h" #include "unlock_indicator.h" -extern pam_state_t pam_state; +extern auth_state_t auth_state; xcb_connection_t *conn; xcb_screen_t *screen; @@ -262,7 +262,7 @@ void grab_pointer_and_keyboard(xcb_connection_t *conn, xcb_screen_t *screen, xcb /* After trying for 10000 times, i3lock will display an error message * for 2 sec prior to terminate. */ if (tries <= 0) { - pam_state = STATE_I3LOCK_LOCK_FAILED; + auth_state = STATE_I3LOCK_LOCK_FAILED; redraw_screen(); sleep(1); errx(EXIT_FAILURE, "Cannot grab pointer/keyboard"); From 15973d1f5225ee4fc0114d70a24a695486f482d4 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse Date: Sat, 15 Apr 2017 09:45:51 +0200 Subject: [PATCH 2/3] Move all PAM code behind UES_PAM and enable that by default. --- Makefile | 1 + i3lock.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/Makefile b/Makefile index b3d4dc2..b7eae33 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,7 @@ CFLAGS += -std=c99 CFLAGS += -pipe CFLAGS += -Wall CPPFLAGS += -D_GNU_SOURCE +CPPFLAGS += -DUSE_PAM CFLAGS += $(shell $(PKG_CONFIG) --cflags cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11) LIBS += $(shell $(PKG_CONFIG) --libs cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11) LIBS += -lpam diff --git a/i3lock.c b/i3lock.c index 2110fef..26c5b9b 100644 --- a/i3lock.c +++ b/i3lock.c @@ -18,7 +18,9 @@ #include #include #include +#ifdef USE_PAM #include +#endif #include #include #include @@ -49,7 +51,9 @@ char color[7] = "ffffff"; uint32_t last_resolution[2]; xcb_window_t win; static xcb_cursor_t cursor; +#ifdef USE_PAM static pam_handle_t *pam_handle; +#endif int input_position = 0; /* Holds the password you enter (in UTF-8). */ static char password[512]; @@ -253,6 +257,7 @@ static void input_done(void) { unlock_state = STATE_STARTED; redraw_screen(); +#ifdef USE_PAM if (pam_authenticate(pam_handle, 0) == PAM_SUCCESS) { DEBUG("successfully authenticated\n"); clear_password_memory(); @@ -266,6 +271,7 @@ static void input_done(void) { exit(0); } +#endif if (debug_mode) fprintf(stderr, "Authentication failure\n"); @@ -597,6 +603,7 @@ void handle_screen_resize(void) { redraw_screen(); } +#ifdef USE_PAM /* * Callback function for PAM. We only react on password request callbacks. * @@ -627,6 +634,7 @@ static int conv_callback(int num_msg, const struct pam_message **msg, return 0; } +#endif /* * This callback is only a dummy, see xcb_prepare_cb and xcb_check_cb. @@ -782,8 +790,10 @@ int main(int argc, char *argv[]) { struct passwd *pw; char *username; char *image_path = NULL; +#ifdef USE_PAM int ret; struct pam_conv conv = {conv_callback, NULL}; +#endif int curs_choice = CURS_NONE; int o; int optind = 0; @@ -877,12 +887,14 @@ int main(int argc, char *argv[]) { * the unlock indicator upon keypresses. */ srand(time(NULL)); +#ifdef USE_PAM /* Initialize PAM */ if ((ret = pam_start("i3lock", username, &conv, &pam_handle)) != PAM_SUCCESS) errx(EXIT_FAILURE, "PAM: %s", pam_strerror(pam_handle, ret)); if ((ret = pam_set_item(pam_handle, PAM_TTY, getenv("DISPLAY"))) != PAM_SUCCESS) errx(EXIT_FAILURE, "PAM: %s", pam_strerror(pam_handle, ret)); +#endif /* Using mlock() as non-super-user seems only possible in Linux. Users of other * operating systems should use encrypted swap/no swap (or remove the ifdef and From 68fc2e8b5f8bcd203598e6da2a98d5aa7359af1e Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse Date: Sat, 15 Apr 2017 14:41:32 +0200 Subject: [PATCH 3/3] Use bsd_auth(3) instead of PAM on OpenBSD Also apply two security measures for OpenBSD: - use explicit_bzero(3) - mlock(2) works for non-root users too --- Makefile | 8 ++++++-- README.md | 4 ++++ i3lock.c | 45 +++++++++++++++++++++++++++++++++++---------- unlock_indicator.h | 8 ++++---- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index b7eae33..3acb272 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ TOPDIR=$(shell pwd) +UNAME=$(shell uname) INSTALL=install PREFIX=/usr @@ -14,13 +15,16 @@ CFLAGS += -std=c99 CFLAGS += -pipe CFLAGS += -Wall CPPFLAGS += -D_GNU_SOURCE -CPPFLAGS += -DUSE_PAM CFLAGS += $(shell $(PKG_CONFIG) --cflags cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11) LIBS += $(shell $(PKG_CONFIG) --libs cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11) -LIBS += -lpam LIBS += -lev LIBS += -lm +# OpenBSD lacks PAM, use bsd_auth(3) instead. +ifneq ($(UNAME),OpenBSD) + LIBS += -lpam +endif + FILES:=$(wildcard *.c) FILES:=$(FILES:.c=.o) diff --git a/README.md b/README.md index fcecbfa..fc486b2 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ Many little improvements have been made to i3lock over time: - You can specify whether i3lock should bell upon a wrong password. - i3lock uses PAM and therefore is compatible with LDAP etc. + On OpenBSD i3lock uses the bsd_auth(3) framework. Requirements ------------ @@ -37,6 +38,9 @@ Running i3lock Simply invoke the 'i3lock' command. To get out of it, enter your password and press enter. +On OpenBSD the `i3lock` binary needs to be setgid `auth` to call the +authentication helpers, e.g. `/usr/libexec/auth/login_passwd`. + Upstream -------- Please submit pull requests to https://github.com/i3/i3lock diff --git a/i3lock.c b/i3lock.c index 26c5b9b..01f0436 100644 --- a/i3lock.c +++ b/i3lock.c @@ -18,7 +18,9 @@ #include #include #include -#ifdef USE_PAM +#ifdef __OpenBSD__ +#include +#else #include #endif #include @@ -30,6 +32,9 @@ #include #include #include +#ifdef __OpenBSD__ +#include /* explicit_bzero(3) */ +#endif #include "i3lock.h" #include "xcb.h" @@ -51,7 +56,7 @@ char color[7] = "ffffff"; uint32_t last_resolution[2]; xcb_window_t win; static xcb_cursor_t cursor; -#ifdef USE_PAM +#ifndef __OpenBSD__ static pam_handle_t *pam_handle; #endif int input_position = 0; @@ -162,6 +167,11 @@ static bool load_compose_table(const char *locale) { * */ static void clear_password_memory(void) { +#ifdef __OpenBSD__ + /* Use explicit_bzero(3) which was explicitly designed not to be + * optimized out by the compiler. */ + explicit_bzero(password, strlen(password)); +#else /* A volatile pointer to the password buffer to prevent the compiler from * optimizing this out. */ volatile char *vpassword = password; @@ -171,6 +181,7 @@ static void clear_password_memory(void) { * compiler from optimizing the calls away, since the value of 'beep' * is not known at compile-time. */ vpassword[c] = c + (int)beep; +#endif } ev_timer *start_timer(ev_timer *timer_obj, ev_tstamp timeout, ev_callback_t callback) { @@ -257,7 +268,19 @@ static void input_done(void) { unlock_state = STATE_STARTED; redraw_screen(); -#ifdef USE_PAM +#ifdef __OpenBSD__ + struct passwd *pw; + + if (!(pw = getpwuid(getuid()))) + errx(1, "unknown uid %u.", getuid()); + + if (auth_userokay(pw->pw_name, NULL, NULL, password) != 0) { + DEBUG("successfully authenticated\n"); + clear_password_memory(); + + exit(0); + } +#else if (pam_authenticate(pam_handle, 0) == PAM_SUCCESS) { DEBUG("successfully authenticated\n"); clear_password_memory(); @@ -603,7 +626,7 @@ void handle_screen_resize(void) { redraw_screen(); } -#ifdef USE_PAM +#ifndef __OpenBSD__ /* * Callback function for PAM. We only react on password request callbacks. * @@ -790,7 +813,7 @@ int main(int argc, char *argv[]) { struct passwd *pw; char *username; char *image_path = NULL; -#ifdef USE_PAM +#ifndef __OpenBSD__ int ret; struct pam_conv conv = {conv_callback, NULL}; #endif @@ -887,7 +910,7 @@ int main(int argc, char *argv[]) { * the unlock indicator upon keypresses. */ srand(time(NULL)); -#ifdef USE_PAM +#ifndef __OpenBSD__ /* Initialize PAM */ if ((ret = pam_start("i3lock", username, &conv, &pam_handle)) != PAM_SUCCESS) errx(EXIT_FAILURE, "PAM: %s", pam_strerror(pam_handle, ret)); @@ -896,10 +919,12 @@ int main(int argc, char *argv[]) { errx(EXIT_FAILURE, "PAM: %s", pam_strerror(pam_handle, ret)); #endif -/* Using mlock() as non-super-user seems only possible in Linux. Users of other - * operating systems should use encrypted swap/no swap (or remove the ifdef and - * run i3lock as super-user). */ -#if defined(__linux__) +/* Using mlock() as non-super-user seems only possible in Linux and OpenBSD. + * Users of other operating systems should use encrypted swap/no swap + * (or remove the ifdef and run i3lock as super-user). + * NB: Alas, swap is encrypted by default on OpenBSD so swapping out + * is not necessarily an issue. */ +#if defined(__linux__) || defined(__OpenBSD__) /* Lock the area where we store the password in memory, we don’t want it to * be swapped to disk. Since Linux 2.6.9, this does not require any * privileges, just enough bytes in the RLIMIT_MEMLOCK limit. */ diff --git a/unlock_indicator.h b/unlock_indicator.h index e4a8d8e..2321620 100644 --- a/unlock_indicator.h +++ b/unlock_indicator.h @@ -11,10 +11,10 @@ typedef enum { } unlock_state_t; typedef enum { - STATE_AUTH_IDLE = 0, /* no authenticator interaction at the moment */ - STATE_AUTH_VERIFY = 1, /* currently verifying the password via authenticator */ - STATE_AUTH_LOCK = 2, /* currently locking the screen */ - STATE_AUTH_WRONG = 3, /* the password was wrong */ + STATE_AUTH_IDLE = 0, /* no authenticator interaction at the moment */ + STATE_AUTH_VERIFY = 1, /* currently verifying the password via authenticator */ + STATE_AUTH_LOCK = 2, /* currently locking the screen */ + STATE_AUTH_WRONG = 3, /* the password was wrong */ STATE_I3LOCK_LOCK_FAILED = 4 /* i3lock failed to load */ } auth_state_t;