#1500 PR closed: Avoid leaking unprotected SSH private key files onto rescue medium¶

Labels: won't fix / can't fix / obsolete

OliverO2 opened issue at 2017-09-18 17:32:¶

Note: This is not about logging into the rescue system via SSH (incoming access). It is about accessing other systems from a running rescue system via SSH private keys stored on the rescue medium (outgoing access).

Problem¶

The 'root' account on the original system may contain SSH private key files without passphrase protection. Such unprotected private key files are typically used to access other systems during unattended operations (e.g. automatic backup to a network server). This kind of access can be useful during restore, too.

Security then depends entirely on keeping such private keys secret, which might be compromised if unprotected private key files leak onto an unencrypted rescue medium. Anyone with physical access to the rescue medium would be allowed unauthenticated access to each account on the network authorizing one of those private keys.

Solution¶

Avoid leaking SSH private key files onto the rescue medium in unprotected form:

This PR enables ReaR to copy the SSH client configuration for 'root' with the option of passphrase-protecting otherwise unprotected private keys. The user can choose between

• copying previously unprotected private keys with added passphrase-protection,
• copying previously unprotected private keys as-is (with a proper warning),
• not copying unprotected private keys at all.

Private key files which already contain a passphrase are assumed to be sufficiently secure and are copied as-is.

The PR also aims to optimize usability during 'rear recover' by asking the user just once for any required SSH passphrase (using ssh-agent).

Tested on Ubuntu 16.04.3 LTS.

OliverO2 commented at 2017-09-19 08:59:¶

@schlomo @jsmeix
Lots of thanks to both of you for reviews and suggestions.

Before investing any further effort in UserInput let me just address the latest comments. Hopefully, we can really keep rear mkrescue non-interactive without compromising security. I might have a viable idea, so just stay tuned.

jsmeix commented at 2017-09-19 12:09:¶

Via https://github.com/rear/rear/pull/1501
there is a new confidential mode (via '-C')
for the UserInput function that does not log
possibly confidential data.

jsmeix commented at 2017-09-19 13:31:¶

I did not follow all the details here only a side note
regarding the comment from @schlomo
https://github.com/rear/rear/pull/1500#discussion_r139680488

not copying unprotected SSH keys unless
specifically configured to do so

@OliverO2
in general no confidential stuff should be copied
into the ReaR recovery system unless the user
specifically configured ReaR to do so, cf.
https://github.com/rear/rear/pull/1472#issuecomment-328459748

jsmeix commented at 2017-09-20 10:01:¶

Via https://github.com/rear/rear/pull/1505 plus the fix in
https://github.com/rear/rear/commit/0f5220ec76dd68a061b55d30cccde5f8aa8505bb
the UserInput function supports now the 'read' options
'-r' for raw input and
'-s' to not echo input from a terminal on that terminal
plus a better description how to use the confidential UserInput mode
in particular an example how to let the user enter a password
that is (hopefully - unless I forgot something) now really quiet

{ password="$( UserInput -I PASSWORD -C -r -s -p 'Enter the pasword' )" ; } 2>/dev/null

where also in debugscripts mode nothing appears in the log,
at least as it seems for me:
I (mis)use for such tests init/default/030_update_recovery_system.sh
because that is an 'init' script that is always and early run.

e205:~/rear.master # head usr/share/rear/init/default/030_update_recovery_system.sh

{ password="$( UserInput -I PASSWORD -C -r -s -p 'Enter the pasword' )" ; } 2>/dev/null
echo -En "$password" | od -a 1>&7

UserInput -I WAIT -t 0

Error "test exit"

# Updating the currently running system.

e205:~/rear.master # export USER_INPUT_PASSWORD='my \t password'

