Re: [PATCH 1/7] util: Drop any supplementary group before dropping privileges

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

On Mon, Aug 29, 2022 at 05:17:03PM +0200, Stefano Brivio wrote:
> Commit a951e0b9efcb ("conf: Add --runas option, changing to given UID
> and GID if started as root") dropped the call to initgroups() that
> used to add supplementary groups corresponding to the user we'll
> eventually run as -- we don't need those.
> 
> However, if the original user belongs to supplementary groups
> (usually not the case, if started as root), we don't drop those,
> now, and rpmlint says:
> 
>   passt.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/passt
>   passt.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/passt.avx2
> 
> Add a call to setgroups() with an empty set, to drop any
> supplementary group we might currently have, before changing GID
> and UID.
> 
> Reported-by: Daniel P. Berrangé <berrange(a)redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>

I'll keep this in mind for the rework I plan in this area.

> ---
>  util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util.c b/util.c
> index 9b87b65..7e10deb 100644
> --- a/util.c
> +++ b/util.c
> @@ -525,7 +525,7 @@ void check_root(struct ctx *c)
>  #endif
>  	}
>  
> -	if (!setgid(c->gid) && !setuid(c->uid))
> +	if (!setgroups(0, NULL) && !setgid(c->gid) && !setuid(c->uid))
>  		return;
>  
>  	fprintf(stderr, "Can't change user/group, exiting");

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]