From: Stefan Sperling Subject: resolve account name of users connecting to gotd To: gameoftrees@openbsd.org Date: Tue, 26 Dec 2023 17:54:30 +0100 Pass the account name of successfully authenticated users from the gotd auth process to the gotd parent process. This can be used to make log messages more readable. With this patch we display the username instead of a UID when reporting successful authentication in the logs. We could expand upon this later and tweak other messages which currently display the UID. But I actually wrote this patch because account name information will be required to provide meaningful commit notifications. One quirk which deviates from our usual style is that auth_check() will not free the *username pointer on failure. I chose to free it in the caller instead to keep the existing auth_check() implementation mostly as it is, since the early returns are an important part of the code flow. Alternatively, we could do another getpwuid() call in the caller but a call to strdup() on the existing passwd object is likely cheaper. There is a small drive-by fix in here which ensures that the message GOTD_IMSG_ACCESS_GRANTED will only be accepted once. ok? ----------------------------------------------- make the gotd auth process provide the user's account name for later use diff 65d7451409bf603e3c302b7d0ce999f7ce542508 74e961a7f170feb5e543f1b173c216f3552c9020 commit - 65d7451409bf603e3c302b7d0ce999f7ce542508 commit + 74e961a7f170feb5e543f1b173c216f3552c9020 blob - b463648c859a1713d622a349df2197f561d994bd blob + 507f067ba3ec02ecf529ea76c99c79938efae1ab --- gotd/auth.c +++ gotd/auth.c @@ -128,8 +128,8 @@ match_identifier(const char *identifier, gid_t *groups } 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) +auth_check(char **username, struct gotd_access_rule_list *rules, + const char *repo_name, uid_t euid, gid_t egid, int required_auth) { struct gotd_access_rule *rule; enum gotd_access access = GOTD_ACCESS_DENIED; @@ -137,6 +137,8 @@ auth_check(struct gotd_access_rule_list *rules, const gid_t groups[NGROUPS_MAX]; int ngroups = NGROUPS_MAX; + *username = NULL; + pw = getpwuid(euid); if (pw == NULL) { if (errno) @@ -145,6 +147,10 @@ auth_check(struct gotd_access_rule_list *rules, const return got_error_set_errno(EACCES, repo_name); } + *username = strdup(pw->pw_name); + if (*username == NULL) + return got_error_from_errno("strdup"); + if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) log_warnx("group membership list truncated"); @@ -178,6 +184,9 @@ recv_authreq(struct imsg *imsg, struct gotd_imsgev *ie size_t datalen; uid_t euid; gid_t egid; + char *username = NULL; + size_t len; + const size_t maxlen = MAX_IMSGSIZE - IMSG_HEADER_SIZE; log_debug("authentication request received"); @@ -200,18 +209,23 @@ recv_authreq(struct imsg *imsg, struct gotd_imsgev *ie 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); + err = auth_check(&username, &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; + goto done; } + len = strlen(username); + if (len > maxlen) + len = maxlen; + 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; + PROC_AUTH, -1, (void *)username, len) == -1) + err = got_error_from_errno("imsg compose ACCESS_GRANTED"); +done: + free(username); + return err; } static void blob - 892a54d71ecfa0e0dc9b036bdc334a9cbb1e4225 blob + 0c6944a273b5b1885488a4dd1d44002cea5cb70e --- gotd/gotd.c +++ gotd/gotd.c @@ -96,6 +96,7 @@ struct gotd_client { struct event tmo; uid_t euid; gid_t egid; + char *username; struct gotd_child_proc *repo; struct gotd_child_proc *auth; struct gotd_child_proc *session; @@ -368,6 +369,7 @@ disconnect(struct gotd_client *client) close(client->fd); else if (client->iev.ibuf.fd != -1) close(client->iev.ibuf.fd); + free(client->username); free(client); client_cnt--; } @@ -1126,6 +1128,7 @@ gotd_dispatch_auth_child(int fd, short event, void *ar struct imsg imsg; uint32_t client_id = 0; int do_disconnect = 0; + size_t datalen; client = find_client_by_proc_fd(fd); if (client == NULL) { @@ -1169,13 +1172,18 @@ gotd_dispatch_auth_child(int fd, short event, void *ar evtimer_del(&client->tmo); + datalen = imsg.hdr.len - IMSG_HEADER_SIZE; + 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: - client->state = GOTD_CLIENT_STATE_ACCESS_GRANTED; + if (client->state != GOTD_CLIENT_STATE_NEW) { + do_disconnect = 1; + err = got_error(GOT_ERR_PRIVSEP_MSG); + } break; default: do_disconnect = 1; @@ -1188,14 +1196,25 @@ gotd_dispatch_auth_child(int fd, short event, void *ar 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); + imsg_free(&imsg); return; + } else { + client->state = GOTD_CLIENT_STATE_ACCESS_GRANTED; + if (datalen > 0) + client->username = strndup(imsg.data, datalen); + if (client->username == NULL && + asprintf(&client->username, "uid %d", client->euid) == -1) { + err = got_error_from_errno("asprintf"); + imsg_free(&imsg); + goto done; + } + imsg_free(&imsg); } repo = gotd_find_repo_by_name(client->auth->repo_name, &gotd); @@ -1205,8 +1224,8 @@ gotd_dispatch_auth_child(int fd, short event, void *ar } kill_auth_proc(client); - log_info("authenticated uid %d for repository %s", - client->euid, repo->name); + log_info("authenticated %s for repository %s", + client->username, repo->name); err = start_session_child(client, repo, gotd.argv0, gotd.confpath, gotd.daemonize, gotd.verbosity);