#2539 PR merged: Add terminal password check via 'TTY_ROOT_PASSWORD'

Labels: enhancement, fixed / solved / done

OliverO2 opened issue at 2020-12-10 22:24:

Pull Request Details:

  • Type: New Feature

  • Impact: Normal

  • Reference to related issue (URL):

  • How was this pull request tested? On Ubuntu 20.04 LTS Server

  • Brief description of the changes in this pull request:

Currently, ReaR allows a password check on SSH sessions only (via SSH_ROOT_PASSWORD).

This PR enables the rescue/recovery system to enforce a password check also on terminal connections (the system console and/or serial terminals).

This prevents unauthenticated local root access

  • while the rescue/recovery system is being run via a remote SSH session, or
  • when a system reboot has inadvertently started the recovery system installed on a local disk partition (e.g. where a buggy firmware has inadvertently reshuffled the boot order).
Usage
  1. Configure TTY_ROOT_PASSWORD in the same way as SSH_ROOT_PASSWORD.
  2. Run rear mkrescue.
  3. Start the rescue/recovery system.
  4. Log in locally, providing the correct password.
Example Configuration

Configure the password test:

TTY_ROOT_PASSWORD='$6$SixRXJl0b37m4rl3$alfGpFd3dp1tBk5/EDquyxqiviB2oQQq3z7VULx9qiBHweFlBquivq28.I4YNLknhKjDax1uM/4c2xuVzdNcy1'
SSH_ROOT_PASSWORD="$TTY_ROOT_PASSWORD"

jsmeix commented at 2020-12-11 13:53:

@OliverO2
again many thanks for your continuous enhancements!

Only an offhanded idea:

I didn't had sufficient free mind-space to have a closer look
so I could misunderstand things but I think what this is about is
to also have a password dialog for the "normal" login on the
ReaR recovery system console (where currently no password is needed).
If I am right I appreciate this option to make things more secure very much!

Do you think an automatism to have TTY_ROOT_PASSWORD and
SSH_ROOT_PASSWORD somehow synchronized would be good?

Offhandedly I think about something like:
If one is set and the other one is empty
automatically set the empty one to the one that is set, e.g.

SSH_ROOT_PASSWORD='mypw'
TTY_ROOT_PASSWORD=''

results that TTY_ROOT_PASSWORD is also set to be 'mypw'
and vice versa.
The current default that both are empty would not change the behaviour.
Only with SSH_ROOT_PASSWORD='mypw' one would get automatically
also that same password to login at the recovery system console.
Probably that could get in the way for automated recovery.
So it must be possible for the user to have SSH_ROOT_PASSWORD set
but tell ReaR to not set TTY_ROOT_PASSWORD.
This could be done with TTY_ROOT_PASSWORD='no' and
is_false $TTY_ROOT_PASSWORD ... to enforce no TTY_ROOT_PASSWORD
and vice versa for SSH_ROOT_PASSWORD.
This would make the password 'no' impossible but such a password is nonsense anyway.

It seems my automated sync idea gets too complicated.

OliverO2 commented at 2020-12-11 14:18:

@jsmeix
I had a similar idea: Having separate passwords depending on the terminal connection used does not make much sense. However, that's the way ReaR grew up and some people might currently depend on it. I did not want to change too much in one step, so basically the next logical thing for me would be:

  • Add a variable LOGIN_ROOT_PASSWORD (initially empty).
  • Use the value of LOGIN_ROOT_PASSWORD as a fallback for TTY_ROOT_PASSWORD and SSH_ROOT_PASSWORD if these are unset/empty.

This way one can have all combinations with LOGIN_ROOT_PASSWORD being the preferred one.

gdha commented at 2020-12-14 11:13:

@OliverO2 @jsmeix IMHO I would go for 1 variable REAR_ROOT_PASSWORD to distinguish between the real root password and the one we define for the ReaR image. For backwards compatibility I would keep SSH_ROOT_PASSWORD for the time being, but in de default.conf file we should mention it will become deprecated in favour of REAR_ROOT_PASSWORD which should be used in the same way foor SSH connections or TTY login sessions. Why do you think about thei proposal?

OliverO2 commented at 2020-12-14 13:40:

@gdha The motivation for having a name REAR_ROOT_PASSWORD sounds reasonable. There is one caveat, though: As @schlomo described in https://github.com/rear/rear/pull/2541#issuecomment-743217373, a prefix of REAR_ might suggest that this is an environment variable. If ReaR would allow that, user's might be tempted to use something like REAR_ROOT_PASSWORD='...' sudo --preserve-env rear mkrescue, which would make the password end up in their shell command history and in ps output for others to see.

So maybe go for RECOVERY_ROOT_PASSWORD instead? Additionally, we should probably follow best practices and stop supporting plain text passwords (those are currently allowed for SSH_ROOT_PASSWORD).

schlomo commented at 2020-12-14 15:20:

About the env names in my opinion 2 things are important:

  1. all officially recognized env variables start from REAR_ to make it visible for which program they are used, similar to ls expecting LS_COLOR or ssh expecting SSH_* variables
  2. env variables that should be read by default (if they exist) should have the same name (plus the REAR_ prefix) as used internally

Why? To make it simple for end-users to understand what is going on.

Furthermore, if we ever want to have a generic env variable based configuration mechanism then we can just iterate over all REAR_* variables and import them (or do we already have this feature?)

What do you think?

OliverO2 commented at 2020-12-14 15:37:

So if I understand correctly, there would be

  • SOME_FUNNY_OPTION used throughout ReaR with a standard initialization in default.conf, and
  • REAR_SOME_FUNNY_OPTION as an optional environment variable, which would initialize SOME_FUNNY_OPTION and have priority over possible settings of SOME_FUNNY_OPTION in *.conf files.

If so, that sounds good to me.

Any other opinions on environment variables containing secrets? And the final naming of the password variable in this case?

schlomo commented at 2020-12-14 16:52:

Yes, like that.

However I am not sure about the priority question: Should env variables always override configuration settings? Or should it only override the built-in defaults?

Probably good to look at other software and follow that.

OliverO2 commented at 2020-12-14 21:29:

I have no good precedent at hand, but if I did a test invocation like this

REAR_OUTPUT_URL=file:///my/test/destination rear mkrescue

and found out that this would not override local.conf or site.conf, I'd be pretty surprised.

So I'd say an environment variable is like a command line parameter: An explicit setting for a single invocation that must always be honored. Configuration files must not overturn such explicit user decisions.

gdha commented at 2021-01-26 13:42:

@OliverO2 Are we good to merge this PR?
@jsmeix Any objections against it?

OliverO2 commented at 2021-01-26 14:13:

@gdha Yes, this is ready to merge as is. Once we agree on some common name unifying TTY_ROOT_PASSWORD and SSH_ROOT_PASSWORD and on whether to stop using plaintext passwords (https://github.com/rear/rear/pull/2539#issuecomment-744447047), we could do that in a subsequent PR.

jsmeix commented at 2021-01-27 07:59:

I do fully agree with
https://github.com/rear/rear/pull/2539#issuecomment-767567952
I prefer small steps that are doable and get them done.

[Export of Github issue for rear/rear.]