e205:~/rear.master # usr/sbin/rear -d -D mkrescue
Relax-and-Recover 2.2 / Git
Using log file: /root/rear.master/var/log/rear/rear-e205.log
UserInput -I PASSWORD needed in /root/rear.master/usr/share/rear/init/default/030_update_recovery_system.sh line 2
Enter the pasword
(timeout 300 seconds)
UserInput: Will use predefined input in USER_INPUT_PASSWORD
Hit any key to interrupt the automated input (timeout 5 seconds)
0000000   m   y  sp   \   t  sp   p   a   s   s   w   o   r   d
0000016
UserInput -I WAIT needed in /root/rear.master/usr/share/rear/init/default/030_update_recovery_system.sh line 5
enter your input

UserInput: Neither real user input nor real default input (both empty or only spaces) results ''
ERROR: test exit
Aborting due to an error, check /root/rear.master/var/log/rear/rear-e205.log for details
Terminated

e205:~/rear.master # less /root/rear.master/var/log/rear/rear-e205.log
...
+ source /root/rear.master/usr/share/rear/init/default/030_update_recovery_system.sh
++ od -a
++ echo -En 'my \t password'
++ UserInput -I WAIT -t 0
...

OliverO2 commented at 2017-09-20 13:25:¶

@schlomo

Sorry, it seems we have reached a point where I am not going to pursue this PR any further. Let me just clarify:

1. We seem to have completely different philosophies regarding how to deal with security vulnerabilities and how to avoid them:

   • While ReaR normally cherry-picks every bit that should go into the recovery system, you now argue that the potentially vulnerable /root/.ssh directory contents should go there completely by default.
   • I think it's not safe to assume that administrators, when changing stuff in /root/.ssh, are fully aware of the consequences in conjunction with ReaR or that they even think about this once ReaR was set up.
   • I think it's not safe to assume that administrators have other protection measures in place. Secrets allowing complete access to sensitive information will end up on unencrypted USB sticks ready for anyone to pick up - an attacker's paradise.
   • So I argue that secrets and other potentially vulnerable configuration files should not be transferred by default. Administrators should decide individually after assessing their specific situation (and probably after thorough consultation with security experts).
   • I simply don't have the time and the necessary in-depth qualification to assess the security implications of every conceivable ssh configuration option, so I'd rather not seduce anyone by providing risky options. I like to play safe and stick to a reasonable minimum with (hopefully) well known consequences.

2. In my view, we are losing focus and wasting time. Examples (all in 500_ssh.sh unless otherwise stated):

   • I had changed the introductory if into has_binary ssh || has_binary sshd || return on your request. This was unrelated to the PR's objective, changed indentation everywhere, and the situation got worse after merging a conflicting commit into master.
   • Issues are being discussed which are unrelated to the code actually changed by this PR:
     • CLONE_USERS
     • splitting it up into prep and build stage (the file contained code writing to $ROOTFS_DIR before)
   • Issues are being discussed which are of too little value to spend time on now, for example:
     • When there is ISO_RECOVER_MODE, PXE_RECOVER_MODE and LANG_RECOVER, SSH_PRIVATE_KEYS_RECOVER_PASSPHRASE (named after the phase where it is used) should be just OK.
     • The strings call in 005_ssh_agent_start.sh might not be strictly required, but makes sense when you don't want to research grep's implementation details (it's meant to be a text-line-based tool) and the extra call doesn't hurt at all.
     • Using cat instead of $(< file), escpecially when it's meant to deliver an empty string if the file doesn't exist, just works. It was just an example in a comment, after all. And opinions on whether $(< file) is actually immediately understood by non-bash-experts looking at the configuration seems questionable.
   • I don't say that these things couldn't be discussed but extensive discussions of questionable value are not what I'd call contributor-friendly. I just value my time more than what I think we're getting out of it. And while I'm happy to improve some other details on-the-fly, I did not intend to do a complete overhaul of 500_ssh.sh.

So if you have very specific ideas of what the code should look like, that's absolutely fine. I'd say just take my ideas and research from this PR and implement it the way you like.

And finally, thanks to you, to the other contributors for creating ReaR, and especially to @jsmeix for being so helpful.

OliverO2 commented at 2017-09-22 19:11:¶

Topic now tracked in issue #1511.

---

[Export of Github issue for rear/rear.]