From: Stefan Sperling Subject: gotd: run authentication in separate process To: gameoftrees@openbsd.org Date: Tue, 27 Dec 2022 16:02:12 +0100 Run authentication in a separate child process, removing the "getpw" pledge promise from the parent. This patch applies on top of my previous patch to fork repo children on demand. diff f015a20bf2366e80c0d25eb8448eed577c148784 a4b6708ab0e570f77974656765a7767f03cdd125 commit - f015a20bf2366e80c0d25eb8448eed577c148784 commit + a4b6708ab0e570f77974656765a7767f03cdd125 blob - ed5bbd87051a58c687b26b3585c2a1e3714d65f3 blob + 57b17a1dd26385f232f8f989e4fa041babfeafe3 --- gotd/auth.c +++ gotd/auth.c @@ -25,18 +25,52 @@ #include #include #include +#include #include #include #include +#include #include #include #include "got_error.h" +#include "got_path.h" #include "gotd.h" #include "log.h" #include "auth.h" +static struct gotd_auth { + pid_t pid; + const char *title; + struct gotd_repo *repo; +} gotd_auth; + +static void auth_shutdown(void); + +static void +auth_sighdlr(int sig, short event, void *arg) +{ + /* + * Normal signal handler rules don't apply because libevent + * decouples for us. + */ + + switch (sig) { + case SIGHUP: + break; + case SIGUSR1: + break; + case SIGTERM: + case SIGINT: + auth_shutdown(); + /* NOTREACHED */ + break; + default: + fatalx("unexpected signal"); + } +} + static int parseuid(const char *s, uid_t *uid) { @@ -109,8 +143,8 @@ const struct got_error * return 1; } -const struct got_error * -gotd_auth_check(struct gotd_access_rule_list *rules, const char *repo_name, +static const struct got_error * +auth_check(struct gotd_access_rule_list *rules, const char *repo_name, uid_t euid, gid_t egid, int required_auth) { struct gotd_access_rule *rule; @@ -150,3 +184,138 @@ gotd_auth_check(struct gotd_access_rule_list *rules, c /* should not happen, this would be a bug */ return got_error_msg(GOT_ERR_NOT_IMPL, "bad access rule"); } + +static const struct got_error * +recv_authreq(struct imsg *imsg, struct gotd_imsgev *iev) +{ + const struct got_error *err; + struct imsgbuf *ibuf = &iev->ibuf; + struct gotd_imsg_auth iauth; + size_t datalen; + + log_debug("authentication request received"); + + datalen = imsg->hdr.len - IMSG_HEADER_SIZE; + if (datalen != sizeof(iauth)) + return got_error(GOT_ERR_PRIVSEP_LEN); + + memcpy(&iauth, imsg->data, datalen); + + err = auth_check(&gotd_auth.repo->rules, gotd_auth.repo->name, + iauth.euid, iauth.egid, iauth.required_auth); + if (err) { + gotd_imsg_send_error(ibuf, PROC_AUTH, iauth.client_id, err); + return err; + } + + if (gotd_imsg_compose_event(iev, GOTD_IMSG_ACCESS_GRANTED, + PROC_AUTH, -1, NULL, 0) == -1) + return got_error_from_errno("imsg compose ACCESS_GRANTED"); + + return NULL; +} + +static void +auth_dispatch(int fd, short event, void *arg) +{ + const struct got_error *err = NULL; + struct gotd_imsgev *iev = arg; + struct imsgbuf *ibuf = &iev->ibuf; + struct imsg imsg; + ssize_t n; + int shut = 0; + + if (event & EV_READ) { + if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN) + fatal("imsg_read error"); + if (n == 0) /* Connection closed. */ + shut = 1; + } + + if (event & EV_WRITE) { + n = msgbuf_write(&ibuf->w); + if (n == -1 && errno != EAGAIN) + fatal("msgbuf_write"); + if (n == 0) /* Connection closed. */ + shut = 1; + } + + for (;;) { + if ((n = imsg_get(ibuf, &imsg)) == -1) + fatal("%s: imsg_get", __func__); + if (n == 0) /* No more messages. */ + break; + + switch (imsg.hdr.type) { + case GOTD_IMSG_AUTHENTICATE: + err = recv_authreq(&imsg, iev); + if (err) + log_warnx("%s: %s", gotd_auth.title, err->msg); + break; + default: + log_debug("%s: unexpected imsg %d", gotd_auth.title, + imsg.hdr.type); + break; + } + + imsg_free(&imsg); + } + + if (!shut) { + gotd_imsg_event_add(iev); + } else { + /* This pipe is dead. Remove its event handler */ + event_del(&iev->ev); + event_loopexit(NULL); + } +} + +void +auth_main(const char *title, struct gotd_repolist *repos, + const char *repo_path) +{ + struct gotd_repo *repo = NULL; + struct gotd_imsgev iev; + struct event evsigint, evsigterm, evsighup, evsigusr1; + + gotd_auth.title = title; + gotd_auth.pid = getpid(); + TAILQ_FOREACH(repo, repos, entry) { + if (got_path_cmp(repo->path, repo_path, + strlen(repo->path), strlen(repo_path)) == 0) + break; + } + if (repo == NULL) + fatalx("repository %s not found in config", repo_path); + gotd_auth.repo = repo; + + signal_set(&evsigint, SIGINT, auth_sighdlr, NULL); + signal_set(&evsigterm, SIGTERM, auth_sighdlr, NULL); + signal_set(&evsighup, SIGHUP, auth_sighdlr, NULL); + signal_set(&evsigusr1, SIGUSR1, auth_sighdlr, NULL); + signal(SIGPIPE, SIG_IGN); + + signal_add(&evsigint, NULL); + signal_add(&evsigterm, NULL); + signal_add(&evsighup, NULL); + signal_add(&evsigusr1, NULL); + + imsg_init(&iev.ibuf, GOTD_FILENO_MSG_PIPE); + iev.handler = auth_dispatch; + iev.events = EV_READ; + iev.handler_arg = NULL; + event_set(&iev.ev, iev.ibuf.fd, EV_READ, auth_dispatch, &iev); + if (event_add(&iev.ev, NULL) == -1) + fatalx("event add"); + + event_dispatch(); + + auth_shutdown(); +} + +static void +auth_shutdown(void) +{ + log_debug("%s: shutting down", gotd_auth.title); + exit(0); +} blob - 506f5e8b84ac2182f7114a5f4569b3b8d6963cc2 blob + fe78e2c9301e0c052e89de790159fb9afaeb5094 --- gotd/auth.h +++ gotd/auth.h @@ -14,6 +14,6 @@ const struct got_error * * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -const struct got_error * -gotd_auth_check(struct gotd_access_rule_list *rules, const char *repo_name, - uid_t euid, gid_t egid, int required_auth); +void +auth_main(const char *title, struct gotd_repolist *repos, + const char *repo_path); blob - 3040877532eed07c6a4bd0d310bf4b9564ceab06 blob + 047f9d4ecb86187d413c4cf02096cccd7398217f --- gotd/gotd.c +++ gotd/gotd.c @@ -82,6 +82,8 @@ struct gotd_client { gid_t egid; struct gotd_child_proc *repo_read; struct gotd_child_proc *repo_write; + struct gotd_child_proc *auth; + int required_auth; char *packfile_path; char *packidx_path; int nref_updates; @@ -98,6 +100,8 @@ static void kill_proc(struct gotd_child_proc *, int); static void gotd_shutdown(void); static const struct got_error *start_repo_child(struct gotd_client *, enum gotd_procid, struct gotd_repo *, char *, const char *, int, int); +static const struct got_error *start_auth_child(struct gotd_client *, int, + struct gotd_repo *, char *, const char *, int, int); static void kill_proc(struct gotd_child_proc *, int); __dead static void @@ -248,6 +252,8 @@ find_client_by_proc_fd(int fd) struct gotd_child_proc *proc = get_client_proc(c); if (proc && proc->iev.ibuf.fd == fd) return c; + if (c->auth && c->auth->iev.ibuf.fd == fd) + return c; } } @@ -315,19 +321,16 @@ wait_for_children(pid_t child_pid) } static void -wait_for_children(pid_t child_pid) +wait_for_child(pid_t child_pid) { pid_t pid; int status; - if (child_pid == 0) - log_debug("waiting for children to terminate"); - else - log_debug("waiting for child PID %ld to terminate", - (long)child_pid); + log_debug("waiting for child PID %ld to terminate", + (long)child_pid); do { - pid = wait(&status); + pid = waitpid(child_pid, &status, WNOHANG); if (pid == -1) { if (errno != EINTR && errno != ECHILD) fatal("wait"); @@ -339,6 +342,25 @@ disconnect(struct gotd_client *client) } static void +kill_auth_proc(struct gotd_client *client) +{ + struct gotd_child_proc *proc; + + if (client->auth == NULL) + return; + + proc = client->auth; + client->auth = NULL; + + event_del(&proc->iev.ev); + msgbuf_clear(&proc->iev.ibuf.w); + close(proc->iev.ibuf.fd); + kill_proc(proc, 0); + wait_for_child(proc->pid); + free(proc); +} + +static void disconnect(struct gotd_client *client) { struct gotd_imsg_disconnect idisconnect; @@ -348,6 +370,8 @@ disconnect(struct gotd_client *client) log_debug("uid %d: disconnecting", client->euid); + kill_auth_proc(client); + idisconnect.client_id = client->id; if (proc) { if (gotd_imsg_compose_event(&proc->iev, @@ -358,7 +382,7 @@ disconnect(struct gotd_client *client) msgbuf_clear(&proc->iev.ibuf.w); close(proc->iev.ibuf.fd); kill_proc(proc, 0); - wait_for_children(proc->pid); + wait_for_child(proc->pid); free(proc); proc = NULL; } @@ -607,15 +631,11 @@ start_client_session(struct gotd_client *client, struc repo = find_repo_by_name(ireq.repo_name); if (repo == NULL) return got_error(GOT_ERR_NOT_GIT_REPO); - err = gotd_auth_check(&repo->rules, repo->name, - client->euid, client->egid, GOTD_AUTH_READ); + err = start_auth_child(client, GOTD_AUTH_READ, repo, + gotd.argv0, gotd.confpath, gotd.daemonize, + gotd.verbosity); if (err) return err; - err = start_repo_child(client, PROC_REPO_READ, repo, - gotd.argv0, gotd.confpath, gotd.daemonize, - gotd.verbosity); - if (err) - return err; } else { err = ensure_client_is_not_reading(client); if (err) @@ -623,18 +643,15 @@ start_client_session(struct gotd_client *client, struc repo = find_repo_by_name(ireq.repo_name); if (repo == NULL) return got_error(GOT_ERR_NOT_GIT_REPO); - err = gotd_auth_check(&repo->rules, repo->name, client->euid, - client->egid, GOTD_AUTH_READ | GOTD_AUTH_WRITE); + err = start_auth_child(client, + GOTD_AUTH_READ | GOTD_AUTH_WRITE, + repo, gotd.argv0, gotd.confpath, gotd.daemonize, + gotd.verbosity); if (err) return err; - err = start_repo_child(client, PROC_REPO_WRITE, repo, - gotd.argv0, gotd.confpath, gotd.daemonize, - gotd.verbosity); - if (err) - return err; } - /* List-refs request will be forwarded once the child is ready. */ + /* Flow continues upon authentication successs/failure or timeout. */ return NULL; } @@ -1281,6 +1298,7 @@ static const char *gotd_proc_names[PROC_MAX] = { static const char *gotd_proc_names[PROC_MAX] = { "parent", "listen", + "auth", "repo_read", "repo_write" }; @@ -1311,7 +1329,7 @@ gotd_shutdown(void) msgbuf_clear(&proc->iev.ibuf.w); close(proc->iev.ibuf.fd); kill_proc(proc, 0); - wait_for_children(proc->pid); + wait_for_child(proc->pid); log_info("terminating"); exit(0); @@ -1407,6 +1425,15 @@ verify_imsg_src(struct gotd_client *client, struct got } else ret = 1; break; + case GOTD_IMSG_ACCESS_GRANTED: + if (proc->type != PROC_AUTH) { + err = got_error_fmt(GOT_ERR_BAD_PACKET, + "authentication of uid %d from PID %d " + "which is not the auth process", + proc->pid, client->euid); + } else + ret = 1; + break; case GOTD_IMSG_REPO_CHILD_READY: if (proc->type != PROC_REPO_READ && proc->type != PROC_REPO_WRITE) { @@ -1906,6 +1933,120 @@ gotd_dispatch_repo_child(int fd, short event, void *ar } static void +gotd_dispatch_auth_child(int fd, short event, void *arg) +{ + const struct got_error *err = NULL; + struct gotd_imsgev *iev = arg; + struct imsgbuf *ibuf = &iev->ibuf; + struct gotd_client *client; + struct gotd_repo *repo = NULL; + ssize_t n; + int shut = 0; + struct imsg imsg; + uint32_t client_id = 0; + int do_disconnect = 0; + enum gotd_procid proc_type; + + client = find_client_by_proc_fd(fd); + if (client == NULL) + fatalx("cannot find client for fd %d", fd); + + if (client->auth == NULL) + fatalx("cannot find auth child process for fd %d", fd); + + if (event & EV_READ) { + if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN) + fatal("imsg_read error"); + if (n == 0) { + /* Connection closed. */ + shut = 1; + goto done; + } + } + + if (event & EV_WRITE) { + n = msgbuf_write(&ibuf->w); + if (n == -1 && errno != EAGAIN) + fatal("msgbuf_write"); + if (n == 0) { + /* Connection closed. */ + shut = 1; + } + goto done; + } + + if (client->auth->iev.ibuf.fd != fd) + fatalx("%s: unexpected fd %d", __func__, fd); + + if ((n = imsg_get(ibuf, &imsg)) == -1) + fatal("%s: imsg_get error", __func__); + if (n == 0) /* No more messages. */ + return; + + evtimer_del(&client->tmo); + + switch (imsg.hdr.type) { + case GOTD_IMSG_ERROR: + do_disconnect = 1; + err = gotd_imsg_recv_error(&client_id, &imsg); + break; + case GOTD_IMSG_ACCESS_GRANTED: + break; + default: + do_disconnect = 1; + log_debug("unexpected imsg %d", imsg.hdr.type); + break; + } + + if (!verify_imsg_src(client, client->auth, &imsg)) { + do_disconnect = 1; + log_debug("dropping imsg type %d from PID %d", + imsg.hdr.type, client->auth->pid); + } + imsg_free(&imsg); + + if (do_disconnect) { + if (err) + disconnect_on_error(client, err); + else + disconnect(client); + goto done; + } + + repo = find_repo_by_name(client->auth->repo_name); + if (repo == NULL) { + err = got_error(GOT_ERR_NOT_GIT_REPO); + goto done; + } + kill_auth_proc(client); + + log_info("authenticated uid %d for repository %s\n", + client->euid, repo->name); + + if (client->required_auth & GOTD_AUTH_WRITE) + proc_type = PROC_REPO_WRITE; + else + proc_type = PROC_REPO_READ; + + err = start_repo_child(client, proc_type, repo, + gotd.argv0, gotd.confpath, gotd.daemonize, + gotd.verbosity); +done: + if (err) + log_warnx("uid %d: %s", client->euid, err->msg); + + /* We might have killed the auth process by now. */ + if (client->auth != NULL) { + if (!shut) { + gotd_imsg_event_add(iev); + } else { + /* This pipe is dead. Remove its event handler */ + event_del(&iev->ev); + } + } +} + +static void gotd_dispatch_repo_child(int fd, short event, void *arg) { struct gotd_imsgev *iev = arg; @@ -2058,6 +2199,9 @@ start_child(enum gotd_procid proc_id, const char *repo case PROC_LISTEN: argv[argc++] = (char *)"-L"; break; + case PROC_AUTH: + argv[argc++] = (char *)"-A"; + break; case PROC_REPO_READ: argv[argc++] = (char *)"-R"; break; @@ -2153,6 +2297,59 @@ static void return NULL; } +static const struct got_error * +start_auth_child(struct gotd_client *client, int required_auth, + struct gotd_repo *repo, char *argv0, const char *confpath, + int daemonize, int verbosity) +{ + struct gotd_child_proc *proc; + struct gotd_imsg_auth iauth; + struct timeval auth_timeout = { 5, 0 }; + + memset(&iauth, 0, sizeof(iauth)); + + proc = calloc(1, sizeof(*proc)); + if (proc == NULL) + return got_error_from_errno("calloc"); + + proc->type = PROC_AUTH; + if (strlcpy(proc->repo_name, repo->name, + sizeof(proc->repo_name)) >= sizeof(proc->repo_name)) + fatalx("repository name too long: %s", repo->name); + log_debug("starting auth for uid %d repository %s", + client->euid, repo->name); + 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->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->repo_path, + proc->pipe[0]); + proc->iev.handler = gotd_dispatch_auth_child; + proc->iev.events = EV_READ; + proc->iev.handler_arg = NULL; + event_set(&proc->iev.ev, proc->iev.ibuf.fd, EV_READ, + gotd_dispatch_auth_child, &proc->iev); + gotd_imsg_event_add(&proc->iev); + + iauth.euid = client->euid; + iauth.egid = client->egid; + 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) + log_warn("imsg compose AUTHENTICATE"); + + client->auth = proc; + client->required_auth = required_auth; + evtimer_add(&client->tmo, &auth_timeout); + return NULL; +} + static void apply_unveil_repo_readonly(const char *repo_path) { @@ -2202,8 +2399,11 @@ main(int argc, char **argv) log_init(1, LOG_DAEMON); /* Log to stderr until daemonized. */ - while ((ch = getopt(argc, argv, "df:LnP:RvW")) != -1) { + while ((ch = getopt(argc, argv, "Adf:LnP:RvW")) != -1) { switch (ch) { + case 'A': + proc_id = PROC_AUTH; + break; case 'd': daemonize = 0; break; @@ -2322,6 +2522,11 @@ main(int argc, char **argv) } if (daemonize && daemon(0, 0) == -1) fatal("daemon"); + } else if (proc_id == PROC_AUTH) { + snprintf(title, sizeof(title), "%s %s", + gotd_proc_names[proc_id], repo_path); + 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); if (error != NULL) @@ -2352,7 +2557,7 @@ main(int argc, char **argv) switch (proc_id) { case PROC_GOTD: #ifndef PROFILE - if (pledge("stdio rpath wpath cpath proc exec getpw " + if (pledge("stdio rpath wpath cpath proc exec " "sendfd recvfd fattr flock unix unveil", NULL) == -1) err(1, "pledge"); #endif @@ -2365,6 +2570,14 @@ main(int argc, char **argv) listen_main(title, fd); /* NOTREACHED */ break; + case PROC_AUTH: +#ifndef PROFILE + if (pledge("stdio getpw", NULL) == -1) + err(1, "pledge"); +#endif + auth_main(title, &gotd.repos, repo_path); + /* NOTREACHED */ + break; case PROC_REPO_READ: #ifndef PROFILE if (pledge("stdio rpath recvfd unveil", NULL) == -1) blob - c9660ad1b2468eb23cf0f8fbd3bb1937ac6e8449 blob + c1090adb4ecfd2e2c122cf9a6074590b01e560dd --- gotd/gotd.h +++ gotd/gotd.h @@ -33,6 +33,7 @@ enum gotd_procid { enum gotd_procid { PROC_GOTD = 0, PROC_LISTEN, + PROC_AUTH, PROC_REPO_READ, PROC_REPO_WRITE, PROC_MAX, @@ -178,6 +179,10 @@ enum gotd_imsg_type { /* Child process management. */ GOTD_IMSG_REPO_CHILD_READY, + + /* Auth child process. */ + GOTD_IMSG_AUTHENTICATE, + GOTD_IMSG_ACCESS_GRANTED, }; /* Structure for GOTD_IMSG_ERROR. */ @@ -411,6 +416,14 @@ int parse_config(const char *, enum gotd_procid, struc uint32_t client_id; }; +/* Structure for GOTD_IMSG_AUTHENTICATE. */ +struct gotd_imsg_auth { + uid_t euid; + gid_t egid; + int required_auth; + uint32_t client_id; +}; + int parse_config(const char *, enum gotd_procid, struct gotd *); /* imsg.c */ blob - f3ea139f65f7d8cf2b35c0d63a1ba9fc62aef542 blob + 64e720e2de27b8b2d1a4ab147d28cdac86cd6db0 --- gotd/parse.y +++ gotd/parse.y @@ -182,7 +182,8 @@ repository : REPOSITORY STRING { } } - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_GOTD || + gotd_proc_id == PROC_AUTH) { new_repo = conf_new_repo($2); } free($2); @@ -198,7 +199,8 @@ repository : REPOSITORY STRING { } } - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_GOTD || + gotd_proc_id == PROC_AUTH) { new_repo = conf_new_repo($2); } free($2); @@ -207,7 +209,8 @@ repoopts1 : PATH STRING { ; repoopts1 : PATH STRING { - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_GOTD || + gotd_proc_id == PROC_AUTH) { if (!got_path_is_absolute($2)) { yyerror("%s: path %s is not absolute", __func__, $2); @@ -225,20 +228,20 @@ repoopts1 : PATH STRING { free($2); } | PERMIT RO STRING { - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_AUTH) { conf_new_access_rule(new_repo, GOTD_ACCESS_PERMITTED, GOTD_AUTH_READ, $3); } } | PERMIT RW STRING { - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_AUTH) { conf_new_access_rule(new_repo, GOTD_ACCESS_PERMITTED, GOTD_AUTH_READ | GOTD_AUTH_WRITE, $3); } } | DENY STRING { - if (gotd_proc_id == PROC_GOTD) { + if (gotd_proc_id == PROC_AUTH) { conf_new_access_rule(new_repo, GOTD_ACCESS_DENIED, 0, $2); }