Rework the code, update keyring handlingsystemd-v233

This had some historical issue... So rework the code:

* split into more functions
* drop the sleep and notify logic
* update keyring handling

Depending on setup and systemd version (233 and up) the keyring handling
fails. Try to fix this by...

* writing to session keyring first
* setting permissions
* linking to user keyring
* unlinking from session keyring

https://mjg59.dreamwidth.org/37333.html

6 files changed, 208 insertions, 194 deletions

diff --git a/Makefile b/Makefile
index 78b5993..5c2b677 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,7 @@ install-bin: bin/ykfde udev/ykfde
 	$(INSTALL) -D -m0755 grub/09_linux $(DESTDIR)/etc/grub.d/09_linux
 	$(INSTALL) -D -m0644 systemd/ykfde.service $(DESTDIR)/usr/lib/systemd/system/ykfde.service
 	$(INSTALL) -D -m0644 systemd/ykfde-2f.service $(DESTDIR)/usr/lib/systemd/system/ykfde-2f.service
-	$(INSTALL) -D -m0644 systemd/ykfde-notify.service $(DESTDIR)/usr/lib/systemd/system/ykfde-notify.service
+	$(INSTALL) -D -m0644 systemd/ykfde-worker.service $(DESTDIR)/usr/lib/systemd/system/ykfde-worker.service
 	$(INSTALL) -d -m0700 $(DESTDIR)/etc/ykfde.d/
 
 install-doc: README.html README-mkinitcpio.html README-dracut.html
