Bug#802211: RFC: wip patch to force sulogin on locked root accounts

[Felipe Sateler]

On Thu, Oct 12, 2017 at 7:07 PM, Stijn van Drongelen <rhymoid at gmail.com> wrote:
> Hi,
>
>> TL;DR...
>
> Yes, I knew about that risk. But shorter messages have failed to draw
> attention to a supposedly "important" bug.
>
> I expected that the original patch was good to go, but I somehow forgot
> that a 720+ day old patch is subject to bitrot. Please correct me if
> I'm wrong, but wouldn't we now need (at least) three patches now to fix
> this problem?
>
> * We have to patch src/sulogin-shell/rescue.service.in and
>   src/sulogin-shell/emergency.service.in in version 232 (the version in
>   stretch).
> * We have to patch src/sulogin-shell/systemd-sulogin-shell.in in
>   version 234 (the version currently in stretch-backports and buster).
> * We have to patch src/sulogin-shell/sulogin-shell.c in version 235
>   (the version currently in sid).
>
> I can probably find some more time in the coming week to contribute
> to a solution

Excellent. I would suggest though to fix this first for 235, and
upstream. That way whatever solution is implemented for stretch[1] can
be designed with compatibility with buster in mind.


[1] If the release team ACKs the change, too.

>. I do have a few comments in advance:
>
> 1) I want to stress that I believe that the only correct solution is
>    an unconditional --force. Patches that do just that do not reduce
>    the boot security of any plausible installation in any way, and are
>    also much easier to implement correctly. The points below can be
>    mostly ignored if we do this.
> 2) The "You are in rescue/emergency mode" message has to change
>    depending on the setting: the mentioned login prompt won't be there
>    when using --force.

It is my understanding that sulogin --force will still ask for
password if getpwnam works.

> 3) I think it's much simpler to build two separate binaries (one that
>    forces, one that doesn't) instead of dynamically adjusting
>    a command line and a message based on the value of an environment
>    variable.

This has the drawback of requiring to modify ExecStart, and thus risk
becoming incompatible if the sulogin wrapper changes interface.

> 4) If you think it's likely (not merely possible) that supplying other
>    options to sulogin will become necessary, then I think it makes more
>    sense to pass sulogin-shell's arguments to sulogin and use getenv()
>    to decide the label of the mode (rescue/emergency/potato), rather
>    than the other way around.
>

I don't have an opinion here.

-- 

Saludos,
Felipe Sateler