From: Stefan Sperling Subject: gotd chroot -> unveil To: gameoftrees@openbsd.org Date: Sun, 11 Dec 2022 14:42:15 +0100 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);