diff --git a/bin/ykfde.c b/bin/ykfde.c
index 96b5559..e72fd0d 100644
--- a/bin/ykfde.c
+++ b/bin/ykfde.c
@@ -264,7 +264,7 @@ int main(int argc, char **argv) {
 
 	if (second_factor == NULL) {
 		/* get second factor from key store */
-		if ((key = request_key("user", "ykfde-2f", NULL, 0)) < 0)
+		if ((key = keyctl_search(KEY_SPEC_USER_KEYRING, "user", "ykfde-2f", 0)) < 0)
 			fprintf(stderr, "Failed requesting key. That's ok if you do not use\n"
 					"second factor. Give it manually if required.\n");
 
diff --git a/dracut/module-setup.sh b/dracut/module-setup.sh
index d7fdb3f..3cab1c8 100755
--- a/dracut/module-setup.sh
+++ b/dracut/module-setup.sh
@@ -22,8 +22,8 @@ install() {
 		inst_simple /usr/lib/systemd/system/cryptsetup-pre.target
 		inst_simple /usr/lib/systemd/system/ykfde-2f.service
 		ln_r $systemdsystemunitdir/ykfde-2f.service $systemdsystemunitdir/sysinit.target.wants/ykfde-2f.service
-		inst_simple /usr/lib/systemd/system/ykfde-notify.service
-		ln_r $systemdsystemunitdir/ykfde-notify.service $systemdsystemunitdir/sysinit.target.wants/ykfde-notify.service
+		inst_simple /usr/lib/systemd/system/ykfde-worker.service
+		ln_r $systemdsystemunitdir/ykfde-worker.service $systemdsystemunitdir/sysinit.target.wants/ykfde-worker.service
 		inst_simple /usr/bin/systemd-ask-password
 		inst_simple /usr/bin/pkill
 		inst_simple /usr/bin/sleep
diff --git a/mkinitcpio/ykfde b/mkinitcpio/ykfde
index 4932dfd..d30e331 100644
--- a/mkinitcpio/ykfde
+++ b/mkinitcpio/ykfde
@@ -11,8 +11,8 @@ build() {
 		add_systemd_unit cryptsetup-pre.target
 		add_systemd_unit ykfde-2f.service
 		add_symlink /usr/lib/systemd/system/sysinit.target.wants/ykfde-2f.service ../ykfde-2f.service
-		add_systemd_unit ykfde-notify.service
-		add_symlink /usr/lib/systemd/system/sysinit.target.wants/ykfde-notify.service ../ykfde-notify.service
+		add_systemd_unit ykfde-worker.service
+		add_symlink /usr/lib/systemd/system/sysinit.target.wants/ykfde-worker.service ../ykfde-worker.service
 		add_binary systemd-ask-password
 		add_binary pkill
 		add_binary sleep
diff --git a/systemd/ykfde-notify.service b/systemd/ykfde-worker.service
index c3a8d21..6f5a18f 100644
--- a/systemd/ykfde-notify.service
+++ b/systemd/ykfde-worker.service
@@ -4,19 +4,13 @@
 # of the GNU General Public License, incorporated herein by reference.
 
 [Unit]
-Description=Notify ykfde about key
+Description=Run ykfde worker
 DefaultDependencies=no
 Before=cryptsetup-pre.target
 Wants=cryptsetup-pre.target
 Requires=ykfde-2f.service
 After=ykfde-2f.service
-ConditionPathExists=/run/ykfde.pid
 
 [Service]
 Type=oneshot
-RemainAfterExit=yes
-ExecStart=/usr/bin/pkill -USR1 --pidfile /run/ykfde.pid
-# ykfde started from udev needs a moment to set up the key
-# in store. It is out of systemd control, so wait a moment
-# here.
-ExecStart=/usr/bin/sleep 0.2
+ExecStart=/usr/lib/udev/ykfde
diff --git a/udev/ykfde.c b/udev/ykfde.c
index 98ca946..ab20fbd 100644
--- a/udev/ykfde.c
+++ b/udev/ykfde.c
@@ -15,7 +15,6 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <stddef.h>
-#include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -52,13 +51,8 @@
 
 #define ASK_PATH	"/run/systemd/ask-password/"
 #define ASK_MESSAGE	"Please enter passphrase for disk"
-#define PID_PATH	"/run/ykfde.pid"
-
-void received_signal(int signal) {
-	/* Do nothing, just interrupt the sleep. */
-	return;
-}
 
+/*** send_on_socket ***/
 static int send_on_socket(int fd, const char *socket_name, const void *packet, size_t size) {
 	union {
 		struct sockaddr sa;
@@ -77,74 +71,140 @@ static int send_on_socket(int fd, const char *socket_name, const void *packet, s
 	return EXIT_SUCCESS;
 }
 
+/*** yk_open_and_check ***/
 static YK_KEY * yk_open_and_check(const unsigned int expected, unsigned int * serial) {
 	YK_KEY * yk;
 
 	if ((yk = yk_open_first_key()) == NULL) {
-		perror("yk_open_first_key() failed");
-		goto error;
+		if (errno != EAGAIN)
+			perror("yk_open_first_key() failed");
+		goto out1;
 	}
 
 	if (serial != NULL) {
 		/* read the serial number from key */
 		if (yk_get_serial(yk, 0, 0, serial) == 0) {
 			perror("yk_get_serial() failed");
-			goto error;
+			goto out2;
 		}
 
 		if (expected > 0 && expected != *serial) {
 			fprintf(stderr, "Opened Yubikey with unexpected serial number (%d != %d)...\n", expected, *serial);
-			goto error;
+			goto out2;
 		}
 	}
 
 	return yk;
 
-error:
+out2:
 	/* close Yubikey */
-	if (yk != NULL)
-		if (yk_close_key(yk) == 0)
-			perror("yk_close_key() failed");
+	if (yk_close_key(yk) == 0)
+		perror("yk_close_key() failed");
 
+out1:
 	return NULL;
 }
 
-static int try_answer(const unsigned int serial, uint8_t slot, const char * ask_file, char * challenge) {
-	int8_t rc = EXIT_FAILURE;
-	YK_KEY * yk;
-	dictionary * ini;
-	const char * ask_message, * ask_socket;
-	int fd_askpass;
-	char response[RESPONSELEN],
-		askpass[PASSPHRASELEN + 2];
-	char * passphrase = askpass + 1;
-	/* keyutils */
-	key_serial_t key;
-	void * payload = NULL;
-	size_t plen;
+/*** read_challenge ***/
+static int read_challenge(const unsigned int serial, char * challenge) {
+	int rc = EXIT_FAILURE;
+	char challengefilename[sizeof(CHALLENGEDIR) + 11 /* "/challenge-" */ + 10 /* unsigned int in char */ + 1];
+	int challengefile;
 
-	memset(response, 0, RESPONSELEN);
-	memset(askpass, 0, PASSPHRASELEN + 2);
+	snprintf(challengefilename, sizeof(challengefilename), CHALLENGEDIR "/challenge-%d", serial);
 
-	*askpass = '+';
+	/* check if challenge file exists */
+	if (access(challengefilename, R_OK) == -1) {
+		goto out1;
+	}
+
+	/* read challenge from file */
+	if ((challengefile = open(challengefilename, O_RDONLY)) < 0) {
+		perror("Failed opening challenge file for reading");
+		goto out1;
+	}
+
+	if (read(challengefile, challenge, CHALLENGELEN) < 0) {
+		perror("Failed reading challenge from file");
+		goto out2;
+	}
+
+	rc = EXIT_SUCCESS;
+
+out2:
+	close(challengefile);
+
+out1:
+	return rc;
+}
+
+/*** get_second_factor ***/
+static char * get_second_factor(void) {
+	key_serial_t key;
+	void * payload = NULL;
 
 	/* get second factor from key store
-	 * if this fails it is not critical... possibly we just do not
-	 * use second factor */
-	key = request_key("user", "ykfde-2f", NULL, 0);
+	 * If this fails it is not critical... possibly we just do not
+	 * use second factor. */
+	key = keyctl_search(KEY_SPEC_USER_KEYRING, "user", "ykfde-2f", 0);
 
 	if (key > 0) {
 		/* if we have a key id we have a key - so this should succeed */
 		if (keyctl_read_alloc(key, &payload) < 0) {
 			perror("Failed reading payload from key");
-			goto out1;
+			return NULL;
 		}
 
+		return payload;
+	}
+
+	return NULL;
+}
+
+/*** get_response ***/
+static int get_response(const unsigned int serial, uint8_t slot, char * challenge, char * passphrase) {
+	YK_KEY * yk;
+	char response[RESPONSELEN];
+	char * second_factor;
+	size_t second_factor_len;
+	/* iniparser */
+	dictionary * ini;
+	char section_ykslot[10 /* unsigned int in char */ + 1 + sizeof(CONFYKSLOT) + 1];
+
+	memset(response, 0, RESPONSELEN);
+
+	if ((second_factor = get_second_factor()) != NULL) {
 		/* we replace part of the challenge with the second factor */
-		plen = strlen(payload);
-		memcpy(challenge, payload, plen < CHALLENGELEN / 2 ? plen : CHALLENGELEN / 2);
+		second_factor_len = strlen(second_factor);
+		memcpy(challenge, second_factor, second_factor_len < CHALLENGELEN / 2 ?
+				second_factor_len : CHALLENGELEN / 2);
+		free(second_factor);
+	}
+
+	/* try to read config file
+	 * If anything here fails we do not care... slot 2 is the default. */
+	if ((ini = iniparser_load(CONFIGFILE)) != NULL) {
+		/* first try the general setting */
+		slot = iniparser_getint(ini, "general:" CONFYKSLOT, slot);
+
+		sprintf(section_ykslot, "%d:" CONFYKSLOT, serial);
+
+		/* then probe for setting with serial number */
+		slot = iniparser_getint(ini, section_ykslot, slot);
+
+		switch (slot) {
+			case 1:
+			case SLOT_CHAL_HMAC1:
+				slot = SLOT_CHAL_HMAC1;
+				break;
+			case 2:
+			case SLOT_CHAL_HMAC2:
+			default:
+				slot = SLOT_CHAL_HMAC2;
+				break;
+		}
 
-		free(payload);
+		iniparser_freedict(ini);
 	}
 
 	/* open Yubikey and check serial */
@@ -161,28 +221,62 @@ static int try_answer(const unsigned int serial, uint8_t slot, const char * ask_
 		goto out2;
 	}
 
+	yubikey_hex_encode((char *) passphrase, (char *) response, SHA1_DIGEST_SIZE);
+
+out2:
 	/* close Yubikey */
-	if (yk_close_key(yk) == 0) {
+	if (yk_close_key(yk) == 0)
 		perror("yk_close_key() failed");
-		goto out1;
-	}
-	yk = NULL;
 
-	yubikey_hex_encode((char *) passphrase, (char *) response, SHA1_DIGEST_SIZE);
+out1:
+	memset(response, 0, RESPONSELEN);
+
+	return EXIT_SUCCESS;
+}
+
+/*** add_keyring ***/
+static int add_keyring(const char * passphrase) {
+	key_serial_t key;
 
-	/* add key to kernel key store */
-	if ((key = add_key("user", "cryptsetup", passphrase, PASSPHRASELEN, KEY_SPEC_USER_KEYRING)) > 0) {
-		if (keyctl_set_timeout(key, 150) < 0)
-			perror("keyctl_set_timeout() failed");
-	} else
+	/* add key to kernel key store
+	 * Put it into session keyring first, set permissions and
+	 * move it to user keyring. */
+	if ((key = add_key("user", "cryptsetup", passphrase,
+			PASSPHRASELEN, KEY_SPEC_SESSION_KEYRING)) < 0) {
 		perror("add_key() failed");
+		return -1;
+	}
 
-	/* key is placed, no ask file... quit here */
-	if (ask_file == NULL) {
-		rc = key > 0 ? EXIT_SUCCESS : EXIT_FAILURE;
-		goto out1;
+	if (keyctl_set_timeout(key, 150) < 0) {
+		perror("keyctl_set_timeout() failed");
+		return -1;
+	}
+
+	if (keyctl_setperm(key, KEY_POS_ALL|KEY_USR_ALL) < 0) {
+		perror("keyctl_setperm() failed");
+		return -1;
+	}
+
+	if (keyctl_link(key, KEY_SPEC_USER_KEYRING) < 0) {
+		perror("keyctl_link() failed");
+		return -1;
 	}
 
+	if (keyctl_unlink(key, KEY_SPEC_SESSION_KEYRING) < 0) {
+		perror("keyctl_unlink() failed");
+		return -1;
+	}
+
+	return EXIT_SUCCESS;
+}
+
+/*** answer_askpass ***/
+static int answer_askpass(const char * ask_file, const char * passphrase) {
+	int rc = EXIT_FAILURE, fd_askpass;
+	const char * ask_message, * ask_socket;
+	/* iniparser */
+	dictionary * ini;
+
 	if ((ini = iniparser_load(ask_file)) == NULL) {
 		perror("cannot parse file");
 		goto out1;
@@ -191,62 +285,78 @@ static int try_answer(const unsigned int serial, uint8_t slot, const char * ask_
 	ask_message = iniparser_getstring(ini, "Ask:Message", NULL);
 
 	if (strncmp(ask_message, ASK_MESSAGE, strlen(ASK_MESSAGE)) != 0)
-		goto out3;
+		goto out2;
 
 	if ((ask_socket = iniparser_getstring(ini, "Ask:Socket", NULL)) == NULL) {
 		perror("Could not get socket name");
-		goto out3;
+		goto out2;
 	}
 
 	if ((fd_askpass = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0)) < 0) {
 		perror("socket() failed");
-		goto out3;
+		goto out2;
 	}
 
-	if (send_on_socket(fd_askpass, ask_socket, askpass, PASSPHRASELEN + 1) < 0) {
+	if (send_on_socket(fd_askpass, ask_socket, passphrase, PASSPHRASELEN + 1) < 0) {
 		perror("send_on_socket() failed");
-		goto out4;
+		goto out3;
 	}
 
 	rc = EXIT_SUCCESS;
 
-out4:
-	close(fd_askpass);
-
 out3:
-	iniparser_freedict(ini);
+	close(fd_askpass);
 
 out2:
-	/* close Yubikey */
-	if (yk != NULL)
-		if (yk_close_key(yk) == 0)
-			perror("yk_close_key() failed");
+	iniparser_freedict(ini);
 
 out1:
-	/* wipe response (cleartext password!) from memory */
-	memset(response, 0, RESPONSELEN);
-	memset(askpass, 0, PASSPHRASELEN + 2);
+	return rc;
+}
+
+/*** walk_askpass ***/
+static int walk_askpass(const char * passphrase) {
+	int rc = EXIT_FAILURE;
+	DIR * dir;
+	struct dirent * ent;
+
+	/* change to directory so we do not have to assemble complete/absolute path */
+	if (chdir(ASK_PATH) != 0) {
+		perror("chdir() failed");
+		return rc;
+	}
+
+	/* Is the request already there? */
+	if ((dir = opendir(ASK_PATH)) != NULL) {
+		while ((ent = readdir(dir)) != NULL) {
+			if (strncmp(ent->d_name, "ask.", 4) == 0) {
+				if ((rc = answer_askpass(ent->d_name, passphrase)) == EXIT_SUCCESS)
+					goto out;
+			}
+		}
+	} else {
+		perror ("opendir() failed");
+		return EXIT_FAILURE;
+	}
+
+	rc = EXIT_SUCCESS;
+
+out:
+	closedir(dir);
 
 	return rc;
 }
 
+/*** main ***/
 int main(int argc, char **argv) {
 	int8_t rc = EXIT_FAILURE;
-	FILE *pidfile;
 	/* Yubikey */
 	YK_KEY * yk;
 	uint8_t slot = SLOT_CHAL_HMAC2;
 	unsigned int serial = 0;
-	/* iniparser */
-	dictionary * ini;
-	char section_ykslot[10 /* unsigned int in char */ + 1 + sizeof(CONFYKSLOT) + 1];
-	/* read challenge */
+	/* challenge and passphrase */
 	char challenge[CHALLENGELEN + 1];
-	char challengefilename[sizeof(CHALLENGEDIR) + 11 /* "/challenge-" */ + 10 /* unsigned int in char */ + 1];
-	int challengefile;
-	/* read dir */
-	DIR * dir;
-	struct dirent * ent;
+	char passphrase[PASSPHRASELEN + 2];
 
 #ifdef DEBUG
 	/* reopening stderr to /dev/console may help debugging... */
@@ -254,24 +364,11 @@ int main(int argc, char **argv) {
 	(void) tmp;
 #endif
 
-	if ((pidfile = fopen(PID_PATH, "w")) != NULL) {
-		if (fprintf(pidfile, "%d", getpid()) < 0) {
-			perror("Failed writing pid");
-			fclose(pidfile);
-			goto out10;
-		}
-		fclose(pidfile);
-	} else {
-		rc = EXIT_FAILURE;
-		perror("Failed opening pid file");
-		goto out10;
-	}
-
-	/* connect to signal */
-	signal(SIGUSR1, received_signal);
-
 	/* initialize static memory */
 	memset(challenge, 0, CHALLENGELEN + 1);
+	memset(passphrase, 0, PASSPHRASELEN + 2);
+
+	*passphrase = '+';
 
 	/* init and open first Yubikey */
 	if (yk_init() == 0) {
@@ -281,7 +378,8 @@ int main(int argc, char **argv) {
 
 	/* open Yubikey and get serial */
 	if ((yk = yk_open_and_check(0, &serial)) == NULL) {
-		fprintf(stderr, "yk_open_and_check() failed\n");
+		if (errno == EAGAIN)
+			rc = EXIT_SUCCESS;
 		goto out30;
 	}
 
@@ -290,97 +388,20 @@ int main(int argc, char **argv) {
 		perror("yk_close_key() failed");
 		goto out30;
 	}
-	yk = NULL;
-
-	sprintf(challengefilename, CHALLENGEDIR "/challenge-%d", serial);
 
-	/* check if challenge file exists */
-	if (access(challengefilename, R_OK) == -1) {
+	if ((rc = read_challenge(serial, challenge)) < 0)
 		goto out30;
-	}
 
-	/* read challenge from file */
-	if ((challengefile = open(challengefilename, O_RDONLY)) < 0) {
-		perror("Failed opening challenge file for reading");
+	if ((rc = get_response(serial, slot, challenge, passphrase + 1)) < 0)
 		goto out30;
-	}
-
-	if (read(challengefile, challenge, CHALLENGELEN) < 0) {
-		perror("Failed reading challenge from file");
-		goto out40;
-	}
-
-	/* try to read config file
-	 * if anything here fails we do not care... slot 2 is the default */
-	if ((ini = iniparser_load(CONFIGFILE)) != NULL) {
-		/* first try the general setting */
-		slot = iniparser_getint(ini, "general:" CONFYKSLOT, slot);
-
-		sprintf(section_ykslot, "%d:" CONFYKSLOT, serial);
 
-		/* then probe for setting with serial number */
-		slot = iniparser_getint(ini, section_ykslot, slot);
-
-		switch (slot) {
-			case 1:
-			case SLOT_CHAL_HMAC1:
-				slot = SLOT_CHAL_HMAC1;
-				break;
-			case 2:
-			case SLOT_CHAL_HMAC2:
-			default:
-				slot = SLOT_CHAL_HMAC2;
-				break;
-		}
-
-		iniparser_freedict(ini);
-	}
-
-	/* change to directory so we do not have to assemble complete/absolute path */
-	if (chdir(ASK_PATH) != 0) {
-		perror("chdir() failed");
-		goto out40;
-	}
-
-	/* Is the request already there? */
-	if ((dir = opendir(ASK_PATH)) != NULL) {
-		while ((ent = readdir(dir)) != NULL) {
-			if (strncmp(ent->d_name, "ask.", 4) == 0) {
-				if ((rc = try_answer(serial, slot, ent->d_name, challenge)) == EXIT_SUCCESS)
-					goto out50;
-			}
-		}
-	} else {
-		perror ("opendir() failed");
-		goto out50;
-	}
-
-	/* Wait for 90 seconds.
-	 * The user has this time to enter his second factor, resulting in
-	 * SIGUSR1 being sent to us. */
-	sleep(90);
-
-	/* try again, but for key store this time */
-	rc = try_answer(serial, slot, NULL, challenge);
-
-out50:
-	/* close dir */
-	closedir(dir);
+	if ((rc = add_keyring(passphrase + 1)) < 0)
+		goto out30;
 
-out40:
-	/* close the challenge file */
-	if (challengefile)
-		close(challengefile);
-	/* Unlink it if we were successful, we can not try again later! */
-	if (rc == EXIT_SUCCESS)
-		unlink(challengefilename);
+	if ((rc = walk_askpass(passphrase)) < 0)
+		goto out30;
 
 out30:
-	/* close Yubikey */
-	if (yk != NULL)
-		if (yk_close_key(yk) == 0)
-			perror("yk_close_key() failed");
-
 	/* release Yubikey */
 	if (yk_release() == 0)
 		perror("yk_release() failed");
@@ -388,8 +409,7 @@ out30:
 out10:
 	/* wipe challenge from memory */
 	memset(challenge, 0, CHALLENGELEN + 1);
-
-	unlink(PID_PATH);
+	memset(passphrase, 0, PASSPHRASELEN + 2);
 
 	return rc;
 }