Commit Diff

Commit:
1e84c7b793afb2f25b7515283340a1f730290c3c
From:
Omar Polo <op@omarpolo.com>
Date:
Message:
simplify client proc bootstrap Drop the IMSG_AUTH_DIR imsg type and stuff everything in a struct kd_auth_proc that gets sent via IMSG_AUTH. Tweak the necessary bits to compile. While here also start documenting what data we send over which imsg. As an added benefit, now we're sending fixed-size messages instead of variable length strings.
Actions:
Patch | Tree
commit - 35c6cd0e100a9bf0cc9354f261b656d497fa34cd
commit + 1e84c7b793afb2f25b7515283340a1f730290c3c
blob - 0b178a42ff7d5f9f95ce9ded30db61b7162aee21
blob + 311ce7967d863fbd1fa4cb4ef25cdbb05282e2ce
--- kamid/client.c
+++ kamid/client.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Omar Polo <op@omarpolo.com>
+ * Copyright (c) 2021, 2022 Omar Polo <op@omarpolo.com>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -250,16 +250,11 @@ client_sig_handler(int sig, short event, void *d)
 	}
 }
 
-#define AUTH_NONE 0
-#define AUTH_USER 1
-#define AUTH_DONE 2
-
 static void
 client_dispatch_listener(int fd, short event, void *d)
 {
-	static int		 auth = AUTH_NONE;
-	static char		 username[64] = {0};
-	static char		 dir[PATH_MAX] = {0};
+	static int		 auth = 0;
+	struct kd_auth_proc	 rauth;
 	struct imsg		 imsg;
 	struct imsgev		*iev = d;
 	struct imsgbuf		*ibuf;
@@ -292,23 +287,19 @@ client_dispatch_listener(int fd, short event, void *d)
 			peerid = imsg.hdr.peerid;
 			if (auth)
 				fatalx("%s: IMSG_AUTH already done", __func__);
-			auth = AUTH_USER;
-			((char *)imsg.data)[IMSG_DATA_SIZE(imsg)-1] = '\0';
-			strlcpy(username, imsg.data, sizeof(username));
-			break;
-		case IMSG_AUTH_DIR:
-			if (auth != AUTH_USER)
-				fatalx("%s: IMSG_AUTH_DIR not after IMSG_AUTH",
-				    __func__);
-			auth = AUTH_DONE;
-			((char *)imsg.data)[IMSG_DATA_SIZE(imsg)-1] = '\0';
-			strlcpy(dir, imsg.data, sizeof(dir));
-			client_privdrop(username, dir);
-			memset(username, 0, sizeof(username));
-			memset(dir, 0, sizeof(username));
+			auth = 1;
+
+			if (IMSG_DATA_SIZE(imsg) != sizeof(rauth))
+				fatalx("mismatching size for IMSG_AUTH");
+			memcpy(&rauth, imsg.data, sizeof(rauth));
+			if (rauth.uname[sizeof(rauth.uname)-1] != '\0' ||
+			    rauth.dir[sizeof(rauth.dir)-1] != '\0')
+				fatalx("IMSG_AUTH strings not NUL-terminated");
+
+			client_privdrop(rauth.uname, rauth.dir);
+			explicit_bzero(&rauth, sizeof(rauth));
 			break;
 		case IMSG_BUF:
-			/* echo! */
 			if (!auth)
 				fatalx("%s: can't handle messages before"
 				    " doing the auth", __func__);
blob - 0c963c740a461c315d0027210d578d980db92bc8
blob + 557c6c924bcce97c4ceaf63fc047a06bab004705
--- kamid/kamid.c
+++ kamid/kamid.c
@@ -296,6 +296,7 @@ do_auth_tls(struct imsg *imsg)
 	struct passwd *pw;
 	struct table *auth, *virt, *userdata;
 	struct kd_auth_req kauth;
+	struct kd_auth_proc rauth;
 	int p[2], free_home = 1;
 
 	if (sizeof(kauth) != IMSG_DATA_SIZE(*imsg))
@@ -355,6 +356,14 @@ do_auth_tls(struct imsg *imsg)
 		log_debug("matched home %s for local user %s",
 		    home, username);
 
+	memset(&rauth, 0, sizeof(rauth));
+	strlcpy(rauth.uname, local_user, sizeof(rauth.uname));
+	if (strlcpy(rauth.dir, home, sizeof(rauth.dir)) >= sizeof(rauth.dir)) {
+		log_warnx("home for %s is bigger than PATH_MAX: %s",
+		    username, home);
+		goto err;
+	}
+
 	if (socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK,
 	    PF_UNSPEC, p) == -1)
 		fatal("socketpair");
@@ -362,9 +371,7 @@ do_auth_tls(struct imsg *imsg)
 	start_child(PROC_CLIENTCONN, p[1], debug, verbose);
 
 	main_imsg_compose_listener(IMSG_AUTH, p[0], imsg->hdr.peerid,
-	    local_user, strlen(local_user)+1);
-	main_imsg_compose_listener(IMSG_AUTH_DIR, -1, imsg->hdr.peerid,
-	    home, strlen(home)+1);
+	    &rauth, sizeof(rauth));
 
 	free(username);
 	free(user);
