Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve cryptographic security and user-friendliness for LUKS volumes #1493

Merged
merged 4 commits into from Oct 4, 2017

Conversation

OliverO2
Copy link
Contributor

Problem

ReaR 2.2 with cryptsetup 1.6.6 (as on Ubuntu 16.04.3 LTS) uses compiled-in default values which are not up-to-date in terms of cryptographic security:

Option cryptsetup 1.6.6 Default More Secure Auto-detectable
Key size --key-size 256 --key-size 512 Yes
Passphrase processing time (msecs) --iter-time 1000 --iter-time 2000 No
Random number generation device --use-urandom --use-random No

For details, see Encryption options for LUKS mode in the ArchWiki.

In addition, ReaR can neither auto-detect nor use existing keyfiles for the automatic decryption of recreated LUKS volumes. A recovered system will ask the user for a passphrase where the original system would not.

Solution

This PR improves LUKS recovery with these characteristics:

  • Auto-detect the --key-size option from the original volume.
  • Auto-detect keyfiles in crypttab and add them to recreated LUKS volumes.
  • Allow additional cryptsetup options, which cannot be auto-detected, to be configured.

Strategy

See comment in commit c9e7e1b.

Note: Removing the documented mode option is an incompatible change, which simplifies the code by processing each option in the same way. Since disklayout.conf is usually re-generated before being edited, introducing this change seems OK to me.

Tested on Ubuntu 16.04.3 LTS.

Change crypt entry in disklayout.conf:
- Remove 'mode' option, absorb its value into 'cipher'.
- Remove undocumented 'key' option.
- Add options 'key_size' and 'keyfile'.
- Document 'password' option.

Change default.conf:
- Add 'LUKS_CRYPTSETUP_OPTIONS' variable with cryptographcially
  up-to-date default settings.

Auto-detect 'key_size' from original LUKS volume and apply as cryptsetup
option '--key-size'.

Auto-detect 'keyfile' option, enable unattended (password-less)
decryption via temporary keyfile without leaking original keyfiles on
rescue medium, securely re-assign keyfiles after restore.
@jsmeix jsmeix added the enhancement Adaptions and new features label Sep 15, 2017
@jsmeix
Copy link
Member

jsmeix commented Sep 15, 2017

@OliverO2 only FYI
you may have a look at the related issue
#1444

@gozora
I know nothing at all about LUKS so that it is bad luck for you ;-)
that you are curious about how ReaR works with LUKS
#1444 (comment)
so that I dare to assign this one to you :-)

@jsmeix
Copy link
Member

jsmeix commented Sep 15, 2017

@OliverO2
as far as I see in your comments in your
usr/share/rear/finalize/GNU/Linux/240_reassign_luks_keyfiles.sh

original keyfiles should have been restored from the backup

it means that with this pull request nothing secret
will be added into the ReaR recovery system?
At least not by default without an explicit user setting?
Cf.
#1472 (comment)
and
#1444 (comment)

@OliverO2
Copy link
Contributor Author

OliverO2 commented Sep 15, 2017

@jsmeix
Yes, no secrets will be added to the rescue medium as stated in the comment of commit c9e7e1b:

Auto-detect 'keyfile' option, enable unattended (password-less)
decryption via temporary keyfile without leaking original keyfiles on
rescue medium, securely re-assign keyfiles after restore.

Also see this comment in 160_include_luks_code.sh.

When you have encrypted disks and you leak secrets onto the unencrypted rescue medium it is like building an expensive underground bank vault and then putting the key into a tin box on your desk. So I just went the extra mile to avoid this.

I agree with #1472 (comment) and below.

My next task is actually to provide a ReaR option to encrypt ssh keys which

  • currently leak onto ReaR's rescue media, and
  • may lack passphrase-protection for unattended operations, and
  • may provide access to the central backup repository.

