Merge pull request #19 from swaywm/async

Async password verification

comm.c

@@ -0,0 +1,109 @@
+#include <stdbool.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include "comm.h"
+#include "log.h"
+#include "swaylock.h"
+
+static int comm[2][2] = {{-1, -1}, {-1, -1}};
+
+ssize_t read_comm_request(char **buf_ptr) {
+	size_t size;
+	ssize_t amt;
+	amt = read(comm[0][0], &size, sizeof(size));
+	if (amt == 0) {
+		return 0;
+	} else if (amt < 0) {
+		swaylock_log_errno(LOG_ERROR, "read pw request");
+		return -1;
+	}
+	swaylock_log(LOG_DEBUG, "received pw check request");
+	char *buf = malloc(size);
+	if (!buf) {
+		swaylock_log_errno(LOG_ERROR, "failed to malloc pw buffer");
+		return -1;
+	}
+	size_t offs = 0;
+	do {
+		amt = read(comm[0][0], &buf[offs], size - offs);
+		if (amt <= 0) {
+			swaylock_log_errno(LOG_ERROR, "failed to read pw");
+			return -1;
+		}
+		offs += (size_t)amt;
+	} while (offs < size);
+
+	*buf_ptr = buf;
+	return size;
+}
+
+bool write_comm_reply(bool success) {
+	if (write(comm[1][1], &success, sizeof(success)) != sizeof(success)) {
+		swaylock_log_errno(LOG_ERROR, "failed to write pw check result");
+		return false;
+	}
+	return true;
+}
+
+bool spawn_comm_child(void) {
+	if (pipe(comm[0]) != 0) {
+		swaylock_log_errno(LOG_ERROR, "failed to create pipe");
+		return false;
+	}
+	if (pipe(comm[1]) != 0) {
+		swaylock_log_errno(LOG_ERROR, "failed to create pipe");
+		return false;
+	}
+	pid_t child = fork();
+	if (child < 0) {
+		swaylock_log_errno(LOG_ERROR, "failed to fork");
+		return false;
+	} else if (child == 0) {
+		close(comm[0][1]);
+		close(comm[1][0]);
+		run_pw_backend_child();
+	}
+	close(comm[0][0]);
+	close(comm[1][1]);
+	return true;
+}
+
+bool write_comm_request(struct swaylock_password *pw) {
+	bool result = false;
+
+	size_t len = pw->len + 1;
+	size_t offs = 0;
+	if (write(comm[0][1], &len, sizeof(len)) < 0) {
+		swaylock_log_errno(LOG_ERROR, "Failed to request pw check");
+		goto out;
+	}
+
+	do {
+		ssize_t amt = write(comm[0][1], &pw->buffer[offs], len - offs);
+		if (amt < 0) {
+			swaylock_log_errno(LOG_ERROR, "Failed to write pw buffer");
+			goto out;
+		}
+		offs += amt;
+	} while (offs < len);
+
+	result = true;
+
+out:
+	clear_password_buffer(pw);
+	return result;
+}
+
+bool read_comm_reply(void) {
+	bool result = false;
+	if (read(comm[1][0], &result, sizeof(result)) != sizeof(result)) {
+		swaylock_log_errno(LOG_ERROR, "Failed to read pw result");
+		result = false;
+	}
+	return result;
+}
+
+int get_comm_reply_fd(void) {
+	return comm[1][0];
+}

include/comm.h

@@ -0,0 +1,18 @@
+#ifndef _SWAYLOCK_COMM_H
+#define _SWAYLOCK_COMM_H
+
+#include <stdbool.h>
+
+struct swaylock_password;
+
+bool spawn_comm_child(void);
+ssize_t read_comm_request(char **buf_ptr);
+bool write_comm_reply(bool success);
+// Requests the provided password to be checked. The password is always cleared
+// when the function returns.
+bool write_comm_request(struct swaylock_password *pw);
+bool read_comm_reply(void);
+// FD to poll for password authentication replies.
+int get_comm_reply_fd(void);
+
+#endif

