Package: release.debian.org Severity: normal Tags: buster User: release.debian....@packages.debian.org Usertags: pu
Dear release team,
Buster's cryptsetup (2:2.1.0-5) doesn't cope well with LUKS2 headers
without any bound keyslot: adding a new key slot to such a header fails,
both via the crypt_keyslot_add_by_volume_key() API call and with
`luksAddKey --master-key`.
This alone doesn't warrant a s-p-u, but some applications seem to use
the following sequences of API calls to change a passphrase:
keyslot = crypt_volume_key_get(cd, …, volume_key, old_passphrase);
crypt_keyslot_destroy(cd, keyslot);
crypt_keyslot_add_by_volume_key(cd, 0, volume_key, new_passphrase);
This is a *very* bad idea in the first place, and they should use
crypt_keyslot_change_by_passphrase() instead: if there was only one
active keyslot and for some reason the crypt_keyslot_add_by_volume_key()
call fails (for instance due to an hardware failure, or because the
PBKDF benchmark triggers the OOM killer) then the entire volume is lost…
However the libcryptsetup bug makes it much, much worse as the call
*always* fails on LUKS2 headers without any bound keyslot. This is what
happened in #928893 (gnome-disk-utility: disk content permanently lost
when changing LUKS password).
Using codesearch.d.n I found that (as far as sid is concerned) beside
src:cryptsetup, only src:libblockdev and src:cryptmount are calling
crypt_keyslot_destroy(). AFAICT src:cryptmount is making a sane use of
the call [0]; libblockdev is affected in Buster but per #932588 will be
fixed to use crypt_keyslot_change_by_passphrase() in the upcoming point
release. Still there might be third party programs we don't know about,
and considering the serious risk of data loss I think it makes sense to
fix the libcryptsetup bug in the next point release, too. Changelog
since 2:2.1.0-5 is as follows, and debdiff attached.
cryptsetup (2:2.1.0-5+deb10u1) buster; urgency=high
* Backport upstream commits c03e3fe8, 725720df and fe4e1de5 to fix support
for LUKS2 headers without any bound keyslot. Adding a new key slot using
the volume key was failing, both via the crypt_keyslot_add_by_volume_key()
API call and with `luksAddKey --master-key`. The former in particular
might yield data loss if, in order to change a passphrase, an application
destroys the keyslot before adding a new one (using the volume key), cf.
#928893. Note that doing so is *unsafe*: applications should instead use
crypt_keyslot_change_by_passphrase() from libcryptsetup >=1.6.0.
Trying to open LUKS2 volume by supplying the volume key on the command
line was also failing if there were no bound keyslot on the header.
(Closes: #934715)
-- Guilhem Moulin <guil...@debian.org> Fri, 16 Aug 2019 19:18:10 +0200
The 3 cherry-picked patches are all backported from 2.2.0 [1,2], and the
version in sid is not affected. (The one in Stretch is not affected
either as it doesn't have LUKS2 support.) The diff also includes unit
tests, but note that the tests in question need root access hence are
ignored by the build daemons. I did ensure that the whole test-suite
still passes on amd64, though.
In case you're unhappy with this changeset, then I propose to only
include the first 2 patches. IMHO what should really be fixed in Buster
is the libcryptsetup part. For the CLI part (third patch) the risk of
data loss is lower as the volume key is stored in a file.
Thanks for considering its inclusion in Buster! CC'ing KiBi for the d-i ack.
Cheers,
--
Guilhem.
[0]
https://codesearch.debian.net/show?file=cryptmount_5.3.1-1%2Farmour-luks.c&line=247
[1] https://gitlab.com/cryptsetup/cryptsetup/issues/466
[2] https://gitlab.com/cryptsetup/cryptsetup/issues/470
diffstat for cryptsetup-2.1.0 cryptsetup-2.1.0
changelog | 16 +
gbp.conf | 1
patches/Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch | 56
++++++
patches/Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch | 86
++++++++++
patches/Mention-limitation-of-crypt_get_volume_key_size.patch | 20 ++
patches/series | 3
6 files changed, 182 insertions(+)
diff -Nru cryptsetup-2.1.0/debian/changelog cryptsetup-2.1.0/debian/changelog
--- cryptsetup-2.1.0/debian/changelog 2019-06-10 14:51:15.000000000 +0200
+++ cryptsetup-2.1.0/debian/changelog 2019-08-16 19:18:10.000000000 +0200
@@ -1,3 +1,19 @@
+cryptsetup (2:2.1.0-5+deb10u1) buster; urgency=high
+
+ * Backport upstream commits c03e3fe8, 725720df and fe4e1de5 to fix support
+ for LUKS2 headers without any bound keyslot. Adding a new key slot using
+ the volume key was failing, both via the crypt_keyslot_add_by_volume_key()
+ API call and with `luksAddKey --master-key`. The former in particular
+ might yield data loss if, in order to change a passphrase, an application
+ destroys the keyslot before adding a new one (using the volume key), cf.
+ #928893. Note that doing so is *unsafe*: applications should instead use
+ crypt_keyslot_change_by_passphrase() from libcryptsetup >=1.6.0.
+ Trying to open LUKS2 volume by supplying the volume key on the command
+ line was also failing if there were no bound keyslot on the header.
+ (Closes: #934715)
+
+ -- Guilhem Moulin <guil...@debian.org> Fri, 16 Aug 2019 19:18:10 +0200
+
cryptsetup (2:2.1.0-5) unstable; urgency=medium
[ Jonas Meurer ]
diff -Nru cryptsetup-2.1.0/debian/gbp.conf cryptsetup-2.1.0/debian/gbp.conf
--- cryptsetup-2.1.0/debian/gbp.conf 2019-06-10 14:51:15.000000000 +0200
+++ cryptsetup-2.1.0/debian/gbp.conf 2019-08-16 19:18:10.000000000 +0200
@@ -4,3 +4,4 @@
[buildpackage]
upstream-tag = v%(version)s
upstream-branch = upstream-2.0.x
+debian-branch = debian-buster
diff -Nru
cryptsetup-2.1.0/debian/patches/Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch
cryptsetup-2.1.0/debian/patches/Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch
---
cryptsetup-2.1.0/debian/patches/Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch
1970-01-01 01:00:00.000000000 +0100
+++
cryptsetup-2.1.0/debian/patches/Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch
2019-08-16 19:18:10.000000000 +0200
@@ -0,0 +1,56 @@
+From c03e3fe88a9761f34b22d2b4d4654353783e2d4f Mon Sep 17 00:00:00 2001
+From: Ondrej Kozina <okoz...@redhat.com>
+Date: Tue, 26 Feb 2019 11:49:58 +0100
+Subject: Fix getting default LUKS2 keyslot encryption parameters.
+
+When information about original keyslot size is missing (no active
+keyslot assigned to default segment) we have to fallback to
+default luks2 encryption parameters even though we know default
+segment cipher and mode.
+
+Fixes: #442.
+---
+ lib/setup.c | 3 ++-
+ tests/api-test-2.c | 19 +++++++++++++++++++
+ 2 files changed, 21 insertions(+), 1 deletion(-)
+
+--- a/lib/setup.c
++++ b/lib/setup.c
+@@ -4632,7 +4632,8 @@ const char *crypt_keyslot_get_encryption
+ cipher = LUKS2_get_cipher(&cd->u.luks2.hdr, CRYPT_DEFAULT_SEGMENT);
+ if (!LUKS2_keyslot_cipher_incompatible(cd, cipher)) {
+ *key_size = crypt_get_volume_key_size(cd);
+- return cipher;
++ if (*key_size)
++ return cipher;
+ }
+
+ /* Fallback to default LUKS2 keyslot encryption */
+--- a/tests/api-test-2.c
++++ b/tests/api-test-2.c
+@@ -914,6 +914,25 @@ static void AddDeviceLuks2(void)
+ FAIL_(crypt_activate_by_volume_key(cd, CDEVICE_1, key3, key_size, 0),
"VK doesn't match any digest assigned to segment 0");
+ crypt_free(cd);
+
++ /*
++ * Check regression in getting keyslot encryption parameters when
++ * volume key size is unknown (no active keyslots).
++ */
++ if (!_fips_mode) {
++ OK_(crypt_init(&cd, DMDIR L_DEVICE_1S));
++ crypt_set_iteration_time(cd, 1);
++ OK_(crypt_format(cd, CRYPT_LUKS2, cipher, cipher_mode, NULL,
key, key_size, NULL));
++ EQ_(crypt_keyslot_add_by_volume_key(cd, 0, NULL, key_size,
PASSPHRASE, strlen(PASSPHRASE)), 0);
++ /* drop context copy of volume key */
++ crypt_free(cd);
++ OK_(crypt_init(&cd, DMDIR L_DEVICE_1S));
++ OK_(crypt_load(cd, CRYPT_LUKS, NULL));
++ EQ_(crypt_volume_key_get(cd, CRYPT_ANY_SLOT, key, &key_size,
PASSPHRASE, strlen(PASSPHRASE)), 0);
++ OK_(crypt_keyslot_destroy(cd, 0));
++ EQ_(crypt_keyslot_add_by_volume_key(cd, 0, key, key_size,
PASSPHRASE, strlen(PASSPHRASE)), 0);
++ crypt_free(cd);
++ }
++
+ _cleanup_dmdevices();
+ }
+
diff -Nru
cryptsetup-2.1.0/debian/patches/Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch
cryptsetup-2.1.0/debian/patches/Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch
---
cryptsetup-2.1.0/debian/patches/Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch
1970-01-01 01:00:00.000000000 +0100
+++
cryptsetup-2.1.0/debian/patches/Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch
2019-08-16 19:18:10.000000000 +0200
@@ -0,0 +1,86 @@
+From 725720dfc31ff26c4a60089a478fe5e882925ef3 Mon Sep 17 00:00:00 2001
+From: Milan Broz <gmazyl...@gmail.com>
+Date: Wed, 14 Aug 2019 12:31:40 +0200
+Subject: Fix volume key file if no LUKS2 keyslots are present.
+
+If all keyslots are removed, LUKS2 has no longer information about
+the volume key size (there is only key digest present).
+
+If user wants to open or add new keyslot, it must get information
+about key size externally.
+
+We do not want to guess key size from the file size (it does not
+work for block devices for example), so require explicit --keyfil
+option in these cases.
+
+Fixes #470.
+---
+ src/cryptsetup.c | 18 ++++++++++++++++--
+ tests/compat-test2 | 7 ++++++-
+ 2 files changed, 22 insertions(+), 3 deletions(-)
+
+--- a/src/cryptsetup.c
++++ b/src/cryptsetup.c
+@@ -1249,6 +1249,13 @@ static int action_open_luks(void)
+
+ if (opt_master_key_file) {
+ keysize = crypt_get_volume_key_size(cd);
++ if (!keysize && !opt_key_size) {
++ log_err(_("Cannot dermine volume key size for LUKS
without keyslots, please use --key-size option."));
++ r = -EINVAL;
++ goto out;
++ } else if (!keysize)
++ keysize = opt_key_size / 8;
++
+ r = tools_read_mk(opt_master_key_file, &key, keysize);
+ if (r < 0)
+ goto out;
+@@ -1553,6 +1560,13 @@ static int action_luksAddKey(void)
+ }
+
+ if (opt_master_key_file) {
++ if (!keysize && !opt_key_size) {
++ log_err(_("Cannot dermine volume key size for LUKS
without keyslots, please use --key-size option."));
++ r = -EINVAL;
++ goto out;
++ } else if (!keysize)
++ keysize = opt_key_size / 8;
++
+ r = tools_read_mk(opt_master_key_file, &key, keysize);
+ if (r < 0)
+ goto out;
+@@ -2752,9 +2766,9 @@ int main(int argc, const char **argv)
+ strcmp(aname, "luksFormat") &&
+ strcmp(aname, "open") &&
+ strcmp(aname, "benchmark") &&
+- (strcmp(aname, "luksAddKey") || !opt_unbound))
++ strcmp(aname, "luksAddKey"))
+ usage(popt_context, EXIT_FAILURE,
+- _("Option --key-size is allowed only for luksFormat,
luksAddKey (with --unbound),\n"
++ _("Option --key-size is allowed only for luksFormat,
luksAddKey,\n"
+ "open and benchmark actions. To limit read from keyfile
use --keyfile-size=(bytes)."),
+ poptGetInvocationName(popt_context));
+
+--- a/tests/compat-test2
++++ b/tests/compat-test2
+@@ -492,7 +492,7 @@ echo $PWD1 | $CRYPTSETUP luksOpen $LOOPD
+ $CRYPTSETUP luksClose $DEV_NAME || fail
+
+ prepare "[21] luksDump" wipe
+-echo $PWD1 | $CRYPTSETUP -q luksFormat $FAST_PBKDF_OPT --uuid $TEST_UUID
--type luks2 $LOOPDEV $KEY1 || fail
++echo $PWD1 | $CRYPTSETUP -q luksFormat --key-size 256 $FAST_PBKDF_OPT --uuid
$TEST_UUID --type luks2 $LOOPDEV $KEY1 || fail
+ echo $PWD1 | $CRYPTSETUP luksAddKey $FAST_PBKDF_OPT $LOOPDEV -d $KEY1 || fail
+ $CRYPTSETUP luksDump $LOOPDEV | grep -q "0: luks2" || fail
+ $CRYPTSETUP luksDump $LOOPDEV | grep -q $TEST_UUID || fail
+@@ -504,6 +504,11 @@ echo $PWD1 | $CRYPTSETUP luksDump -q $LO
+ fips_mode || {
+ echo $PWD1 | $CRYPTSETUP luksAddKey $FAST_PBKDF_OPT --master-key-file
$VK_FILE $LOOPDEV || fail
+ }
++# Use volume key file without keyslots
++$CRYPTSETUP luksErase -q $LOOPDEV || fail
++$CRYPTSETUP luksOpen --master-key-file $VK_FILE --key-size 256
--test-passphrase $LOOPDEV || fail
++echo $PWD1 | $CRYPTSETUP luksAddKey $FAST_PBKDF_OPT --master-key-file
$VK_FILE --key-size 256 $LOOPDEV || fail
++echo $PWD1 | $CRYPTSETUP luksOpen --test-passphrase $LOOPDEV || fail
+
+ prepare "[22] remove disappeared device" wipe
+ dmsetup create $DEV_NAME --table "0 39998 linear $LOOPDEV 2" || fail
diff -Nru
cryptsetup-2.1.0/debian/patches/Mention-limitation-of-crypt_get_volume_key_size.patch
cryptsetup-2.1.0/debian/patches/Mention-limitation-of-crypt_get_volume_key_size.patch
---
cryptsetup-2.1.0/debian/patches/Mention-limitation-of-crypt_get_volume_key_size.patch
1970-01-01 01:00:00.000000000 +0100
+++
cryptsetup-2.1.0/debian/patches/Mention-limitation-of-crypt_get_volume_key_size.patch
2019-08-16 19:18:10.000000000 +0200
@@ -0,0 +1,20 @@
+From fe4e1de56639f1e6851ff8e47729f703a25dece4 Mon Sep 17 00:00:00 2001
+From: Milan Broz <gmazyl...@gmail.com>
+Date: Mon, 29 Jul 2019 14:32:13 +0200
+Subject: Mention limitation of crypt_get_volume_key_size().
+
+---
+ lib/libcryptsetup.h | 2 ++
+ 1 file changed, 2 insertions(+)
+
+--- a/lib/libcryptsetup.h
++++ b/lib/libcryptsetup.h
+@@ -1448,6 +1448,8 @@ uint64_t crypt_get_iv_offset(struct cryp
+ *
+ * @return volume key size
+ *
++ * @note For LUKS2, this function can be used only if there is at least
++ * one keyslot assigned to data segment.
+ */
+ int crypt_get_volume_key_size(struct crypt_device *cd);
+
diff -Nru cryptsetup-2.1.0/debian/patches/series
cryptsetup-2.1.0/debian/patches/series
--- cryptsetup-2.1.0/debian/patches/series 1970-01-01 01:00:00.000000000
+0100
+++ cryptsetup-2.1.0/debian/patches/series 2019-08-16 19:18:10.000000000
+0200
@@ -0,0 +1,3 @@
+Fix-getting-default-LUKS2-keyslot-encryption-paramet.patch
+Mention-limitation-of-crypt_get_volume_key_size.patch
+Fix-volume-key-file-if-no-LUKS2-keyslots-are-present.patch
signature.asc
Description: PGP signature
