Hi!

I also think this would be great!

Well, we can always fix login to behave more robustly, no? :)

That's more or less what the attached patch does. It could certainly be
improved, as the knowledge of when to fallback is spread all over the
place, but that's an existing problem in the code anyway.

The SHELL variable in configure.in is changed to an explicit "/bin/sh"
because relying on $SHELL might change depending on the shell used for
configure, and the existing code expects /bin/sh for fallback and script
invokation cases, this could be considered a bug on its own though. The
only fishy point is when calling shell() with a second argument, which
will get preserved, and might not quite match what was invoked
afterwards, but probably not worth worrying.

The code could also warn that it needed to fallback to a POSIX shell,
but I'm not sure what's the policy from the shadow code PoV here.

Tested with:

  # chsh root -s /bin/csh
  chsh: Warning: /bin/csh does not exist
  # su
  # echo $SHELL
  /bin/sh
  # exit
  # su -
  # echo $SHELL
  /bin/sh
  # exit
  # login -f root
  Last login: Tue Apr  5 01:36:13 CEST 2011 on pts/10
  # echo $SHELL
  /bin/sh

And on a virtual console.

regards,
guillem

On Tue, 2011-04-05 at 01:49 +0200, Guillem Jover wrote:
[...]
[...]

This appears to open up any accounts that have been deliberately
disabled by setting their shell to a nonexistent path.  I know that's a
dumb way to disable an account, but that doesn't make this any less of a
security hole.

How about checking for the configured shell in /etc/shells before
enabling the fallback?

Ben.

Thanks for looking at this!  I'd definitely be happy to see a solution that
lets us shrink our Essential set without making the system less robust.

Yes, it seems to handle the missing-shell case.  What about the case of
execle() failing?  It's at least as likely for a shell to be broken because
its dependencies are not yet unpacked as it is that it's broken because the
shell itself is not unpacked.

Ah good catch! Done with the attached patch.

Well, ENOENT should handle not only missing executable, also missing
dynamic linker, etc. Maybe missing shared libraries depending on the
implementation, but on glibc systems, because those are handled by
the dynamic linker it's too late as the kernel has already handed over
control to it, which will fail hard. Equivalent to the case of the
shell binary segfaulting. Those cases would need to be handled
by forking and watching the childs (ugh :). In any case I've now
relaxed the ENOENT case to just execute the fallback when
apropriate on any execle() error, in case there's other error
instances which might make sense to fallback on.

But then bash only depends on libc and libncurses, which are
pseudo-essential, so if those and the dynamic linker are
non-functional then the system has bigger problems than root not
being able to login. For the unpack case you mention I guess it would
just be a matter of keeping those libraries in the Pre-Depends when
removing it from Essential.

Something else to do would be to add a versioned dependency to bash
on the fixed login package.

Would all this address your concerns?

regards,
guillem

we're proposing to change this in login, I'd like it to be as robust as we
can make it.

Also: libncurses is pseudo-essential, but the soname could of course change
in the future...  unpack new bash without first unpacking libncurses6 (if we
suppose we're *not* requiring bash to obey the usable-while-unpacked rule
which causes bash to currently pre-depend on its shlib deps), or unpack new
essential packages which force *removal* of libncurses5 in favor of
libncurses6, thus leaving bash unpacked yet broken, and a trap that catches
a failure to load shared libs becomes useful even for bash.

* Guillem Jover [2011-04-05 06:19 +0200]:

mksh.prerm contains:

    remove|upgrade|deconfigure)
        update-alternatives --remove ksh /bin/mksh
        update-alternatives --remove ksh /bin/mksh-static
        remove-shell /bin/mksh
        remove-shell /bin/mksh-static

bash.postrm contains:

    remove|purge|disappear)
        if which remove-shell >/dev/null && [ -f /etc/shells ]; then
            remove-shell /bin/bash
            remove-shell /bin/rbash
        fi

... so they are missing from /etc/shells after they have been removed.
Alternatives include a hardcoded list instead of relying on /etc/shells
or an additional file that contains all shells that were ever part of
/etc/shells.  prerm could also fail it the shell is set as root's (or
any, otherwise setups using sudo instead of root might break) login
shell.

Carsten

Ben Hutchings writes ("Re: Moving bash from essential/required to important?"):

Quite.

I don't think that's sufficient either.  I think this is just a bad
idea.

It might be OK if it were limited to some specified list of
nonexistent shells.

Ian.

Hi Guillem,

There did not seem to be a consensus in the last discussion.

Or did I miss any other discussions?

Best Regards,

So can we have a prerm script for bash that sets the root
shell back to /bin/sh, or at least asks the admin if they want
zsh or tcsh, and warns about any other users?

Any of this stuff of trying to have login figure out the
right shell seems like a new remote exploit in the making.

Hi,

It is too late for making changes related to this bug in Stretch. :-(
In the next cycle we will evaluate switching to login implementatiln in
util-linux per #833256. This bug may be solved by the switch or later in
util-linux.

Cheers,
Balint

[2017-01-21 20:54] Balint Reczey <balint@balintreczey.hu>

Hi! What is the current state of bug? There was fine (IMO) proposal,

	So can we have a prerm script for bash that sets the root
	shell back to /bin/sh, or at least asks the admin if they want
	zsh or tcsh, and warns about any other users?

but as bash=5.0-2 it did not make its way. What is missing? Should I
submit patch, implementing this proposal?

Hi Dmitry,

Dmitry Bogatov <KAction@debian.org> ezt írta (időpont: 2019. márc.
10., V, 20:13):

Only su moved to util-linux due to lack of time. :-(

I think submitting the patch against bash makes sense, but the timing
is unfortunate again, since the full freeze is about to start.
It bash gets patched after the release we can make it happen for Buster+1.

Cheers,
Balint

Hi Dmitry et al,

is there an open bug against bash for this?

Is there anything to be done in src:shadow for this at all?

I understand it was agreed to not patch shadow with a fallback for
an absent shell. Then, all that is to be done lies with bash?

Thanks,
Chris

Control: reassign -1 bash

Reassigning to bash, as TTBOMK the last thing to be done is for bash
to drop its Essential: yes flag and change its priority.

Chris