Move PAM into a child process

comm.c

@@ -0,0 +1,96 @@
+#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];
+
+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 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;
+}

include/comm.h

@@ -0,0 +1,8 @@
+#ifndef _SWAYLOCK_COMM_H
+#define _SWAYLOCK_COMM_H
+
+bool spawn_comm_child(void);
+ssize_t read_comm_request(char **buf_ptr);
+bool write_comm_reply(bool success);
+
+#endif

include/swaylock.h

@@ -110,8 +110,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 initialize_pw_backend(void);
+void run_pw_backend_child(void);
+void clear_buffer(char *buf, size_t size);
+
 #endif

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

@@ -11,13 +11,17 @@
 #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;
 }
 

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;
-}