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

Miloslav Trmac
2013-01-14T18:16:18Z

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