"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
gotd chroot -> unveil
To:
gameoftrees@openbsd.org
Date:
Sun, 11 Dec 2022 14:42:15 +0100

Download raw body.

Thread
This patch requires my "gotd listen process" patch to be applied first:
https://marc.gameoftrees.org/thread/1670581855.68945_0.html

Switch gotd from chroot(2) to unveil(2).

In the future, gotd will fork+exec new processes for each client connection.
Using unveil instead of chroot avoids having to start such processes as root.

The -portable version could use chroot(2) where no equivalent to unveil(2)
exists. A future component which starts new processes will be isolated as
a separate process, which could run as root in the -portable version.
 
diff 9efeb39aded750f79f59323a2a71b90ad6442bb8 89bbb2dad6f111b9c444bed968162bc3fc8f3b35
commit - 9efeb39aded750f79f59323a2a71b90ad6442bb8
commit + 89bbb2dad6f111b9c444bed968162bc3fc8f3b35
blob - 3c62987cb615e5a1580f36670da87e9eecf8ea1c
blob + 97731f1e9b245bfea3cec151f3d56b6c81205390
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -433,7 +433,7 @@ send_client_info(struct gotd_imsgev *iev, struct gotd_
 
 	proc = get_client_proc(client);
 	if (proc) {
-		if (strlcpy(iclient.repo_name, proc->chroot_path,
+		if (strlcpy(iclient.repo_name, proc->repo_path,
 		    sizeof(iclient.repo_name)) >= sizeof(iclient.repo_name)) {
 			return got_error_msg(GOT_ERR_NO_SPACE,
 			    "repo name too long");
@@ -904,7 +904,7 @@ recv_packfile(struct gotd_client *client)
 	pipe[1] = -1;
 
 	if (asprintf(&basepath, "%s/%s/receiving-from-uid-%d.pack",
-	    client->repo_write->chroot_path, GOT_OBJECTS_PACK_DIR,
+	    client->repo_write->repo_path, GOT_OBJECTS_PACK_DIR,
 	    client->euid) == -1) {
 		err = got_error_from_errno("asprintf");
 		goto done;
@@ -916,7 +916,7 @@ recv_packfile(struct gotd_client *client)
 
 	free(basepath);
 	if (asprintf(&basepath, "%s/%s/receiving-from-uid-%d.idx",
-	    client->repo_write->chroot_path, GOT_OBJECTS_PACK_DIR,
+	    client->repo_write->repo_path, GOT_OBJECTS_PACK_DIR,
 	    client->euid) == -1) {
 		err = got_error_from_errno("asprintf");
 		basepath = NULL;
@@ -1341,7 +1341,7 @@ gotd_shutdown(void)
 			proc = get_proc_for_pid(pid);
 			log_warnx("%s %s child process terminated; signal %d",
 			    proc ? gotd_proc_names[proc->type] : "",
-			    proc ? proc->chroot_path : "", WTERMSIG(status));
+			    proc ? proc->repo_path : "", WTERMSIG(status));
 		}	
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
 
@@ -1918,12 +1918,12 @@ gotd_dispatch(int fd, short event, void *arg)
 				disconnect(client);
 		} else {
 			if (do_packfile_install)
-				err = install_pack(client, proc->chroot_path,
+				err = install_pack(client, proc->repo_path,
 				    &imsg);
 			else if (do_ref_updates)
 				err = begin_ref_updates(client, &imsg);
 			else if (do_ref_update)
-				err = update_ref(client, proc->chroot_path,
+				err = update_ref(client, proc->repo_path,
 				    &imsg);
 			if (err)
 				log_warnx("uid %d: %s", client->euid, err->msg);
@@ -1941,7 +1941,7 @@ start_child(enum gotd_procid proc_id, const char *chro
 }
 
 static pid_t
-start_child(enum gotd_procid proc_id, const char *chroot_path,
+start_child(enum gotd_procid proc_id, const char *repo_path,
     char *argv0, const char *confpath, int fd, int daemonize, int verbosity)
 {
 	char	*argv[11];
@@ -1982,9 +1982,9 @@ start_child(enum gotd_procid proc_id, const char *chro
 	argv[argc++] = (char *)"-f";
 	argv[argc++] = (char *)confpath;
 
-	if (chroot_path) {
+	if (repo_path) {
 		argv[argc++] = (char *)"-P";
-		argv[argc++] = (char *)chroot_path;
+		argv[argc++] = (char *)repo_path;
 	}
 
 	if (!daemonize)
@@ -2038,16 +2038,16 @@ start_repo_children(struct gotd *gotd, char *argv0, co
 		    sizeof(proc->repo_name)) >= sizeof(proc->repo_name))
 			fatalx("repository name too long: %s", repo->name);
 		log_debug("adding repository %s", repo->name);
-		if (realpath(repo->path, proc->chroot_path) == NULL)
+		if (realpath(repo->path, proc->repo_path) == NULL)
 			fatal("%s", repo->path);
 		if (socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK,
 		    PF_UNSPEC, proc->pipe) == -1)
 			fatal("socketpair");
-		proc->pid = start_child(proc->type, proc->chroot_path, argv0,
+		proc->pid = start_child(proc->type, proc->repo_path, argv0,
 		    confpath, proc->pipe[1], daemonize, verbosity);
 		imsg_init(&proc->iev.ibuf, proc->pipe[0]);
 		log_debug("proc %s %s is on fd %d",
-		    gotd_proc_names[proc->type], proc->chroot_path,
+		    gotd_proc_names[proc->type], proc->repo_path,
 		    proc->pipe[0]);
 		proc->iev.handler = gotd_dispatch;
 		proc->iev.events = EV_READ;
@@ -2060,6 +2060,16 @@ apply_unveil(void)
 }
 
 static void
+apply_unveil_repo_readonly(const char *repo_path)
+{
+	if (unveil(repo_path, "r") == -1)
+		fatal("unveil %s", repo_path);
+
+	if (unveil(NULL, NULL) == -1)
+		fatal("unveil");
+}
+
+static void
 apply_unveil(void)
 {
 	struct gotd_repo *repo;
@@ -2215,11 +2225,7 @@ main(int argc, char **argv)
 			fatal("cannot listen on unix socket %s",
 			    gotd.unix_socket_path);
 		}
-		if (chroot(GOTD_EMPTY_PATH) == -1)
-			fatal("chroot");
-		if (chdir("/") == -1)
-			fatal("chdir(\"/\")");
-		if (daemonize && daemon(1, 0) == -1)
+		if (daemonize && daemon(0, 0) == -1)
 			fatal("daemon");
 	} else if (proc_id == PROC_REPO_READ || proc_id == PROC_REPO_WRITE) {
 		error = got_repo_pack_fds_open(&pack_fds);
@@ -2232,11 +2238,7 @@ main(int argc, char **argv)
 			fatalx("repository path not specified");
 		snprintf(title, sizeof(title), "%s %s",
 		    gotd_proc_names[proc_id], repo_path);
-		if (chroot(repo_path) == -1)
-			fatal("chroot");
-		if (chdir("/") == -1)
-			fatal("chdir(\"/\")");
-		if (daemonize && daemon(1, 0) == -1)
+		if (daemonize && daemon(0, 0) == -1)
 			fatal("daemon");
 	} else
 		fatal("invalid process id %d", proc_id);
@@ -2270,18 +2272,20 @@ main(int argc, char **argv)
 		break;
 	case PROC_REPO_READ:
 #ifndef PROFILE
-		if (pledge("stdio rpath recvfd", NULL) == -1)
+		if (pledge("stdio rpath recvfd unveil", NULL) == -1)
 			err(1, "pledge");
 #endif
-		repo_read_main(title, pack_fds, temp_fds);
+		apply_unveil_repo_readonly(repo_path);
+		repo_read_main(title, repo_path, pack_fds, temp_fds);
 		/* NOTREACHED */
 		exit(0);
 	case PROC_REPO_WRITE:
 #ifndef PROFILE
-		if (pledge("stdio rpath recvfd", NULL) == -1)
+		if (pledge("stdio rpath recvfd unveil", NULL) == -1)
 			err(1, "pledge");
 #endif
-		repo_write_main(title, pack_fds, temp_fds);
+		apply_unveil_repo_readonly(repo_path);
+		repo_write_main(title, repo_path, pack_fds, temp_fds);
 		/* NOTREACHED */
 		exit(0);
 	default:
blob - 4ba88670ecbb2552a329c63ded99527718ffdca1
blob + 5227ab25941ce1ee09f25658ae621317a7cc6341
--- gotd/gotd.conf.5
+++ gotd/gotd.conf.5
@@ -58,10 +58,7 @@ requires root privileges in order to create its unix s
 .Xr gotd 8 .
 Initially,
 .Xr gotd 8
-requires root privileges in order to create its unix socket and start
-child processes in a
-.Xr chroot 2
-environment.
+requires root privileges in order to create its unix socket.
 Afterwards,
 .Xr gotd 8
 drops privileges to the specified
blob - 9ec3939ca07f41efabccdf15c8ed41e0a606fe86
blob + f7524de7c95a08df6542dacc78d3295c249be091
--- gotd/gotd.h
+++ gotd/gotd.h
@@ -50,7 +50,7 @@ struct gotd_child_proc {
 	pid_t pid;
 	enum gotd_procid type;
 	char repo_name[NAME_MAX];
-	char chroot_path[PATH_MAX];
+	char repo_path[PATH_MAX];
 	int pipe[2];
 	struct gotd_imsgev iev;
 	size_t nhelpers;
blob - 349b5768badc4d88d707fe79a442572a9762b141
blob + 4716f4da690117859980910c5c467ea48b7fa838
--- gotd/repo_read.c
+++ gotd/repo_read.c
@@ -858,7 +858,8 @@ repo_read_main(const char *title, int *pack_fds, int *
 }
 
 void
-repo_read_main(const char *title, int *pack_fds, int *temp_fds)
+repo_read_main(const char *title, const char *repo_path,
+    int *pack_fds, int *temp_fds)
 {
 	const struct got_error *err = NULL;
 	struct gotd_imsgev iev;
@@ -870,11 +871,7 @@ repo_read_main(const char *title, int *pack_fds, int *
 
 	arc4random_buf(&clients_hash_key, sizeof(clients_hash_key));
 
-	/*
-	 * Open a repository in the root directory.
-	 * We are already in chroot at this point.
-	 */
-	err = got_repo_open(&repo_read.repo, "/", NULL, pack_fds);
+	err = got_repo_open(&repo_read.repo, repo_path, NULL, pack_fds);
 	if (err)
 		goto done;
 	if (!got_repo_is_bare(repo_read.repo)) {
blob - 8dc9ffd3453f593a9b4aae0566ce57742feb0f97
blob + f936d91926478e395e4051103bd86870838922a5
--- gotd/repo_read.h
+++ gotd/repo_read.h
@@ -14,5 +14,5 @@ void repo_read_main(const char *, int *, int *);
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-void repo_read_main(const char *, int *, int *);
+void repo_read_main(const char *, const char *, int *, int *);
 void repo_read_shutdown(void);
blob - abed0bcfbf6fbf7acff490cac29ee2e12f6207cc
blob + 2b06c2f7956001962fa2df8ef028f9f1ac519b4f
--- gotd/repo_write.c
+++ gotd/repo_write.c
@@ -1391,7 +1391,8 @@ repo_write_main(const char *title, int *pack_fds, int 
 }
 
 void
-repo_write_main(const char *title, int *pack_fds, int *temp_fds)
+repo_write_main(const char *title, const char *repo_path,
+    int *pack_fds, int *temp_fds)
 {
 	const struct got_error *err = NULL;
 	struct gotd_imsgev iev;
@@ -1403,11 +1404,7 @@ repo_write_main(const char *title, int *pack_fds, int 
 
 	arc4random_buf(&clients_hash_key, sizeof(clients_hash_key));
 
-	/*
-	 * Open a repository in the root directory.
-	 * We are already in chroot at this point.
-	 */
-	err = got_repo_open(&repo_write.repo, "/", NULL, pack_fds);
+	err = got_repo_open(&repo_write.repo, repo_path, NULL, pack_fds);
 	if (err)
 		goto done;
 	if (!got_repo_is_bare(repo_write.repo)) {
blob - 7e5a7a9839979cbe97d2571a61acbc00cc378898
blob + cb5ff4a606c537ef026d2f095e10c280b2ebe87b
--- gotd/repo_write.h
+++ gotd/repo_write.h
@@ -14,5 +14,5 @@ void repo_write_main(const char *, int *, int *);
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-void repo_write_main(const char *, int *, int *);
+void repo_write_main(const char *, const char *, int *, int *);
 void repo_write_shutdown(void);