Re: [PATCH v2 3/4] chsh: Add libuser support

[Miloslav Trmac <mitr@redhat.com>]

Hello,
----- Original Message -----
> diff --git a/login-utils/libuser.c b/login-utils/libuser.c
> new file mode 100644
> index 0000000..9a3e3b1
> --- /dev/null
> +++ b/login-utils/libuser.c
> +static int auth_lu(const char *service_name, struct lu_context *ctx,
> uid_t uid,
> +			const char *username) {
> +	if(!lu_uses_elevated_privileges(ctx)) {
> +		/* Drop privileges */
> +		if (setegid(getgid()) == -1) {
> +			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
> +			return FALSE;
> +		}
> +		if (seteuid(getuid()) == -1) {
> +			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
> +			return FALSE;
> +		}
> +		if (initgroups(username, 0)) {
> +			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
> +			return FALSE;
> +		}

I can't see how this can work: after seteuid() the program no longer has root privileges (CAP_SETGID) to change the groups.

(On second thought, if the program is setuid, the setuid execution mechanism doesn't change supplementary groups, so perhaps the call isn't strictly necessary; Still, initializing the groups makes the environment more deterministic.  And as long as initgroups() is called, it should be called in a way that works.)

> +	lu_ent_set_string(ent, attr, val);
> +	if (!lu_user_modify(ctx, ent, &error)) {
> +		lu_ent_free(ent);
> +		lu_end(ctx);
> +		err(EXIT_FAILURE, _("Shell not changed: %s\n"),
> lu_strerror(error));

This error message is fine for chsh, but not great for chfn.
    Mirek