The listen process now communicates the client UID/GID to the parent,
and the auth process now verifies UID/GID on behalf of the parent.

This allows us to remove the "unix" pledge promise from the parent,
removing parent access to syscalls such as listen() and accept() in
the AF_UNIX domain. The short-lived auth process seems like a better
place for this than the parent which keeps running forever. Neither
process would call listen() and accept() under normal circumstances.
But pledge() would not prevent them from doing so.

I don't want to trust the listener alone on UID/GID, so I need to
perform a secondary check of the peer somewhere. Again, the auth
process seems like a good place.
 
diff 33223026dc355d2180b79d22cbd06044ee342411 468ed428fd1abc15f97fec583f29eb1455dce126
commit - 33223026dc355d2180b79d22cbd06044ee342411
commit + 468ed428fd1abc15f97fec583f29eb1455dce126
blob - 57b17a1dd26385f232f8f989e4fa041babfeafe3
blob + fd7f8c11aa319b18aa9f13289797a9809af40003
--- gotd/auth.c
+++ gotd/auth.c
@@ -16,6 +16,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/socket.h>
 #include <sys/queue.h>
 #include <sys/uio.h>
 
@@ -192,6 +193,8 @@ recv_authreq(struct imsg *imsg, struct gotd_imsgev *ie
 	struct imsgbuf *ibuf = &iev->ibuf;
 	struct gotd_imsg_auth iauth;
 	size_t datalen;
+	uid_t euid;
+	gid_t egid;
 
 	log_debug("authentication request received");
 
@@ -201,6 +204,19 @@ recv_authreq(struct imsg *imsg, struct gotd_imsgev *ie
 
 	memcpy(&iauth, imsg->data, datalen);
 
+	if (imsg->fd == -1)
+		return got_error(GOT_ERR_PRIVSEP_NO_FD);
+
+	if (getpeereid(imsg->fd, &euid, &egid) == -1)
+		return got_error_from_errno("getpeerid");
+
+	if (iauth.euid != euid)
+		return got_error(GOT_ERR_UID);
+	if (iauth.egid != egid)
+		return got_error(GOT_ERR_GID);
+
+	log_debug("authenticating uid %d gid %d", euid, egid);
+
 	err = auth_check(&gotd_auth.repo->rules, gotd_auth.repo->name,
 	    iauth.euid, iauth.egid, iauth.required_auth);
 	if (err) {
blob - 4e693a39b4414ee735aa6d7f2649143c79ce3a88
blob + 05f659daea632d0e305556351e4d6a5e97519fa0
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -1225,8 +1225,6 @@ recv_connect(uint32_t *client_id, struct imsg *imsg)
 	size_t datalen;
 	int s = -1;
 	struct gotd_client *client = NULL;
-	uid_t euid;
-	gid_t egid;
 
 	*client_id = 0;
 
@@ -1246,11 +1244,6 @@ recv_connect(uint32_t *client_id, struct imsg *imsg)
 		goto done;
 	}
 
-	if (getpeereid(s, &euid, &egid) == -1) {
-		err = got_error_from_errno("getpeerid");
-		goto done;
-	}
-
 	client = calloc(1, sizeof(*client));
 	if (client == NULL) {
 		err = got_error_from_errno("calloc");
@@ -1264,8 +1257,9 @@ recv_connect(uint32_t *client_id, struct imsg *imsg)
 	client->fd = s;
 	s = -1;
 	client->delta_cache_fd = -1;
-	client->euid = euid;
-	client->egid = egid;
+	/* The auth process will verify UID/GID for us. */
+	client->euid = iconnect.euid;
+	client->egid = iconnect.egid;
 	client->nref_updates = -1;
 
 	imsg_init(&client->iev.ibuf, client->fd);
@@ -2308,14 +2302,23 @@ start_auth_child(struct gotd_client *client, int requi
     struct gotd_repo *repo, char *argv0, const char *confpath,
     int daemonize, int verbosity)
 {
+	const struct got_error *err = NULL;
 	struct gotd_child_proc *proc;
 	struct gotd_imsg_auth iauth;
+	int fd;
 
 	memset(&iauth, 0, sizeof(iauth));
 
+	fd = dup(client->fd);
+	if (fd == -1)
+		return got_error_from_errno("dup");
+
 	proc = calloc(1, sizeof(*proc));
-	if (proc == NULL)
-		return got_error_from_errno("calloc");
+	if (proc == NULL) {
+		err = got_error_from_errno("calloc");
+		close(fd);
+		return err;
+	}
 
 	proc->type = PROC_AUTH;
 	if (strlcpy(proc->repo_name, repo->name,
@@ -2346,8 +2349,11 @@ start_auth_child(struct gotd_client *client, int requi
 	iauth.required_auth = required_auth;
 	iauth.client_id = client->id;
 	if (gotd_imsg_compose_event(&proc->iev, GOTD_IMSG_AUTHENTICATE,
-	    PROC_GOTD, -1, &iauth, sizeof(iauth)) == -1)
+	    PROC_GOTD, fd, &iauth, sizeof(iauth)) == -1) {
 		log_warn("imsg compose AUTHENTICATE");
+		close(fd);
+		/* Let the auth_timeout handler tidy up. */
+	}
 
 	client->auth = proc;
 	client->required_auth = required_auth;
@@ -2562,7 +2568,7 @@ main(int argc, char **argv)
 	case PROC_GOTD:
 #ifndef PROFILE
 		if (pledge("stdio rpath wpath cpath proc exec "
-		    "sendfd recvfd fattr flock unix unveil", NULL) == -1)
+		    "sendfd recvfd fattr flock unveil", NULL) == -1)
 			err(1, "pledge");
 #endif
 		break;
@@ -2576,7 +2582,7 @@ main(int argc, char **argv)
 		break;
 	case PROC_AUTH:
 #ifndef PROFILE
-		if (pledge("stdio getpw", NULL) == -1)
+		if (pledge("stdio getpw recvfd unix", NULL) == -1)
 			err(1, "pledge");
 #endif
 		auth_main(title, &gotd.repos, repo_path);
blob - c1090adb4ecfd2e2c122cf9a6074590b01e560dd
blob + fe0c0107c18a0d38f7565091ea1d3badb537969e
--- gotd/gotd.h
+++ gotd/gotd.h
@@ -414,6 +414,8 @@ struct gotd_imsg_connect {
 /* Structure for GOTD_IMSG_CONNECT. */
 struct gotd_imsg_connect {
 	uint32_t client_id;
+	uid_t euid;
+	gid_t egid;
 };
 
 /* Structure for GOTD_IMSG_AUTHENTICATE. */
blob - 96427e857a4b693de2dec2665df312e682a43f73
blob + e20369a78e149e0e24de1d635e24b7a6161e4d16
--- gotd/listen.c
+++ gotd/listen.c
@@ -190,6 +190,8 @@ gotd_accept(int fd, short event, void *arg)
 	int s = -1;
 	struct gotd_listen_client *client = NULL;
 	struct gotd_imsg_connect iconn;
+	uid_t euid;
+	gid_t egid;
 
 	backoff.tv_sec = 1;
 	backoff.tv_usec = 0;
@@ -226,6 +228,11 @@ gotd_accept(int fd, short event, void *arg)
 	if (listen_client_cnt >= GOTD_MAXCLIENTS)
 		goto err;
 
+	if (getpeereid(s, &euid, &egid) == -1) {
+		log_warn("getpeerid");
+		goto err;
+	}
+
 	client = calloc(1, sizeof(*client));
 	if (client == NULL) {
 		log_warn("%s: calloc", __func__);
@@ -235,10 +242,13 @@ gotd_accept(int fd, short event, void *arg)
 	client->fd = s;
 	s = -1;
 	add_client(client);
-	log_debug("%s: new client connected on fd %d", __func__, client->fd);
+	log_debug("%s: new client connected on fd %d uid %d gid %d", __func__,
+	    client->fd, euid, egid);
 
 	memset(&iconn, 0, sizeof(iconn));
 	iconn.client_id = client->id;
+	iconn.euid = euid;
+	iconn.egid = egid;
 	s = dup(client->fd);
 	if (s == -1) {
 		log_warn("%s: dup", __func__);
blob - bbfa823cda9c46a471aefb0a9263d11583de85b4
blob + c951c76bc67d396a591c05f06bd362c98aa7f678
--- include/got_error.h
+++ include/got_error.h
@@ -182,6 +182,8 @@
 #define GOT_ERR_REF_PROTECTED	164
 #define GOT_ERR_REF_BUSY	165
 #define GOT_ERR_COMMIT_BAD_AUTHOR 166
+#define GOT_ERR_UID		167
+#define GOT_ERR_GID		168
 
 struct got_error {
         int code;
blob - 3b3a634ed0c2f2b1d0dd925891e217c07e57cf5c
blob + 7e698531e6e75bd34bc4b2f0c280aded67f43de0
--- lib/error.c
+++ lib/error.c
@@ -233,6 +233,8 @@ static const struct got_error got_errors[] = {
 	{ GOT_ERR_REF_BUSY, "reference cannot be updated; please try again" },
 	{ GOT_ERR_COMMIT_BAD_AUTHOR, "commit author formatting would "
 	    "make Git unhappy" },
+	{ GOT_ERR_UID, "bad user ID" },
+	{ GOT_ERR_GID, "bad group ID" },
 };
 
 static struct got_custom_error {