#2541 PR closed: Avoid that *_ROOT_PASSWORD values are shown in debugscript mode

Labels: enhancement, bug

jsmeix opened issue at 2020-12-11 09:09:

  • Type: Bug Fix

  • Reference to related issue (URL):
    https://github.com/rear/rear/issues/2540

  • Brief description of the changes in this pull request:
    Use { confidential_command ; } 2>/dev/null
    cf. the comment for the UserInput function in
    lib/_input-output-functions.sh

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

Now things work reasonably well for me both
with SSH_ROOT_PASSWORD='qqqq' in etc/rear/local.conf
and without SSH_ROOT_PASSWORD in etc/rear/local.conf but with

# export SSH_ROOT_PASSWORD='qqqq'
# usr/sbin/rear -D mkrescue

that password is set in the recovery system
without a trace about its value in the log.

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

I like to sleep over it over the weekend and if it still looks OK to me on Monday
and if there are no objections I would like to merge it on Monday.

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

Two remarks from my point of view regarding SSH_ROOT_PASSWORD in default.conf:

  1. The comment advising against password exposure takes much time to read. In conjunction with other comments in default.conf it increases the risk of overlooking important hints. I'd very much favor a simple single-line comment in default.conf about possibly exposing passwords, and more elaborate explanations in the Wiki.
  2. A conditional initialization in default.conf could in principle be done for every variable. Unless there is a convincing use case, I would advise against this as it makes finding the variables in default.conf much harder. I don't see a sound use case for an environment variable SSH_ROOT_PASSWORD as long as one sticks with password hashes. Setting the variable setting outside of rear would just move the problem elsewhere. In the worst case, the password hash ends up in an unprivileged user's shell history files (SSH_ROOT_PASSWORD='...' sudo --preserve-env rear mkrescue).

schlomo commented at 2020-12-11 14:17:

We once had a principle that every variable must be initialized in default.conf(hence the name). I think that this is a good principle for us to keep. If - for whatever reason - configuration needs to be passed in via environment variables, then it should be done explicitly and those environment variables should be prefixed with REAR_ in my humble opinion.

About the increasing amount of comments: I also agree with the suggestion to keep those explanations somewhere else and would like to suggest the ReaR website for this. It is easily accessible.

jsmeix commented at 2020-12-11 14:37:

I will never ever fix outdated comments somewhere else because
I will basically always forget to search for stuff somewhere else.
I will leave that job to those who prefer the comments elsewhere.

Feel free to move or remove as many comments as you like
but then never ask me if you don't understand something.

Furthermore I will no longer deal with issues
where I do not easily understand the code "as is".
I will leave such issues completely to others.
Not my own code => not my problem.

OliverO2 commented at 2020-12-11 15:23:

@jsmeix
No one wants to make your life any harder, so let's try to find a solution that makes you happy, too.

We already have references to GitHub issues in the comments. So what if we'd have a comment referencing a section in the User Guide (doc/user-guide/03-configuration.adoc)? Such a section on how to set passwords could contain stuff like

  • Avoid plain text passwords.
  • Avoid exposing passwords in debugscript mode.
  • Prevent password hacking by generating cryptographically strong password hashes (prefer openssl passwd -6).
  • Remember proper quotes as password hashes contain $ characters.

In this case, a comment in default.conf could be like

# SSH_ROOT_PASSWORD defines a password for remote access to the recovery system as 'root' via SSH
# without requiring a public/private key pair. This password is valid only while the recovery system
# is running and will not allow access afterwards to the restored target system.
# SSH_ROOT_PASSWORD is ignored when SSH_FILES is set to a 'false' value.
# Please see the section 'Setting Passwords' in 'doc/user-guide/03-configuration.adoc'.
SSH_ROOT_PASSWORD=

As everything would be in the same repository, it would be searchable in one step. Would that make it work for you?

[Export of Github issue for rear/rear.]