Game of Trees Repos

Commit Diff

Commit:: 365cf0f34d08316d433e730a8663283029f729b3
From:: Stefan Sperling <stsp@stsp.name>
Date:: Thu Dec 29 18:10:54 2022 UTC
Message:: move "unix" pledge promise from gotd parent to auth process The listen process now communicates the client UID/GID to the parent, and the auth process verifies this 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. ok tracey@ op@
Actions:: Patch | Tree
commit - ef4e2f01b0bff14f003a72321ce0165a0df72fa1
commit + 365cf0f34d08316d433e730a8663283029f729b3
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");
 
@@ -200,7 +203,20 @@ recv_authreq(struct imsg *imsg, struct gotd_imsgev *ie
 		return got_error(GOT_ERR_PRIVSEP_LEN);
 
 	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_disconnect {
 /* 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;
@@ -224,7 +226,12 @@ 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) {
@@ -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 {