[Pkg-utopia-maintainers] Bug#928893: gnome-disk-utility: disk content permamently lost when changing LUKS password

[Guilhem Moulin]

Hi there,

On Fri, 19 Jul 2019 at 22:14:49 -0300, intrigeri wrote:
> it turns out this is caused by a bug in libblockdev, which is fixed in
> sid already (although it seems like upstream applied the fix for
> unrelated reasons and it's not clear whether they realized this bug
> was a possibility).

AFAICT there were two bugs in src/plugins/crypto.c:bd_crypto_luks_change_key_blob()

    https://sources.debian.org/src/libblockdev/2.20-7/src/plugins/crypto.c/#L1359

The API calls to libcryptsetup roughly goes as follows:

    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);

The first call uses the old passphrase to unlock a keyslot and set the
volume key.   (In LUKS2 the volume key of open devices isn't accessible
to userspace, but of course it's no problem here since the passphrase is
used to unlock a keyslot, which yields the volume key.)

The second call removes the keyslot used to get the volume key in the
first call.

The third call adds a new key slot with the new passphrase.


There are IMHO two issues with these calls (regardless of the LUKS
format version):

  1. It only adds the new slot *after* deleting the old one, so there is
     moment where the LUKS header might have no active key slot left.
     Worse, if crypt_keyslot_add_by_volume_key() fails for whichever
     reason, then the header is left in a broken state; if the user
     doesn't notice and closes the mapped device (or simply reboots)
     then the entire content of the device is lost.

  2. The second argument of crypt_keyslot_add_by_volume_key() is always 0,
     while the user might want to change another key slot.

I'm unfortunately not familiar with libblockdev, but the attached
program, to be linked against libcryptsetup, shows these problems
AFAICT.


Format a device as LUKSv1 (although the same happens with v2), with a
random passphrase for key slot 0, and passphrase “test” for key slot
1.  (The extra PBKDF argument are there just so the test doesn't take
too long.)
    
    $ dd if=/dev/zero of=/tmp/disk.img bs=1M count=64
    $ head -c 32 /dev/urandom >/tmp/keyslot0
    $ cryptsetup luksFormat --pbkdf-force-iterations 1000 --type luks1 \
        -q --key-file=/tmp/keyslot0 /tmp/disk.img
    $ cryptsetup luksAddKey --pbkdf-force-iterations 1000 \
        --key-file=/tmp/keyslot0 -q /tmp/disk.img <<<test
    $ ./928893 /tmp/disk.img "test" "test2"
    Key slot 0 is full, please select another one.
    928893: Error: crypt_keyslot_add_by_volume_key

The third API call fails because key slot 0 is already enabled.  But key
slot 1 was already removed.


Re-format the device as LUKSv2, with a single key slot unlocked with
passphrase “test”.

    $ cryptsetup luksFormat --pbkdf-force-iterations 4 --pbkdf-memory 32 \
        --type luks2 -q /tmp/disk.img <<<test
    $ ./928893 /tmp/disk.img "test" "test2"
    Failed to initialise default LUKS2 keyslot parameters.
    928893: Error: crypt_keyslot_add_by_volume_key

Ka-boom, the only key slot was removed but not re-created.  The whole
device will be lost when the user reboot.  This *may* also happen with v1
if for whatever reason (hardware failure for instance) the new key slot
can't be added.  But it *always* happens with LUKSv2, which is Buster's
default LUKS format version for `luksFormat`.

Investigating further why crypt_keyslot_add_by_volume_key() *always*
fails with LUKSv2, with some help from gdb I believe the calltrace goes
as follows:

    lib/setup.c:crypt_keyslot_add_by_volume_key()
        calls crypt_keyslot_add_by_key() at line 3414
    lib/setup.c:crypt_keyslot_add_by_key()
        calls LUKS2_keyslot_params_default() at line 5291
    lib/luks2/luks2_keyslot.c:LUKS2_keyslot_params_default()
        calls crypt_keyslot_get_encryption() at line 154
    lib/setup.c:crypt_keyslot_get_encryption()
        calls crypt_get_volume_key_size() at line 4634
    lib/setup.c:crypt_get_volume_key_size()
        calls LUKS2_get_volume_key_size() at line 4550, which returns -1
        and cd->volume_key is NULL so crypt_get_volume_key_size()
        returns 0

LUKS2_get_volume_key_size() fails because the key size is specified in
the ‘keyslots’ object of LUKSv2's JSON header [0], and that object is
the empty array at that point.  The volume key is known to the caller,
but not to crypt_keyslot_get_encryption().  This is arguably a libcryptsetup
bug, but blindly calling crypt_keyslot_add_by_volume_key() *after*
crypt_keyslot_destroy() is extremely problematic in itself, and
usingcrypt_keyslot_add_by_volume_key() on an header without any
key-slots is of limited interest.  As intrigeri wrote, libblockdev 2.22
fixes the problem by replacing these 2 libcryptsetup API calls with the
more robust crypt_keyslot_change_by_passphrase() (available in
libcryptsetup ≥1.6.0).

Apologies for incorrectly pointing to getrlimits(2) earlier: I'm
personally not familiar with udisks/libblockdev myself and hadn't
realized `getrlimit(RLIMIT_MEMLOCK,)` was bypassed since the Argon2d/i
benchmark process is privileged.

Cheers,
-- 
Guilhem.

[0] https://gitlab.com/cryptsetup/LUKS2-docs/blob/master/luks2_doc_wip.pdf
    sec. 3.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 928893.c
Type: text/x-csrc
Size: 931 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-utopia-maintainers/attachments/20190720/907e63eb/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-utopia-maintainers/attachments/20190720/907e63eb/attachment.sig>