From: Omar Polo <op@omarpolo.com>
Subject: Re: gotd: support UIDs in the `user' directive
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Tue, 06 Aug 2024 14:33:30 +0200

On 2024/08/06 08:48:00 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Aug 05, 2024 at 06:37:32PM +0200, Omar Polo wrote:
> > This adds support to use both user ids in addition to user login names
> > in the `user' directive.
> > 
> > My first attempt was more like `connection limit user' is handled, i.e.
> > with gotd_parseuid(), except that I found awkward to parse /etc/passwd
> > twice, hence the user_name/user_id split in this diff.
> > 
> > thoughts/comments?
> 
> Special cases where either username or uid are not set will
> harm us one day.
> 
> I would feel better with having fully initialized user info
> (name and uid) available as early as possible, even if that
> means multiple passes over the password database. This won't
> be a performance problem.


here's a different/simpler diff.  instead of using got_parseuid() we can
defer that check in main(): this avoids both an extra lookup and
handling two different views on the user (uid and string).

I'm wondering if we should attempt first to parse the string as a number
to avoid an extra lookup in that case (i know it's not even remotely
performance sensitive, but it's still an extra lookup :P)

I'm also bundling the diff for permit/deny since the `user' one now
builds on top of it.

-----------------------------------------------
commit b6469cbbe47a6e20397941797603d9fd4c6f423b
from: Omar Polo <op@omarpolo.com>
date: Tue Aug  6 12:27:20 2024 UTC
 
 gotd: allow (as documented) numeric IDs in permit/deny
 
diff 14e517326fc34e0bd7c6b6141a72164f62d8d235 b6469cbbe47a6e20397941797603d9fd4c6f423b
commit - 14e517326fc34e0bd7c6b6141a72164f62d8d235
commit + b6469cbbe47a6e20397941797603d9fd4c6f423b
blob - 775f13f301bbb2743b0dd8d3cea68054ca32e779
blob + 01af5e5c4c08dcca08f25a7062b410af4dc78b2f
--- gotd/parse.y
+++ gotd/parse.y
@@ -133,6 +133,7 @@ typedef struct {
 %token	<v.string>	STRING
 %token	<v.number>	NUMBER
 %type	<v.tv>		timeout
+%type	<v.string>	numberstring
 
 %%
 
@@ -161,6 +162,15 @@ varset		: STRING '=' STRING	{
 		}
 		;
 
+numberstring	: STRING
+		| NUMBER {
+			if (asprintf(&$$, "%lld", (long long)$1) == -1) {
+				yyerror("asprintf: %s", strerror(errno));
+				YYERROR;
+			}
+		}
+		;
+
 timeout		: NUMBER {
 			if ($1 < 0) {
 				yyerror("invalid timeout: %lld", $1);
@@ -707,14 +717,14 @@ repoopts1	: PATH STRING {
 			}
 			free($2);
 		}
-		| PERMIT RO STRING {
+		| PERMIT RO numberstring {
 			if (gotd_proc_id == PROC_AUTH) {
 				conf_new_access_rule(new_repo,
 				    GOTD_ACCESS_PERMITTED, GOTD_AUTH_READ, $3);
 			} else
 				free($3);
 		}
-		| PERMIT RW STRING {
+		| PERMIT RW numberstring {
 			if (gotd_proc_id == PROC_AUTH) {
 				conf_new_access_rule(new_repo,
 				    GOTD_ACCESS_PERMITTED,
@@ -722,7 +732,7 @@ repoopts1	: PATH STRING {
 			} else
 				free($3);
 		}
-		| DENY STRING {
+		| DENY numberstring {
 			if (gotd_proc_id == PROC_AUTH) {
 				conf_new_access_rule(new_repo,
 				    GOTD_ACCESS_DENIED, 0, $2);

-----------------------------------------------
commit 37c4005e614b0786735d45f082b388b0cc9f747f (main)
from: Omar Polo <op@omarpolo.com>
date: Tue Aug  6 12:27:24 2024 UTC
 
 gotd: support UIDs in the user directive
 
diff b6469cbbe47a6e20397941797603d9fd4c6f423b 37c4005e614b0786735d45f082b388b0cc9f747f
commit - b6469cbbe47a6e20397941797603d9fd4c6f423b
commit + 37c4005e614b0786735d45f082b388b0cc9f747f
blob - d767e207751a606a2f1d4daf3666723e6fe5db5b
blob + e087575f78236937985aeb124917d2ff9962a9de
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -2007,6 +2007,7 @@ main(int argc, char **argv)
 	char *argv0 = argv[0];
 	char title[2048];
 	struct passwd *pw = NULL;
+	uid_t uid;
 	char *repo_path = NULL;
 	enum gotd_procid proc_id = PROC_GOTD;
 	struct event evsigint, evsigterm, evsighup, evsigusr1, evsigchld;
@@ -2016,6 +2017,7 @@ main(int argc, char **argv)
 	char hostname[HOST_NAME_MAX + 1];
 	FILE *diff_f1 = NULL, *diff_f2 = NULL;
 	int diff_fd1 = -1, diff_fd2 = -1;
+	const char *errstr;
 
 	TAILQ_INIT(&procs);
 
@@ -2080,6 +2082,11 @@ main(int argc, char **argv)
 		return 1;
 
 	pw = getpwnam(gotd.user_name);
+	if (pw == NULL) {
+		uid = strtonum(gotd.user_name, 0, UID_MAX - 1, &errstr);
+		if (errstr == NULL)
+			pw = getpwuid(uid);
+	}
 	if (pw == NULL)
 		fatalx("user %s not found", gotd.user_name);
 
blob - bd6a17f9a4698f159dcf27a9d7f9e2b96e89a9bd
blob + fd312076688c5cf8bb47bce4dc90bb1df44222db
--- gotd/gotd.conf.5
+++ gotd/gotd.conf.5
@@ -105,6 +105,7 @@ Afterwards,
 drops privileges to the specified
 .Ar user .
 If not specified, the user _gotd will be used.
+Numeric user IDs are also accepted.
 .El
 .Sh REPOSITORY CONFIGURATION
 At least one repository context must exist for
blob - 01af5e5c4c08dcca08f25a7062b410af4dc78b2f
blob + 01ee6087b16167393b859e94a1c702f7c64af765
--- gotd/parse.y
+++ gotd/parse.y
@@ -242,7 +242,7 @@ main		: LISTEN ON STRING {
 			}
 			free($3);
 		}
-		| USER STRING {
+		| USER numberstring {
 			if (strlcpy(gotd->user_name, $2,
 			    sizeof(gotd->user_name)) >=
 			    sizeof(gotd->user_name)) {