blob - db25c4112dcb5d9713a34fc25b24c1d432b60eab
blob + 6a9f167f1877451fcec866b2f6ef0c748aa1a47e
--- kamid/kamid.h
+++ kamid/kamid.h
@@ -40,9 +40,8 @@ enum imsg_type {
 	IMSG_RECONF_PKI_KEY,
 	IMSG_RECONF_LISTEN,
 	IMSG_RECONF_END,
-	IMSG_AUTH,
-	IMSG_AUTH_DIR,
-	IMSG_AUTH_TLS,
+	IMSG_AUTH,		/* kd_auth_proc */
+	IMSG_AUTH_TLS,		/* kd_auth_req */
 	IMSG_CONN_GONE,
 	IMSG_BUF,
 	IMSG_MSIZE,
@@ -106,6 +105,11 @@ struct kd_auth_req {
 	char		hash[128+1];
 };
 
+struct kd_auth_proc {
+	char		uname[LOGIN_NAME_MAX];
+	char		dir[PATH_MAX];
+};
+
 /* kamid.c */
 extern int verbose;
 int	main_imsg_compose_listener(int, int, uint32_t, const void *, uint16_t);
blob - 8df8890afda9b5ad12af7a81c69dc416216b8185
blob + b2105530e2b2cfd3c5f65fc4751dca13bc9be9fb
--- kamid/listener.c
+++ kamid/listener.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Omar Polo <op@omarpolo.com>
+ * Copyright (c) 2021, 2022 Omar Polo <op@omarpolo.com>
  * Copyright (c) 2018 Florian Obser <florian@openbsd.org>
  * Copyright (c) 2004, 2005 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2004 Esben Norby <norby@openbsd.org>
@@ -328,6 +328,9 @@ listener_dispatch_main(int fd, short event, void *d)
 			listener_receive_config(&imsg, &nconf, &pki);
 			break;
 		case IMSG_AUTH:
+			if (IMSG_DATA_SIZE(imsg) != sizeof(struct kd_auth_proc))
+				fatalx("mismatching size for IMSG_AUTH");
+
 			find.id = imsg.hdr.peerid;
 			client = SPLAY_FIND(clients_tree_id, &clients, &find);
 			if (client == NULL) {
@@ -347,18 +350,7 @@ listener_dispatch_main(int fd, short event, void *d)
 			    client->iev.events, client->iev.handler, client);
 			listener_imsg_compose_client(client, IMSG_AUTH,
 			    client->id, imsg.data, IMSG_DATA_SIZE(imsg));
-			break;
-		case IMSG_AUTH_DIR:
-			find.id = imsg.hdr.peerid;
-			client = SPLAY_FIND(clients_tree_id, &clients, &find);
-			if (client == NULL) {
-				log_info("got AUTH_DIR but client gone");
-				break;
-			}
 
-			listener_imsg_compose_client(client, IMSG_AUTH_DIR,
-			    0, imsg.data, IMSG_DATA_SIZE(imsg));
-
 			client->bev = bufferevent_new(client->fd,
 			    client_read, client_write, client_error,
 			    client);
blob - 28f514eaba85c9b0e3434d9803cdeffbdee9f12d
blob + 373b881200da8e0c7a0c4e5f05ccf6d98baa56db
--- ninepscript/script.c
+++ ninepscript/script.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Omar Polo <op@omarpolo.com>
+ * Copyright (c) 2021, 2022 Omar Polo <op@omarpolo.com>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -1590,15 +1590,18 @@ spawn_client_proc(void)
 static void
 prepare_child_for_test(struct test *t)
 {
-	struct passwd	*pw;
+	struct passwd		*pw;
+	struct kd_auth_proc	 rauth;
 
 	if ((pw = getpwuid(uid)) == NULL)
 		fatal("getpwuid(%d)", uid);
 
+	memset(&rauth, 0, sizeof(rauth));
+	strlcpy(rauth.uname, pw->pw_name, sizeof(rauth.uname));
+	strlcpy(rauth.dir, dir, sizeof(rauth.dir));
+
 	imsg_compose(&ibuf, IMSG_AUTH, 0, 0, -1,
-	    pw->pw_name, strlen(pw->pw_name)+1);
-	imsg_compose(&ibuf, IMSG_AUTH_DIR, 0, 0, -1,
-	    dir, strlen(dir)+1);
+	    &rauth, sizeof(rauth));
 
 	if (imsg_flush(&ibuf) == -1)
 		fatal("imsg_flush");
@@ -1709,7 +1712,7 @@ main(int argc, char **argv)
 
 	if (dir == NULL)
 		fatal("missing root test dir");
-	
+
 	if (stat(dir, &sb) == -1)
 		fatal("stat(\"%s\")", dir);
 	uid = sb.st_uid;