The idea incorporates asking the user for an initial recovery master password (resembling #1444 (comment)), although I would not recommend putting this into configuration files.

@jsmeix
Copy link
Member

jsmeix commented Sep 15, 2017

@OliverO2
many thanks for your explanation.

As you can also see in
#1489 (comment)
encryption stuff is not one of my favourite areas of interest.
Accordingly I do very much appreciate it when you check
and even improve those things in ReaR.
I look forward to your ssh improvements.
As long as in etc/rear/local.conf even a dumb

SSH_ROOT_PASSWORD="rear"

still "just works" to "just get" ssh access into the recovery system
I am happy because I use that all the time on my testing systems.
( I do not what to have the "good old times" back when one had
to do special setup to get ssh access into the recovery system. ;-)

Copy link
Member

@gozora gozora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all let me apologize for taking me so long to reply to this PR, but I've somehow missed it :-(.
For me implementing automatic volume unlocking is somehow politic decision, and as I don't like politics I let @schlomo decide whether something like this should or should not be implemented. (c.f. #1444 (comment)).

@OliverO2 this PR is broken for systems that don't use automatic unlocking of volumes (which is completely valid setup)

As man crypttab states:

The third field specifies the encryption password. If the field is not present or the password is set to "none" or "-", the password has to be manually entered during system boot. Otherwise, the field is interpreted as a absolute path to a file containing the encryption password. For swap encryption, /dev/urandom or the hardware device /dev/hw_random can be used as the password file; using /dev/random may prevent boot completion if the system does not have enough entropy to generate a truly random encryption key.

I've tested your PR with following crypttab:

luks-0ef6a905-e6a2-4a3d-92ab-caa541f9306a UUID=0ef6a905-e6a2-4a3d-92ab-caa541f9306a none 
Cdata1 UUID=acd42bd9-53d0-41b5-8d31-f03d320d2b75 /root/keyfile1 luks

With following result during recovery:

++ echo '2017-09-23 21:18:05.163296443 Re-assigning keyfile none to LUKS device /dev/sda2'
2017-09-23 21:18:05.163296443 Re-assigning keyfile none to LUKS device /dev/sda2
+++ basename none
++ temp_keyfile=/tmp/LUKS-keyfile-none
++ '[' -f /tmp/LUKS-keyfile-none ']'
++ target_keyfile=/mnt/local/none
++ '[' -f /mnt/local/none ']'
+++ dirname /mnt/local/none
++ mkdir -p /mnt/local
++ cp -p /tmp/LUKS-keyfile-none /mnt/local/none
++ rm /tmp/LUKS-keyfile-none
++ StopIfError 'Could not restore keyfile none for LUKS device /dev/sda2 from temporary keyfile'
++ ((  0 != 0  ))
++ LogPrintError 'none was not restored - LUKS device /dev/sda2 has been assigned a new keyfile'
++ Log 'none was not restored - LUKS device /dev/sda2 has been assigned a new keyfile'

As you might have guessed, this resulted to unbootable OS.

V.

# may set LUKS_CRYPTSETUP_OPTIONS="--iter-time 2000 --use-urandom" instead, but be aware that this produces a
# low-quality master encryption key. For details, see the cryptsetup(8) manual page.
LUKS_CRYPTSETUP_OPTIONS="--iter-time 2000 --use-random"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoring virtual machines lacks entropy, so it might happen that during restore of VM ReaR will just "sit and wait" ..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Secure by default, but may not cover every use case. User should re-configure LUKS_CRYPTSETUP_OPTIONS in these cases. Whether it's a good idea to use encryption inside entropy-lacking environments is another question.

# 'finalize' stage.
# The scheme for generating a temporary keyfile path must be the same here and in the 'finalize' stage.
keyfile="${TMPDIR:-/tmp}/LUKS-keyfile-$(basename $keyfile)"
dd bs=512 count=4 if=/dev/urandom of="$keyfile"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've stated in default.conf:

but be aware that this produces a
low-quality master encryption key. For details, see the cryptsetup(8) manual page.

I think that having disks encrypted with key created by /dev/urandom (in case that $target_keyfile does not exist) should be advertised to user as well...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disks are encrypted via a different key: This master encryption key still remains at high quality if LUKS_CRYPTSETUP_OPTIONS is used in its default configuration (--use-random).

/dev/urandom is used here to generate what's basically a very long (2k) temporary password, which remains valid only until the original keyfile will be restored and re-assigned. If this doesn't succeed, the "temporary password" becomes permanent and the following message is issued:

LogPrintError "$original_keyfile was not restored - LUKS device $device has been assigned a new keyfile"

I would hope that this gives the user a sufficient warning for this improbable case, possibly resulting from a non-functional crypttab configuration anyway. Would you say even more advice is needed?

Log "Re-assigning keyfile $original_keyfile to LUKS device $device"

# The scheme for generating a temporary keyfile path must be the same here and in the 'layout/prepare' stage.
temp_keyfile="${TMPDIR:-/tmp}/LUKS-keyfile-$(basename $original_keyfile)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I've used following keys for encryption:

/secure_dir1/key1
/secure_dir2/key1

Maybe I've misunderstood something, but wouldn't this mean that disks encryption will be done by same key, when in reality we've used two keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Good catch. Will have to correct this. I happen to use a configuration where key files are kept in the same directoriy.

@OliverO2
Copy link
Contributor Author

@gozora

For me implementing automatic volume unlocking is somehow politic decision, and as I don't like politics I let @schlomo decide whether something like this should or should not be implemented. (c.f. #1444 (comment)).

Isn't that the original system administrator's decision that we just recreate during recovery? He has finally decided to go with crypttab keyfiles after all. Do you think that this decision should be overruled by ReaR?

this PR is broken for systems that don't use automatic unlocking of volumes (which is completely valid setup)

You are right. I'll correct this. Can you point me to the manpage you've used? It seems to be a rather old version of the cryptsetup suite as newer versions (like this one) don't seem to allow verbatim passwords there anymore.

@gozora
Copy link
Member

gozora commented Sep 25, 2017

Isn't that the original system administrator's decision that we just recreate during recovery? He has finally decided to go with crypttab keyfiles after all. Do you think that this decision should be overruled by ReaR?

Like I said, for me this is politics, I don't like politics. ReaR is my spare time project and I don't do thing I don't like in my spare time.
So again, if @schlomo does not object neither me will object.

You are right. I'll correct this. Can you point me to the manpage you've used? It seems to be a rather old version of the cryptsetup suite as newer versions (like this one) don't seem to allow verbatim passwords there anymore.

I don't think so.

Excerpt from http://manpages.ubuntu.com/manpages/xenial/en/man5/crypttab.5.html:

   It can also be a device name (e.g. /dev/urandom), note however that
   LUKS requires a persistent key and therefore does not support random
   data keys.

   If the key file is the string “none”, a passphrase will be read
   interactively from the console. In this case, the options precheck,
   check, checkargs and tries may be useful.

But to answer your question I've used man crypttab from CentOS 7.3

@OliverO2
Copy link
Contributor Author

OK, I misunderstood this one:

The third field specifies the encryption password.

It initially sounded to me like is was meant to be a verbatim password first, and a path to a keyfile only under certain circumstances. When reading more carefully, it was never really meant to be a password. So they have just improved the explanation in newer incarnations of that manual page.

Anyway, I'll take care of the special cases of 'none' and '-'.

@jsmeix
Copy link
Member

jsmeix commented Sep 26, 2017

A "caution!" regarding running any programs in ReaR
that may (sometimes) do interactive user dialogs like
"a passphrase will be read interactively from the console":

In ReaR interactive user dialogs won't work by default because
both STDERR and STDOUT are redirected into ReaR's log file,
cf. "What to do with stdout and stderr" at
https://github.com/rear/rear/wiki/Coding-Style

@OliverO2
Copy link
Contributor Author

@gozora Should work now:

  • temporary keyfiles are named after the target (/dev/mapper) name, which must be unique
  • none entries in crypttab are recognized

@OliverO2
Copy link
Contributor Author

@jsmeix
I've even tried to use the new UserInput -C variant to gather a password in layout/prepare/GNU/Linux/160_include_luks_code.sh, replacing the two branches in the elif ... fi section at the end with:

    else
        if [ -n "$password" ] ; then
            password="$(UserInput -I LUKS_DEVICE_PASSWORD -C -t 0 -p "Please enter the password for LUKS device $target_name ($source_device):")"
        fi
        echo "echo \"$password\" | cryptsetup luksFormat --batch-mode $cryptsetup_options $source_device"
        echo "echo \"$password\" | cryptsetup luksOpen $source_device $target_name"
    fi

UserInput works as advertised...

2017-09-26 16:17:10.938445407 UserInput: called in /usr/share/rear/layout/prepare/GNU/Linux/160_include_luks_code.sh line 54
2017-09-26 16:17:10.940826635 UserInput: No choices specified
2017-09-26 16:17:10.942013330 Please enter the password for LUKS device Foxtrot-02 (/dev/sdb1):
2017-09-26 16:17:26.970608078 UserInput: 'read' got user input

...but passwords then appeared in diskrestore.sh and in the logfile nonetheless:

+++ echo '2017-09-26 16:18:11.732036596 Creating luks device Foxtrot-02 on /dev/sdb1'
2017-09-26 16:18:11.732036596 Creating luks device Foxtrot-02 on /dev/sdb1
+++ echo abcdef5
+++ cryptsetup luksFormat --batch-mode --cipher aes-xts-plain64 --key-size 512 --hash sha256 --uuid e1949e50-2b2a-46ae-becf-70e0122df4fb --iter-time 2000 --use-random /dev/sdb1
+++ cryptsetup luksOpen /dev/sdb1 Foxtrot-02
+++ echo abcdef5
+++ component_created /dev/mapper/Foxtrot-02 crypt

So I've reverted this change.

@schlomo
Copy link
Member

schlomo commented Sep 26, 2017

@OliverO2 isn't that in debug mode? Then I would expect the passwords to show up. Users are not expected to use ReaR in debug mode so that regular usage would be save.

@OliverO2
Copy link
Contributor Author

@schlomo Invocation was just rear recover. Neither -d nor -D were used.

Seems like it's in diskrestore.sh, which begins:

#!/bin/bash

LogPrint "Start system layout restoration."

mkdir -p /mnt/local
if create_component "vgchange" "rear" ; then
    lvm vgchange -a n >/dev/null
    component_created "vgchange" "rear"
fi

set -e
set -x

Log output looks like this:

2017-09-26 19:09:23.782373730 ======================
2017-09-26 19:09:23.783321212 Running 'layout/recreate' stage
2017-09-26 19:09:23.784120375 ======================
2017-09-26 19:09:23.789236573 Including layout/recreate/default/100_ask_confirmation.sh
2017-09-26 19:09:23.790280158 Please confirm that '/var/lib/rear/layout/diskrestore.sh' is as you expect.
2017-09-26 19:09:24.823612547 User selected: 5) Continue recovery
2017-09-26 19:09:24.827109572 Including layout/recreate/default/200_run_script.sh
2017-09-26 19:09:24.830486207 Start system layout restoration.
/var/lib/rear/layout/diskrestore.sh: line 7: lvm: command not found
+++ create_component /dev/sda disk
+++ local device=/dev/sda
+++ local type=disk
+++ local touchfile=disk--dev-sda
+++ '[' -e /tmp/rear.QUvz5k8O4h9QKwx/tmp/touch/disk--dev-sda ']'
+++ return 0
+++ Log 'Stop mdadm'
++++ date '+%Y-%m-%d %H:%M:%S.%N '
+++ local 'timestamp=2017-09-26 19:09:24.835891725 '
+++ test 1 -gt 0
+++ echo '2017-09-26 19:09:24.835891725 Stop mdadm'
2017-09-26 19:09:24.835891725 Stop mdadm
+++ grep -q md /proc/mdstat
+++ Log 'Erasing MBR of disk /dev/sda'
++++ date '+%Y-%m-%d %H:%M:%S.%N '
+++ local 'timestamp=2017-09-26 19:09:24.838378106 '
+++ test 1 -gt 0
+++ echo '2017-09-26 19:09:24.838378106 Erasing MBR of disk /dev/sda'
2017-09-26 19:09:24.838378106 Erasing MBR of disk /dev/sda
+++ dd if=/dev/zero of=/dev/sda bs=512 count=1
1+0 records in
1+0 records out
512 bytes copied, 0.0308365 s, 16.6 kB/s
+++ sync

