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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
resolve account name of users connecting to gotd
To:
gameoftrees@openbsd.org
Date:
Tue, 26 Dec 2023 17:54:30 +0100

Download raw body.

Thread
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);