include/swaylock.h

@@ -62,7 +62,6 @@ struct swaylock_state {
 	struct loop *eventloop;
 	struct loop_timer *clear_indicator_timer; // clears the indicator
 	struct loop_timer *clear_password_timer;  // clears the password buffer
-	struct loop_timer *verify_password_timer;
 	struct wl_display *display;
 	struct wl_compositor *compositor;
 	struct zwlr_layer_shell_v1 *layer_shell;
@@ -110,8 +109,11 @@ void render_frame(struct swaylock_surface *surface);
 void render_frames(struct swaylock_state *state);
 void damage_surface(struct swaylock_surface *surface);
 void damage_state(struct swaylock_state *state);
-void initialize_pw_backend(void);
-bool attempt_password(struct swaylock_password *pw);
 void clear_password_buffer(struct swaylock_password *pw);
+void schedule_indicator_clear(struct swaylock_state *state);
+
+void initialize_pw_backend(void);
+void run_pw_backend_child(void);
+void clear_buffer(char *buf, size_t size);
 
 #endif

loop.c

@@ -124,7 +124,7 @@ void loop_add_fd(struct loop *loop, int fd, short mask,
 	}
 	event->callback = callback;
 	event->data = data;
-	wl_list_insert(&loop->fd_events, &event->link);
+	wl_list_insert(loop->fd_events.prev, &event->link);
 
 	struct pollfd pfd = {fd, mask, 0};
 

main.c

@@ -17,6 +17,7 @@
 #include <wordexp.h>
 #include "background-image.h"
 #include "cairo.h"
+#include "comm.h"
 #include "log.h"
 #include "loop.h"
 #include "pool-buffer.h"
@@ -956,6 +957,17 @@ static void display_in(int fd, short mask, void *data) {
 	}
 }
 