[...]

+++ component_created btrfsmountedsubvol:/mnt/reserve btrfsmountedsubvol
+++ local device=btrfsmountedsubvol:/mnt/reserve
+++ local type=btrfsmountedsubvol
+++ local touchfile=btrfsmountedsubvol-btrfsmountedsubvol:-mnt-reserve
+++ touch /tmp/rear.QUvz5k8O4h9QKwx/tmp/touch/btrfsmountedsubvol-btrfsmountedsubvol:-mnt-reserve
+++ set +x
2017-09-26 19:10:02.921303941 Disk layout created.
2017-09-26 19:10:02.923994344 Including layout/recreate/default/250_verify_mount.sh
2017-09-26 19:10:02.928602720 Finished running 'layout/recreate' stage in 39 seconds
2017-09-26 19:10:02.929844149 ======================
2017-09-26 19:10:02.930850913 Running 'restore' stage
2017-09-26 19:10:02.932030174 ======================

@schlomo
Copy link
Member

schlomo commented Sep 26, 2017

Maybe the code around diskrestore actually enables set -x permanently.

@OliverO2
Copy link
Contributor Author

Apparently, it's layout/prepare/default/540_generate_device_code.sh. Nothing for me to mess with at this time ;-).

@gozora
Copy link
Member

gozora commented Sep 27, 2017

