multiuser disabled - fail more gracefully

[Kazuo Kuroi]

Hello Geoff,

That's a good suggestion. but I suggest that if your code can't run on 
UNIX platforms that it would need an include guard against it. Matt has 
taken a lot of time to ensure this not only runs on proprietary old C 
compilers, but also on older OSes where OpenSSH is not a good option.

I only express personal hesitancy here because I had to, along with 
another dev, meddle greatly in svr-chansession to allow it to run on 
IRIX properly - and I'm mildly concerned your proposed change could 
introduce linuxisms if it's not include guarded.

I would thus ask that any changes are first included on a Linux include 
guard, and then we could test it on other platforms like IRIX or AIX 
before we remove it.

Just my .02 as the sorta-kinda maintainer for IRIX.

- Kaz Kuroi

On 3/9/2021 5:18 AM, Geoff Winkless wrote:
> Hi
>
> I appreciate that there's an compile-time option
> DROPBEAR_SVR_MULTIUSER=0 to skip the setuid/gid sections, but can I
> make a humble suggestion that we fail gracefully if someone* runs a
> dropbear that _doesn't_ have that option configured on a linux kernel
> that's compiled single-user.
>
> *Not me, of course, I would obviously never do anything that stupid...
>
> The idea being, if you're running as root and logging in as root, it
> shouldn't matter if the setuid stuff fails, so for example In
> svr-chansession.c, something like:
>
>          /* We can only change uid/gid as root ... */
>          if (getuid() == 0) {
>
> -                if ((setgid(ses.authstate.pw_gid) < 0) ||
> +                if ((setgid(ses.authstate.pw_gid) < 0 &&
> ses.authstate.pw_uid != 0) ||
>                         (initgroups(ses.authstate.pw_name,
>                                                  ses.authstate.pw_gid) < 0)) {
>                          dropbear_exit("Error changing user group");
>                  }
> -                if (setuid(ses.authstate.pw_uid) < 0 &&
> ses.authstate.pw_uid != 0) {
> +                if (setuid(ses.authstate.pw_uid) < 0 &&
> ses.authstate.pw_uid != 0) {
>                         dropbear_exit("Error changing user");
>                  }
>          } else {
>
> There are a few other places (probably everywhere where SVR_MULTIUSER
> is checked, I suppose) where the same principle could be applied.
>
> Geoff