+static void comm_in(int fd, short mask, void *data) {
+	if (read_comm_reply()) {
+		// Authentication succeeded
+		state.run_display = false;
+	} else {
+		state.auth_state = AUTH_STATE_INVALID;
+		schedule_indicator_clear(&state);
+		damage_state(&state);
+	}
+}
+
 int main(int argc, char **argv) {
 	swaylock_log_init(LOG_ERROR);
 	initialize_pw_backend();
@@ -1081,6 +1093,8 @@ int main(int argc, char **argv) {
 	loop_add_fd(state.eventloop, wl_display_get_fd(state.display), POLLIN,
 			display_in, NULL);
 
+	loop_add_fd(state.eventloop, get_comm_reply_fd(), POLLIN, comm_in, NULL);
+
 	state.run_display = true;
 	while (state.run_display) {
 		errno = 0;

meson.build

@@ -127,6 +127,7 @@ dependencies = [
 sources = [
 	'background-image.c',
 	'cairo.c',
+	'comm.c',
 	'log.c',
 	'loop.c',
 	'main.c',

pam.c

@@ -5,25 +5,37 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "comm.h"
 #include "log.h"
 #include "swaylock.h"
 
+static char *pw_buf = NULL;
+
 void initialize_pw_backend(void) {
-	// TODO: only call pam_start once. keep the same handle the whole time
+	if (!spawn_comm_child()) {
+		exit(EXIT_FAILURE);
+	}
 }
 
-static int function_conversation(int num_msg, const struct pam_message **msg,
+static int handle_conversation(int num_msg, const struct pam_message **msg,
 		struct pam_response **resp, void *data) {
-	struct swaylock_password *pw = data;
 	/* PAM expects an array of responses, one for each message */
-	struct pam_response *pam_reply = calloc(
-			num_msg, sizeof(struct pam_response));
+	struct pam_response *pam_reply =
+		calloc(num_msg, sizeof(struct pam_response));
+	if (pam_reply == NULL) {
+		swaylock_log(LOG_ERROR, "Allocation failed");
+		return PAM_ABORT;
+	}
 	*resp = pam_reply;
 	for (int i = 0; i < num_msg; ++i) {
 		switch (msg[i]->msg_style) {
 		case PAM_PROMPT_ECHO_OFF:
 		case PAM_PROMPT_ECHO_ON:
-			pam_reply[i].resp = strdup(pw->buffer); // PAM clears and frees this
+			pam_reply[i].resp = strdup(pw_buf); // PAM clears and frees this
+			if (pam_reply[i].resp == NULL) {
+				swaylock_log(LOG_ERROR, "Allocation failed");
+				return PAM_ABORT;
+			}
 			break;
 		case PAM_ERROR_MSG:
 		case PAM_TEXT_INFO:
@@ -51,38 +63,53 @@ static const char *get_pam_auth_error(int pam_status) {
 	}
 }
 
-bool attempt_password(struct swaylock_password *pw) {
+void run_pw_backend_child(void) {
 	struct passwd *passwd = getpwuid(getuid());
 	char *username = passwd->pw_name;
 
-	bool success = false;
-
-	const struct pam_conv local_conversation = {
-		.conv = function_conversation,
-		.appdata_ptr = pw,
+	const struct pam_conv conv = {
+		.conv = handle_conversation,
+		.appdata_ptr = NULL,
 	};
-	pam_handle_t *local_auth_handle = NULL;
-	if (pam_start("swaylock", username, &local_conversation, &local_auth_handle)
-			!= PAM_SUCCESS) {
+	pam_handle_t *auth_handle = NULL;
+	if (pam_start("swaylock", username, &conv, &auth_handle) != PAM_SUCCESS) {
 		swaylock_log(LOG_ERROR, "pam_start failed");
-		goto out;
+		exit(EXIT_FAILURE);
 	}
 
-	int pam_status = pam_authenticate(local_auth_handle, 0);
-	if (pam_status == PAM_SUCCESS) {
-		swaylock_log(LOG_DEBUG, "pam_authenticate succeeded");
-		success = true;
-	} else {
-		swaylock_log(LOG_ERROR, "pam_authenticate failed: %s",
-			get_pam_auth_error(pam_status));
+	/* This code does not run as root */
+	swaylock_log(LOG_DEBUG, "Prepared to authorize user %s", username);
+
+	int pam_status = PAM_SUCCESS;
+	while (1) {
+		ssize_t size = read_comm_request(&pw_buf);
+		if (size < 0) {
+			exit(EXIT_FAILURE);
+		} else if (size == 0) {
+			break;
+		}
+
+		int pam_status = pam_authenticate(auth_handle, 0);
+		bool success = pam_status == PAM_SUCCESS;
+		if (!success) {
+			swaylock_log(LOG_ERROR, "pam_authenticate failed: %s",
+				get_pam_auth_error(pam_status));
+		}
+
+		if (!write_comm_reply(success)) {
+			clear_buffer(pw_buf, size);
+			exit(EXIT_FAILURE);
+		}
+
+		clear_buffer(pw_buf, size);
+		free(pw_buf);
+		pw_buf = NULL;
 	}
 
-	if (pam_end(local_auth_handle, pam_status) != PAM_SUCCESS) {
+	if (pam_end(auth_handle, pam_status) != PAM_SUCCESS) {
 		swaylock_log(LOG_ERROR, "pam_end failed");
-		success = false;
+		exit(EXIT_FAILURE);
 	}
 
-out:
-	clear_password_buffer(pw);
-	return success;
+	exit((pam_status == PAM_SUCCESS) ? EXIT_SUCCESS : EXIT_FAILURE);
 }

password.c

@@ -5,19 +5,24 @@
 #include <string.h>
 #include <unistd.h>
 #include <xkbcommon/xkbcommon.h>
+#include "comm.h"
 #include "log.h"
 #include "loop.h"
 #include "seat.h"
 #include "swaylock.h"
 #include "unicode.h"
 
-void clear_password_buffer(struct swaylock_password *pw) {
+void clear_buffer(char *buf, size_t size) {
 	// Use volatile keyword so so compiler can't optimize this out.
-	volatile char *buffer = pw->buffer;
+	volatile char *buffer = buf;
 	volatile char zero = '\0';
-	for (size_t i = 0; i < sizeof(pw->buffer); ++i) {
+	for (size_t i = 0; i < size; ++i) {
 		buffer[i] = zero;
 	}
+}
+
+void clear_password_buffer(struct swaylock_password *pw) {
+	clear_buffer(pw->buffer, sizeof(pw->buffer));
 	pw->len = 0;
 }
 
@@ -47,7 +52,7 @@ static void clear_indicator(void *data) {
 	damage_state(state);
 }
 
-static void schedule_indicator_clear(struct swaylock_state *state) {
+void schedule_indicator_clear(struct swaylock_state *state) {
 	if (state->clear_indicator_timer) {
 		loop_remove_timer(state->eventloop, state->clear_indicator_timer);
 	}
@@ -72,57 +77,28 @@ static void schedule_password_clear(struct swaylock_state *state) {
 			state->eventloop, 10000, clear_password, state);
 }
 
-static void handle_preverify_timeout(void *data) {
-	struct swaylock_state *state = data;
-	state->verify_password_timer = NULL;
-}
-
 static void submit_password(struct swaylock_state *state) {
 	if (state->args.ignore_empty && state->password.len == 0) {
 		return;
 	}
 
 	state->auth_state = AUTH_STATE_VALIDATING;
-	damage_state(state);
 
-	// We generally want to wait until all surfaces are showing the
-	// "verifying" state before we go and verify the password, because
-	// verifying it is a blocking operation. However, if the surface is on
-	// an output with DPMS off then it won't update, so we set a timer.
-	state->verify_password_timer = loop_add_timer(
-			state->eventloop, 50, handle_preverify_timeout, state);
-
-	while (state->run_display && state->verify_password_timer) {
-		errno = 0;
-		if (wl_display_flush(state->display) == -1 && errno != EAGAIN) {
-			break;
-		}
-		loop_poll(state->eventloop);
-
-		bool ok = 1;
-		struct swaylock_surface *surface;
-		wl_list_for_each(surface, &state->surfaces, link) {
-			if (surface->dirty) {
-				ok = 0;
-			}
-		}
-		if (ok) {
-			break;
-		}
+	if (!write_comm_request(&state->password)) {
+		state->auth_state = AUTH_STATE_INVALID;
+		schedule_indicator_clear(state);
 	}
-	wl_display_flush(state->display);
 
-	if (attempt_password(&state->password)) {
-		state->run_display = false;
-		return;
-	}
-	state->auth_state = AUTH_STATE_INVALID;
 	damage_state(state);
-	schedule_indicator_clear(state);
 }
 
 void swaylock_handle_key(struct swaylock_state *state,
 		xkb_keysym_t keysym, uint32_t codepoint) {
+	// Ignore input events if validating
+	if (state->auth_state == AUTH_STATE_VALIDATING) {
+		return;
+	}
+
 	switch (keysym) {
 	case XKB_KEY_KP_Enter: /* fallthrough */
 	case XKB_KEY_Return:

shadow.c

@@ -4,24 +4,15 @@
 #include <stdbool.h>
 #include <sys/types.h>
 #include <unistd.h>
-#include "log.h"
-#include "swaylock.h"
 #ifdef __GLIBC__
 // GNU, you damn slimy bastard
 #include <crypt.h>
 #endif
+#include "comm.h"
+#include "log.h"
+#include "swaylock.h"
 
-static int comm[2][2];
-
-static void clear_buffer(void *buf, size_t bytes) {
-	volatile char *buffer = buf;
-	volatile char zero = '\0';
-	for (size_t i = 0; i < bytes; ++i) {
-		buffer[i] = zero;
-	}
-}
-
-static void run_child(void) {
+void run_pw_backend_child(void) {
 	/* This code runs as root */
 	struct passwd *pwent = getpwuid(getuid());
 	if (!pwent) {
@@ -51,44 +42,30 @@ static void run_child(void) {
 	}
 
 	/* This code does not run as root */
-	swaylock_log(LOG_DEBUG, "prepared to authorize user %s", pwent->pw_name);
+	swaylock_log(LOG_DEBUG, "Prepared to authorize user %s", pwent->pw_name);
 
-	size_t size;
-	char *buf;
 	while (1) {
-		ssize_t amt;
-		amt = read(comm[0][0], &size, sizeof(size));
-		if (amt == 0) {
-			break;
-		} else if (amt < 0) {
-			swaylock_log_errno(LOG_ERROR, "read pw request");
-		}
-		swaylock_log(LOG_DEBUG, "received pw check request");
-		buf = malloc(size);
-		if (!buf) {
-			swaylock_log_errno(LOG_ERROR, "failed to malloc pw buffer");
+		char *buf;
+		ssize_t size = read_comm_request(&buf);
+		if (size < 0) {
 			exit(EXIT_FAILURE);
+		} else if (size == 0) {
+			break;
 		}
-		size_t offs = 0;
-		do {
-			amt = read(comm[0][0], &buf[offs], size - offs);
-			if (amt <= 0) {
-				swaylock_log_errno(LOG_ERROR, "failed to read pw");
-				exit(EXIT_FAILURE);
-			}
-			offs += (size_t)amt;
-		} while (offs < size);
-		bool result = false;
+
 		char *c = crypt(buf, encpw);
 		if (c == NULL) {
-			swaylock_log_errno(LOG_ERROR, "crypt");
-		}
-		result = strcmp(c, encpw) == 0;
-		if (write(comm[1][1], &result, sizeof(result)) != sizeof(result)) {
-			swaylock_log_errno(LOG_ERROR, "failed to write pw check result");
+			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);
+		}
+
 		clear_buffer(buf, size);
 		free(buf);
 	}
@@ -103,25 +80,11 @@ void initialize_pw_backend(void) {
 				"swaylock needs to be setuid to read /etc/shadow");
 		exit(EXIT_FAILURE);
 	}
-	if (pipe(comm[0]) != 0) {
-		swaylock_log_errno(LOG_ERROR, "failed to create pipe");
+
+	if (!spawn_comm_child()) {
 		exit(EXIT_FAILURE);
 	}
-	if (pipe(comm[1]) != 0) {
-		swaylock_log_errno(LOG_ERROR, "failed to create pipe");
-		exit(EXIT_FAILURE);
-	}
-	pid_t child = fork();
-	if (child == 0) {
-		close(comm[0][1]);
-		close(comm[1][0]);
-		run_child();
-	} else if (child < 0) {
-		swaylock_log_errno(LOG_ERROR, "failed to fork");
-		exit(EXIT_FAILURE);
-	}
-	close(comm[0][0]);
-	close(comm[1][1]);
+
 	if (setgid(getgid()) != 0) {
 		swaylock_log_errno(LOG_ERROR, "Unable to drop root");
 		exit(EXIT_FAILURE);
@@ -136,29 +99,3 @@ void initialize_pw_backend(void) {
 		exit(EXIT_FAILURE);
 	}
 }
-
-bool attempt_password(struct swaylock_password *pw) {
-	bool result = false;
-	size_t len = pw->len + 1;
-	size_t offs = 0;
-	if (write(comm[0][1], &len, sizeof(len)) < 0) {
-		swaylock_log_errno(LOG_ERROR, "Failed to request pw check");
-		goto ret;
-	}
-	do {
-		ssize_t amt = write(comm[0][1], &pw->buffer[offs], len - offs);
-		if (amt < 0) {
-			swaylock_log_errno(LOG_ERROR, "Failed to write pw buffer");
-			goto ret;
-		}
-		offs += amt;
-	} while (offs < len);
-	if (read(comm[1][0], &result, sizeof(result)) != sizeof(result)) {
-		swaylock_log_errno(LOG_ERROR, "Failed to read pw result");
-		goto ret;
-	}
-	swaylock_log(LOG_DEBUG, "pw result: %d", result);
-ret:
-	clear_password_buffer(pw);
-	return result;
-}