@OliverO2 thanks for updating this PR.
From my point of view, this improvement works well enough to be merged, so if there are not other objections I'll merge it next days.

V.

@jsmeix
Copy link
Member

jsmeix commented Sep 27, 2017

Only a side note how to run commands really silently:
To run commands that deal with confidential data
really silently in ReaR see the comment in the
UserInput function that reads:

#       If confidential user input is needed also in debugscripts mode the caller of the UserInput function
#       must call it in an appropriate (temporary) environment e.g. with STDERR redirected to /dev/null like:
#           { password="$( UserInput -I PASSWORD -C -r -s -p 'Enter the pasword' )" ; } 2>/dev/null

I.e. you need to put confidentially working commands into
an appropriate silencing enviroment for example like

{ root_password="$( grep '^root:' /etc/shadow | cut -d ':' -f2 )" ; } 2>/dev/null
{ echo "$root_password" ; } 1>/dev/null 2>/dev/null

You cannot rely on that 'set -x' is not somehow set.
The user can set it, a script may set it, whatever else...

@gozora
Copy link
Member

gozora commented Oct 4, 2017

@OliverO2 thanks for this improvement!

@jsmeix jsmeix added this to the ReaR v2.3 milestone Oct 4, 2017
@OliverO2 OliverO2 deleted the feature/luks-improvements branch November 8, 2017 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants