On 2022/11/17 10:03:36 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> gotd group auth is broken. It matches against the groups of
> the user running gotd instead of the user which is connecting
> to the socket :-)

woops!

> Fix this and add tests for various cases
> where gotd should deny repository read access.
> 
> ok?

i've lost more time than i'd like to admit because i checked out a
fresh tree to try this and forgot to run `make obj' for gotd and
gotctl.  all the tests were failing... D:

diff reads fine, ok for me.  two comments below

> @@ -107,12 +112,22 @@ gotd_auth_check(struct gotd_access_rule_list *rules, c
>  
>  const struct got_error *
>  gotd_auth_check(struct gotd_access_rule_list *rules, const char *repo_name,
> -    gid_t *groups, int ngroups, uid_t euid, gid_t egid,
> -    int required_auth)
> +    uid_t euid, gid_t egid, int required_auth)
>  {
>  	struct gotd_access_rule *rule;
>  	enum gotd_access access = GOTD_ACCESS_DENIED;
> +	struct passwd *pw;
> +	gid_t groups[NGROUPS_MAX];
> +	int ngroups;

ngroups should be initialized to NGROUPS_MAX, isn't it?  I get "group
membership list truncated" otherwise, probably because luckily it gets
zeroe'd at -O0

> +	errno = 0; /* allow us to tell apart getpwuid does not clear */

I'm also not sure this is needed.

> +	pw = getpwuid(euid);
> +	if (pw == NULL)
> +		return got_error_from_errno("getpwuid");
> +
> +	if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1)
> +		log_warnx("group membership list truncated");
> +
>  	STAILQ_FOREACH(rule, rules, entry) {
>  		if (!match_identifier(rule->identifier, groups, ngroups,
>  		    euid, egid))