Bug#932588: buster-pu: package libblockdev/2.20-7+deb10u1
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
Hi,
I'd like to make a stable upload for libblockdev fixing (at least
partially) #928893 (which was severe enough to be mentioned in the
release notes.
Full debdiff is attached. CCing the excellent changelog entry from
intrigeri here:
libblockdev (2.20-7+deb10u1) buster; urgency=medium
[ intrigeri ]
* Use existing cryptsetup API for changing keyslot passphrase.
Cherry-pick upstream fix to use existing cryptsetup API for atomically
changing a keyslot passphrase, instead of deleting the old keyslot
before adding the new one. This avoids data loss when attempting to
change the passphrase of a LUKS2 device via udisks2, e.g. from GNOME
Disks.
Deleting a keyslot and then adding one is risky: if anything goes wrong
before the new keyslot is successfully added, no usable keyslot is left
and the device cannot be unlocked anymore. There's little chances this
causes actual problems with LUKS1, but LUKS2 defaults to the memory-hard
Argon2 key derivation algorithm, which is implemented in cryptsetup with
the assumption that it runs as root with no MEMLOCK ulimit; this
assumption is wrong when run by udisks2.service under
LimitMEMLOCK=65536, which breaks adding the new keyslot, and makes us
hit the problematic situation (user data loss) every time.
With this change, changing a LUKS2 passphrase via udisks2 will still
fail in some cases, until the MEMLOCK ulimit problem is solved in
cryptsetup or workaround'ed in udisks2. But at least, if it fails, it
will fail _atomically_ and the original passphrase will still work.
(Closes: #928893)
Huge thanks to intrigeri and Guilem for debugging this issue.
Regarding the version number: 2.20-8 was never released to the archive
(the next upload was 2.22-1). Do you prefer to use 2.20-8 for stable
uploads in such a case or is 2.20-7+deb10u1 preferred?
Regards,
Michael
-- System Information:
Debian Release: bullseye/sid
APT prefers unstable
APT policy: (500, 'unstable'), (200, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 4.19.0-5-amd64 (SMP w/4 CPU cores)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), LANGUAGE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
diff --git a/debian/changelog b/debian/changelog
index c9bfefa..9b8fd89 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,29 @@
+libblockdev (2.20-7+deb10u1) buster; urgency=medium
+
+ [ intrigeri ]
+ * Use existing cryptsetup API for changing keyslot passphrase.
+ Cherry-pick upstream fix to use existing cryptsetup API for atomically
+ changing a keyslot passphrase, instead of deleting the old keyslot
+ before adding the new one. This avoids data loss when attempting to
+ change the passphrase of a LUKS2 device via udisks2, e.g. from GNOME
+ Disks.
+ Deleting a keyslot and then adding one is risky: if anything goes wrong
+ before the new keyslot is successfully added, no usable keyslot is left
+ and the device cannot be unlocked anymore. There's little chances this
+ causes actual problems with LUKS1, but LUKS2 defaults to the memory-hard
+ Argon2 key derivation algorithm, which is implemented in cryptsetup with
+ the assumption that it runs as root with no MEMLOCK ulimit; this
+ assumption is wrong when run by udisks2.service under
+ LimitMEMLOCK=65536, which breaks adding the new keyslot, and makes us
+ hit the problematic situation (user data loss) every time.
+ With this change, changing a LUKS2 passphrase via udisks2 will still
+ fail in some cases, until the MEMLOCK ulimit problem is solved in
+ cryptsetup or workaround'ed in udisks2. But at least, if it fails, it
+ will fail _atomically_ and the original passphrase will still work.
+ (Closes: #928893)
+
+ -- Michael Biebl <biebl@debian.org> Sat, 20 Jul 2019 23:18:18 +0200
+
libblockdev (2.20-7) unstable; urgency=medium
* Cherry-pick Use-512bit-keys-in-LUKS-by-default.patch:
diff --git a/debian/gbp.conf b/debian/gbp.conf
index 206bbd0..7d49ad9 100644
--- a/debian/gbp.conf
+++ b/debian/gbp.conf
@@ -1,5 +1,5 @@
[DEFAULT]
-debian-branch = debian/sid
+debian-branch = debian/buster
upstream-branch = upstream/latest
pristine-tar = True
sign-tags = True
diff --git a/debian/patches/Use-existing-cryptsetup-API-for-changing-keyslot-passphra.patch b/debian/patches/Use-existing-cryptsetup-API-for-changing-keyslot-passphra.patch
new file mode 100644
index 0000000..a125583
--- /dev/null
+++ b/debian/patches/Use-existing-cryptsetup-API-for-changing-keyslot-passphra.patch
@@ -0,0 +1,91 @@
+From: Vojtech Trefny <vtrefny@redhat.com>
+Date: Tue, 12 Mar 2019 09:28:05 +0100
+Subject: Use existing cryptsetup API for changing keyslot passphrase
+
+Instead of manually removing the keyslot and adding new a one.
+Our old code also doesn't work in FIPS mode.
+
+(cherry picked from commit 34ed7becf4536ee1277175abdf47c075f340af61)
+---
+ src/plugins/crypto.c | 40 +++++++++-------------------------------
+ tests/crypto_test.py | 3 +++
+ 2 files changed, 12 insertions(+), 31 deletions(-)
+
+diff --git a/src/plugins/crypto.c b/src/plugins/crypto.c
+index 6b5be9d..205499f 100644
+--- a/src/plugins/crypto.c
++++ b/src/plugins/crypto.c
+@@ -1359,8 +1359,6 @@ gboolean bd_crypto_luks_remove_key (const gchar *device, const gchar *pass, cons
+ gboolean bd_crypto_luks_change_key_blob (const gchar *device, const guint8 *pass_data, gsize data_len, const guint8 *npass_data, gsize ndata_len, GError **error) {
+ struct crypt_device *cd = NULL;
+ gint ret = 0;
+- gchar *volume_key = NULL;
+- gsize vk_size = 0;
+ guint64 progress_id = 0;
+ gchar *msg = NULL;
+
+@@ -1385,41 +1383,21 @@ gboolean bd_crypto_luks_change_key_blob (const gchar *device, const guint8 *pass
+ return FALSE;
+ }
+
+- vk_size = crypt_get_volume_key_size(cd);
+- volume_key = (gchar *) g_malloc (vk_size);
+-
+- ret = crypt_volume_key_get (cd, CRYPT_ANY_SLOT, volume_key, &vk_size, (char*) pass_data, data_len);
+- if (ret < 0) {
+- g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_DEVICE,
+- "Failed to load device's volume key: %s", strerror_l(-ret, c_locale));
+- crypt_free (cd);
+- g_free (volume_key);
+- bd_utils_report_finished (progress_id, (*error)->message);
+- return FALSE;
+- }
+-
+- /* ret is the number of the slot with the given pass */
+- ret = crypt_keyslot_destroy (cd, ret);
+- if (ret != 0) {
+- g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_REMOVE_KEY,
+- "Failed to remove the old passphrase: %s", strerror_l(-ret, c_locale));
+- crypt_free (cd);
+- g_free (volume_key);
+- bd_utils_report_finished (progress_id, (*error)->message);
+- return FALSE;
+- }
+-
+- ret = crypt_keyslot_add_by_volume_key (cd, ret, volume_key, vk_size, (char*) npass_data, ndata_len);
++ ret = crypt_keyslot_change_by_passphrase (cd, CRYPT_ANY_SLOT, CRYPT_ANY_SLOT,
++ (char*) pass_data, data_len,
++ (char*) npass_data, ndata_len);
+ if (ret < 0) {
+- g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_ADD_KEY,
+- "Failed to add the new passphrase: %s", strerror_l(-ret, c_locale));
++ if (ret == -EPERM)
++ g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_DEVICE,
++ "Failed to change the passphrase: No keyslot with given passphrase found.");
++ else
++ g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_ADD_KEY,
++ "Failed to change the passphrase: %s", strerror_l (-ret, c_locale));
+ crypt_free (cd);
+- g_free (volume_key);
+ bd_utils_report_finished (progress_id, (*error)->message);
+ return FALSE;
+ }
+
+- g_free (volume_key);
+ crypt_free (cd);
+ bd_utils_report_finished (progress_id, "Completed");
+ return TRUE;
+diff --git a/tests/crypto_test.py b/tests/crypto_test.py
+index c048570..747b702 100644
+--- a/tests/crypto_test.py
++++ b/tests/crypto_test.py
+@@ -401,6 +401,9 @@ class CryptoTestChangeKey(CryptoTestCase):
+ succ = create_fn(self.loop_dev, PASSWD, None)
+ self.assertTrue(succ)
+
++ with six.assertRaisesRegex(self, GLib.GError, r"No keyslot with given passphrase found."):
++ BlockDev.crypto_luks_change_key(self.loop_dev, "wrong-passphrase", PASSWD2)
++
+ succ = BlockDev.crypto_luks_change_key(self.loop_dev, PASSWD, PASSWD2)
+ self.assertTrue(succ)
+
diff --git a/debian/patches/series b/debian/patches/series
index 94d3e03..22a914c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
Use-512bit-keys-in-LUKS-by-default.patch
+Use-existing-cryptsetup-API-for-changing-keyslot-passphra.patch
Reply to: