commit 0bcde4c8df9f0fc2d418667c5f91831a88a6a425 from: Stefan Sperling via: Thomas Adam date: Fri Dec 30 14:58:04 2022 UTC 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@ commit - 95ef3f8a77725d7ef0d173b0c0da5e8089ba0645 commit + 0bcde4c8df9f0fc2d418667c5f91831a88a6a425 blob - 57b17a1dd26385f232f8f989e4fa041babfeafe3 blob + fd7f8c11aa319b18aa9f13289797a9809af40003 --- gotd/auth.c +++ gotd/auth.c @@ -16,6 +16,7 @@ */ #include +#include #include #include @@ -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 - 5e2400783e338df5abf9a5f86a071d7494a5438b blob + 04db9fb475f2974cda1ee29f5a46ad6f1da68f2b --- gotd/gotd.c +++ gotd/gotd.c @@ -1224,8 +1224,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; @@ -1245,11 +1243,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"); @@ -1263,8 +1256,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); @@ -2307,14 +2301,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, @@ -2345,8 +2348,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; @@ -2561,7 +2567,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; @@ -2575,7 +2581,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 - 402de7d527bf413bd0c66dbf155fd68c7d6ef586 blob + 05ee51ee95afd632964c6b89345f6d87c0b8104b --- include/got_error.h +++ include/got_error.h @@ -184,6 +184,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 - 70e4070cea9cffbc893e8e23a355207fa4c15bd3 blob + a697bdae77a1c2923357d0dfd72de0abc30c2b67 --- lib/error.c +++ lib/error.c @@ -241,6 +241,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 {