Add initial LUKS2 support #2504
Conversation
This is needed for recreation of the same version of LUKS that was on the system. Without it, default version of LUKS will be created depending on version of cryptsetup. Default header format is LUKS1 with cryptsetup < 2.1.0 and LUKS2 with cryptsetup >= 2.1.0.
Use 'blkid' to check that it's LUKS and to find its version. It should be prefered over 'cryptsetup' according to LUKS maintainers.
|
@vcrhonek |
| @@ -13,6 +13,9 @@ create_crypt() { | |||
| value=${option#*=} | |||
|
|
|||
| case "$key" in | |||
| type) | |||
| cryptsetup_options+=" --type $value" | |||
There was a problem hiding this comment.
Just off the top of my head:
I think the --type option is not supported by older cryptsetup versions
that is provided in older Linux operating systems
that are perhaps still supported by ReaR, cf.
https://github.com/rear/rear/blob/master/doc/rear-release-notes.txt#L2513
ReaR-2.6 is supported on the following Linux based operating systems:
o Fedora 29, 30, 31, and 32
o RHEL 6, 7, and 8
o CentOS 6, 7, and 8
o Scientific Linux 6 and 7
o SLES 12 and 15
o openSUSE Leap 15.x
o Debian 8, and 9
o Ubuntu 16, 17, and 18
So some backward compatibility code could be required here.
See in particular the sections
"Try hard to care about possible errors" and
"Maintain backward compatibility" and
"Dirty hacks welcome" in
https://github.com/rear/rear/wiki/Coding-Style
There was a problem hiding this comment.
I found it:
#2432 (comment)
So for all supported SLES versions cryptsetup supports the --type option.
@vcrhonek
did you check that for all supported RHEL and the other Red Hat based operating systems
cryptsetup supports the --type option?
There was a problem hiding this comment.
It's not supported on all systems, at least not at RHEL 6.
There was a problem hiding this comment.
@vcrhonek
if you do no longer need RHEL 6 support in current ReaR upstream master code
we can drop support for RHEL 6 and other Red Hat based operating systems
that are basically same for next ReaR version 2.7
cf. what I wrote about older operating systems in our release notes
https://github.com/rear/rear/blob/master/doc/rear-release-notes.txt#L2556
so we could drop support for RHEL 6 and related operating systems
with a note what is known to no longer work
i.e. LUKS support (also LUKS 1 for RHEL 6) in this case.
When no LUKS is used dmsetup ls --target crypt should not output anything
so that usr/share/rear/layout/save/GNU/Linux/260_crypt_layout.sh
results nothing about LUKS in disklayout.conf.
There was a problem hiding this comment.
I've checked all Red Hat based operating systems and type option is supported on all of them except RHEL 6 and CentOS 6. I've also discovered that blkid -p -o export returns VERSION=256 for LUKS 1 on RHEL 6 so it won't get through LUKS version check anyway.
| Log "Skipping $target_name (its $source_device is not a LUKS device)" | ||
| continue | ||
| fi | ||
|
|
||
| # Detect LUKS version | ||
| version=$( grep "VERSION" $TMP_DIR/blkid.output | cut -d= -f2 ) | ||
| if !( test $version = "1" || test $version = "2") ; then |
There was a problem hiding this comment.
The condition !( ...) will not work as intended
because "man bash" reads
If the extglob shell option is enabled
...
!(pattern-list)
Matches anything except one of the given patterns
and usr/sbin/rear has the extglob shell option enabled
https://github.com/rear/rear/blob/master/usr/sbin/rear#L314
There was a problem hiding this comment.
Sigh - I needed three commits to now hopefully have that fixed.
Sometimes bash scripting is so weird.
Fixed version test, cf. https://github.com/rear/rear/pull/2504/files#r510856646 plus more explanatory (error) messages.
Fixed how the LUKS version is determined
|
@gdha |
|
@vcrhonek |
|
@jsmeix |
If more than one keyslot is used in LUKS header the 'grep' command looking for 'Cipher key' would return more than one line. But we need just one value. Use 'key_size' from the first found key.
|
I've tested and your changes and it works fine, no problem there. But I've found different issue that emerged when multiple keys were defined in LUKS header (this is not done by system installer by default, that's main reason why I've missed it before). I fixed that with additional commit. |
|
@vcrhonek |
|
@rear/contributors |
|
Yesterday I found no time to merge is so I will merge it today afternoon. |
|
@vcrhonek Regarding your question in your initial entry (excerpts) In general the ReaR recovery system must be free of secrets I don't know about LUKS internals so I don't know if complete LUKS headers |
|
@jsmeix I see. I don't know much about LUKS internals either. Most of the header is encrypted, but I'm afraid that if someone has copy of it, keys from the header could be used to access the data even if these keys are no longer valid (were removed in more recent version of header). I can ask LUKS experts. I was thinking to introduce LUKS_HEADER_RESTORE variable that would (when explicitly configured by user) cause usage of luksHeaderBackup/Restore instead of calling luksFormat. I did few tests and it seems to work fine. Please let me know if you think that it would be beneficial, I can contribute the code. |
|
And this example is a good reason why |
|
I verified that things fail for me as I described above in With current GitHub master code I get This is the matching excerpt from the log file I have /var/lib/rear/layout/diskrestore.sh runs with so any non-zero exit code in diskrestore.sh results /var/lib/rear/layout/diskrestore.sh is generated only during |
In layout/save/GNU/Linux/260_crypt_layout.sh grep for "Key:" (instead of "Cipher key") to get the key_size value from the 'cryptsetup luksDump' output in case of LUKS2, see #2504 (comment) and subsequent comments therein.
|
With b49a4fc According to @vcrhonek |
|
@mannp @adatum |
@jsmeix I was very happy to see the merge for LUKS2 support and installed the git version of rear to give it a try yesterday. Got sidetracked, but will set aside some time to give it a try :) |
|
@mannp So I would recommend to better update again to our current GitHub master code. Perhaps you even use a cryptsetup version where my |
In layout/save/GNU/Linux/260_crypt_layout.sh added basic checks that the cipher key_size hash uuid values exist cf. #2504 (comment)
|
With For me things still work well with that checks. With artificial causing such errors via in usr/share/rear/layout/save/GNU/Linux/260_crypt_layout.sh |
|
And on top of that via With that my above artificial causing such errors lets "rear -D mkrescue" fail |
|
@jsmeix I have not been using rear for a long time so need to configure and test on a spare machine, so it will take me some time to get together :) I previously was using this on my main machines -> https://aur.archlinux.org/packages/relax-and-recover-git/ |
|
@jsmeix I've looked on recent updates and it looks good and works fine on my test machines, thanks! Have you considered omitting missing values instead of reporting error? Because luksFormat doesn't require any of {cipher, key-size, hash} and if omitted default value is used. Maybe it could omit missing option value and print warning? I'm not saying that I think it would be better, I guess that depends on situation and user preferences. I was also thinking about 'Key:'/'Cipher key:" issue for second time. If only 'Cipher key:' was present in luksDump, parsing would fail again. I have no idea whether that can happen, to be honest. If that happens, we can match both and use first found like But overall, it looks good to me now and I'm looking forward to see results on other setups and distributions. |
|
@mannp That's what I do myself. I don't know about whatever third-party ReaR packages. |
|
@vcrhonek Currently LUKS support (both LUKS1 and LUKS2) is under step by step enhancement I just proceed little step by little step where each little step should keep things working Because at any time a more important issue (e.g. a severe security issue with SUSE E.g. as long as the LUKS recreation code in usr/share/rear/layout/prepare/ When the LUKS recreation code in layout/prepare/ was enhanced as a precondition Currently I have time to enhance the LUKS code so I will do some more enhancements |
|
Right now I see the documentation in Actually with the current LUKS recreation code in Fortunately nobody reads documenation so such errors are |
The "cryptseup luksFormat" command does not require any of the cipher, key-size, hash option values because if omitted a cryptseup default value is used, cf. #2504 (comment)
|
#2506 |
Depending on the version the "cryptsetup luksDump" command outputs the key_size value as a line like " Key: 512 bits" and/or as a line like " Cipher key: 512 bits" cf. #2504 (comment) and subsequent comments so we grep for both lines but use only the first match.
|
@vcrhonek Accordingly currently in #2506 How I did set up a LUKS2 volume on a test VM was using plain I need According to "man cryptsetup" the So I think we should handle all options optionally in |
|
@jsmeix Yes, I agree that I'm not sure about I did try to edit disklayout.conf just before I ran I'll try to save some time tomorrow and look at your recent updates. |
|
In #2506 My primary intent behind is not that "all will just work well" My primary intent is to not let "rear recover" needlessly fail |
I absolutely agree with that and I had it on mind in first part of #2504 (comment) |
…onal_values Make recreating LUKS volumes work with optional cryptsetup options: The "cryptseup luksFormat" command does not require any of the type, cipher, key-size, hash, uuid option values because if omitted a cryptseup default value is used, cf. #2504 (comment) Right UUID values are mantatory for LUKS volumes that will be mounted during startup of the recreated system. But this does not mean ReaR should error out when there is no cryptsetup uuid value because it is possible to run "rear recover" with enforced MIGRATION_MODE and manually correct the restored /mnt/local/etc/crypttab file to use the new UUIDs before the initrd is recreated and the bootloader is (re)-installed cf. #2509
In layout/save/GNU/Linux/260_crypt_layout.sh grep for "Key:" (instead of "Cipher key") to get the key_size value from the 'cryptsetup luksDump' output in case of LUKS2, see rear#2504 (comment) and subsequent comments therein.
In layout/save/GNU/Linux/260_crypt_layout.sh added basic checks that the cipher key_size hash uuid values exist cf. rear#2504 (comment)
The "cryptseup luksFormat" command does not require any of the cipher, key-size, hash option values because if omitted a cryptseup default value is used, cf. rear#2504 (comment)
Depending on the version the "cryptsetup luksDump" command outputs the key_size value as a line like " Key: 512 bits" and/or as a line like " Cipher key: 512 bits" cf. rear#2504 (comment) and subsequent comments so we grep for both lines but use only the first match.
Relax-and-Recover (ReaR) Pull Request Template
Please fill in the following items before submitting a new pull request:
Pull Request Details:
Type: Bug Fix / New Feature / Enhancement / Other?
Enhancement.
Impact: Low / Normal / High / Critical / Urgent
Normal.
Reference to related issue (URL):
Failed to recover LUKS version 2 #2204
How was this pull request tested?
Quite thoroughly. I've run many backup followed by recover tests on Fedora 32, RHEL 8.2 and Fedora 25 (because there is LUKS1 by default on f25). I didn't notice any issues when I used default encryption during install of the system. I've also tried various ciphers/key-sizes/hash variants to ensure that values given by luksDump parsing on LUKS2 are meaningful and to recreating LUKS1 on system where LUKS2 is default and vice versa.
Brief description of the changes in this pull request:
a) it adds new parameter 'type' to 'crypt' keyword used in disklayout.conf. Using this parameter allows to recreate the same version of LUKS that was on the system
b) it adds LUKS version detection, parsing depending on version and usage of 'type' parameter have been added to
/usr/share/rear/layout/save/GNU/Linux/260_crypt_layout.sh
I'm aware that this is useful mainly for basic LUKS2 setup (let's say similar to LUKS1), but I believe that even so it could be sufficient for many users.
And I wonder... wouldn't be better to backup and restore complete LUKS header instead of taking values from luksDump and creating new one? That would simplify things a lot and, for example, users won't lose keys from additional keyslots, ...