Project

General

Profile

Feature #15450

Create LUKS2 persistent volumes by default

Added by je843 over 1 year ago. Updated 8 months ago.

Status:
In Progress
Priority:
Normal
Assignee:
-
Category:
Persistence
Target version:
-
Start date:
03/23/2018
Due date:
% Done:

0%

Feature Branch:
wip/feature/15450-switch-to-luks2, t-p-s:feature/15450-switch-to-luks2,
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

Cryptsetup 2.0.x supports the LUKS2 format that includes the Argon2i and Argon2id hash algorithms. Argon2 is a memory-hard hash that strengthens low-entropy passphrases.
Attacker costs should be much higher then the current Cryptsetup 1.X which uses PBKDF2 which hashes with SHA-256.

cryptsetup allows converting existing LUKS volumes to LUKS2. But for the first iteration, let's only consider using LUKS2 for newly created persistent volumes.


Related issues

Related to Tails - Feature #14468: Add VeraCrypt support to Tails In Progress 03/01/2017 07/30/2018
Blocked by Tails - Bug #15460: Test suite broken with Java 9+ In Progress 03/27/2018
Blocked by Tails - Feature #15944: Port Tails to Buster Resolved 09/12/2018

Associated revisions

Revision 4029cbd2 (diff)
Added by CyrilBrulebois 8 months ago

Switch to LUKS2 in tails-persistence-setup (refs: #15450).

In buster, cryptsetup defaults to LUKS2 already, but UDisks2 doesn't.
Let's specify luks2 as the desired encryption type when formatting
through UDisks2.

Revision 024b5737 (diff)
Added by CyrilBrulebois 8 months ago

Test suite: ensure persistence partition is LUKS 2 (refs: #15450).

Revision df1248df (diff)
Added by CyrilBrulebois 8 months ago

Switch to LUKS2 in tails-persistence-setup (refs: #15450).

In buster, cryptsetup defaults to LUKS2 already, but UDisks2 doesn't.
Let's specify luks2 as the desired encryption type when formatting
through UDisks2.

Revision 16b0f1c5 (diff)
Added by CyrilBrulebois 8 months ago

Test suite: ensure persistence partition is LUKS 2 (refs: #15450).

History

#1 Updated by intrigeri over 1 year ago

  • Status changed from New to Confirmed
  • Target version set to Tails_4.0

Cryptsetup 2.0.x supports the LUKS2 format that includes the Argon2i and Argon2id hash algorithms.
Argon2 is a memory-hard hash that strengthens low-entropy passphrases.
Attacker costs should be much higher then the current Cryptsetup 1.X which uses PBKDF2 which hashes with SHA-256.

I'm looking forward to switching to LUKS2 but I think it's premature as the cryptsetup 2.0.2 release notes read:

Please note that authenticated disk encryption, non-cryptographic
data integrity protection (dm-integrity), use of Argon2 Password-Based
Key Derivation Function and the LUKS2 on-disk format itself are new
features and can contain some bugs.
[…]
Please do not use LUKS2 without properly configured backup […]

Let's revisit this closer to the Tails 4.0 (Buster) release.

It is not in use for Debian yet but is/will be implemented in Ubuntu 18.04

FWIW I think Tails 3.9 will need cryptsetup 2.x for #14468. But that does not mean we'll start creating LUKS2 persistent volumes that early.

#2 Updated by intrigeri over 1 year ago

  • Subject changed from Upgrade to cryptsetup 2.0.X to hash passphrase with Argon2 for persistent volume to Upgrade to LUKS2 persistent volumes with Argon2 Password-Based Key Derivation Function

#3 Updated by intrigeri over 1 year ago

  • Subject changed from Upgrade to LUKS2 persistent volumes with Argon2 Password-Based Key Derivation Function to Consider upgrading to LUKS2 persistent volumes with Argon2 Password-Based Key Derivation Function
  • Description updated (diff)
  • Type of work changed from Code to Research

#4 Updated by intrigeri over 1 year ago

  • Priority changed from Normal to Low

#5 Updated by u over 1 year ago

#6 Updated by intrigeri about 1 year ago

intrigeri wrote:

FWIW I think Tails 3.9 will need cryptsetup 2.x for #14468. But that does not mean we'll start creating LUKS2 persistent volumes that early.

FTR we did without cryptsetup 2.x in Tails 3.9 (the only drawback AFAIK is that we're not supporting VeraCrypt volumes with a custom PIM) so anyone willing to work on this, please target Tails 4.0 (Buster).

#7 Updated by intrigeri 10 months ago

FWIW Fedora 30 should create LUKS2 by default.

#8 Updated by intrigeri 9 months ago

cryptsetup (2:2.1.0-1) unstable; urgency=medium

  * New upstream release.  Highlights include:
    - The on-disk LUKS format version now defaults to LUKS2 (use `luksFormat
      --type luks1` to use LUKS1 format). Closes: #919725.

And with this version:

$ cryptsetup --help
[…]
Default PBKDF for LUKS2: argon2i
    Iteration time: 2000, Memory required: 1048576kB, Parallel threads: 4
[…]

So we should have this for free on feature/buster once the package migrates to testing. And then:

  1. We should check that tails-persistence-setup indeed creates LUKS2 by default (who knows, perhaps it calls udisks in a way that creates LUKS1, or udisks defaults to LUKS1).
  2. Our test suite will tell us if we need to adjust anything on our side.

#9 Updated by intrigeri 8 months ago

  • Subject changed from Consider upgrading to LUKS2 persistent volumes with Argon2 Password-Based Key Derivation Function to Create LUKS2 persistent volumes by default
  • Priority changed from Low to Normal

#10 Updated by intrigeri 8 months ago

  • Type of work changed from Research to Test

#11 Updated by CyrilBrulebois 8 months ago

  • Assignee set to CyrilBrulebois

For reference, LUKS1 leads to:

kibi@armor:~/work/clients/tails/persistence-setup.git$ sudo cryptsetup status /dev/mapper/luks1
/dev/mapper/luks1 is active.
  type:    LUKS1
  cipher:  aes-xts-plain64
  keysize: 256 bits
  device:  /dev/loop0
  loop:    /tmp/luks1.img
  offset:  4096 sectors
  size:    200704 sectors
  mode:    read/write

while LUKS2 leads to:

(sid-amd64-devel)kibi@armor:~/work/clients/tails/persistence-setup.git$ sudo cryptsetup status /dev/mapper/luks2
/dev/mapper/luks2 is active.
WARNING: Locking directory /run/cryptsetup is missing!
  type:    LUKS2
  cipher:  aes-xts-plain64
  keysize: 512 bits
  key location: dm-crypt
  device:  /dev/loop1
  loop:    /tmp/luks2.img
  sector size:  512
  offset:  32768 sectors
  size:    172032 sectors
  mode:    read/write

Test changes

Looking at features/step_definitions/usb.rb, it seems:

  • the cryptsetup status vs. device: matching will likely be unaffected.
  • the cryptsetup luksOpen will likely be unaffected.
  • checking the output of udisksctl info --block-device from stretch (didn't run the daemon inside my buster/sid chroot yet), it seems we might want to add an extra check on \s+IdVersion:\s+1 vs. \s+IdVersion:\s+2 to distinguish between LUKS and LUKS2.

Code changes

On the format side, looking at the udisks2 documentation

The Format() method

Format (IN s type, IN a{sv} options);

If the option encrypt.passphrase is given then a LUKS device is created with the given passphrase and the file system is created on the unlocked device. The unlocked device will be left open. This parameter can be used to supply the binary contents of an arbitrary keyfile by passing a value of type “ay”. Option encrypt.type can be used to specify encryption "technology" that will be used. Currently only “luks1” and “luks2” are supported.

which I assume should be set here:

    my $fstype    = $self->persistence_filesystem_type;
    my $fsoptions = {
        %{$self->persistence_filesystem_options},
        'encrypt.passphrase' => $opts->{passphrase},
    };

which is then used right below:

    $self->udisks_service->get_object($self->persistence_partition)
         ->as_interface('org.freedesktop.UDisks2.Block')
         ->Format(
             dbus_call_async, dbus_call_timeout, 3600 * 1000,
             $fstype, $fsoptions)
         ->set_notify(sub { $opts->{end_cb}->({
             created_device => $created_device,
             format_reply   => @_,
         })});

I'll try forcing luks1 and luks2, to double check.

#12 Updated by intrigeri 8 months ago

If the option encrypt.passphrase is given then a LUKS device is created with the given passphrase and the file system is created on the unlocked device. The unlocked device will be left open. This parameter can be used to supply the binary contents of an arbitrary keyfile by passing a value of type “ay”. Option encrypt.type can be used to specify encryption "technology" that will be used. Currently only “luks1” and “luks2” are supported.

which I assume should be set here:
[...]
I'll try forcing luks1 and luks2, to double check.

On current feature/buster, tails-persistence-setup creates LUKS1 volumes. So yeah, we should pass encrypt.type=luks2 there.

#13 Updated by CyrilBrulebois 8 months ago

I've tried to add a check in the test suite, but features/persistence.feature seems to be hitting some issues at the moment, so I went for double checking from within a feature/buster image → luks1 by default; and starting over, adding such a line in persistence-setup, led to luks2.

diff --git a/lib/Tails/Persistence/Setup.pm b/lib/Tails/Persistence/Setup.pm
index 0a83c5a..c00deae 100644
--- a/lib/Tails/Persistence/Setup.pm
+++ b/lib/Tails/Persistence/Setup.pm
@@ -571,6 +571,7 @@ method create_persistent_encrypted_filesystem (
     my $fsoptions = {
         %{$self->persistence_filesystem_options},
         'encrypt.passphrase' => $opts->{passphrase},
+        'encrypt.type'       => 'luks2',
     };

     $self->udisks_service->get_object($self->persistence_partition)

Thinking how to ship this… Publishing a new tails-persistence-setup version with a mandatory luks2 could hinder our ability to use updated versions for the next bugfix releases. So I was thinking about a local patch in our feature/buster branch instead.

@intrigeri: does that look about right?

#14 Updated by intrigeri 8 months ago

So I was thinking about a local patch in our feature/buster branch instead.

intrigeri: does that look about right?

Absolutely: e.g. we have a couple config/chroot_local-patches/*buster.diff already, that are supposed to come from the corresponding feature/buster branch of the relevant packaging repo.

Hint that might save you a couple manpage reading minutes:

(cd persistence-setup/lib && git diff --src-prefix=a/usr/share/perl5/ --dst-prefix=b/usr/share/perl5/ --relative=lib master.. . ) > ~/tails/git/config/chroot_local-patches/tps-buster.diff

#15 Updated by CyrilBrulebois 8 months ago

  • Status changed from Confirmed to In Progress

#16 Updated by CyrilBrulebois 8 months ago

  • QA Check set to Ready for QA

Pushed feature/15450-switch-to-luks2 which contains the local patch and an update to the test suite, and pushed a buster branch in persistence-setup.git (which in hindsight could have/should have been a topic branch instead…).

I haven't been able to run the test suite successfully (on two different machines, so probably not a possible apparmor issue), and I'll try to get back to that once the sprint has yielded results on the test suite front.

#17 Updated by CyrilBrulebois 8 months ago

  • QA Check changed from Ready for QA to Dev Needed

Switching back to Dev Needed. I failed to spot the crux of the issue with the updated feature/buster branch: my test suite changes need work.

#18 Updated by CyrilBrulebois 8 months ago

I'll need to clean up my commits and push an updated branch, but results are better already:

6 scenarios (1 failed, 5 passed)
46 steps (1 failed, 45 passed)

We'll need to rework something in the test suite though, as our test suite target is stretch, which doesn't know about LUKS2… The failing scenario is:

cucumber features/persistence.feature:23 # Scenario: Writing files to a read/write-enabled persistent partition

Trace excerpt:

    Then only the expected files are present on the persistence partition on USB drive "__internal"               # features/step_definitions/usb.rb:680
      luks_open: Unsupported LUKS version 2. (Guestfs::Error)
      ./features/step_definitions/usb.rb:699:in `luks_open'
      ./features/step_definitions/usb.rb:699:in `block (2 levels) in <top (required)>'
      ./features/support/helpers/storage_helper.rb:218:in `guestfs_disk_helper'
      ./features/step_definitions/usb.rb:689:in `/^only the expected files are present on the persistence partition on USB drive "([^"]+)"$/'
      features/persistence.feature:31:in `Then only the expected files are present on the persistence partition on USB drive "__internal"'

#19 Updated by CyrilBrulebois 8 months ago

(Updated branch pushed earlier today.)

So this issue comes from this line (features/step_definitions/usb.rb:695 in my branch, with an offset due to some line addition above):

    g.luks_open(partition, @persistence_password, luks_mapping)

FWIW, there's another such luks_open call here, in the disk_mkpartfs function:

features/support/helpers/storage_helper.rb:        g.luks_open(primary_partition, opts[:luks_password], luks_mapping)

AFAICT, luks_open is exported by the libguestfs ruby bindings, and calls cryptsetup directly (see daemon/luks.c in its source). So it looks like upgrading libguestfs wouldn't help, one would have to upgrade cryptsetup

I'm not sure whether it'd be reasonable to require a cryptsetup backport (which doesn't exist in stretch-backports at the moment) to run the test suite…

@intrigeri, @anonym: what do you think?

(FWIW, already clocked 3 hours on that topic, between research, code, test suite, and digging through documentation / source files.)

#20 Updated by intrigeri 8 months ago

  • Assignee deleted (CyrilBrulebois)
  • Target version deleted (Tails_4.0)
  • Type of work changed from Test to Code

Thanks for this research!

We had reclassified this ticket as "let's do it for 4.0" a few days ago under the assumption that it was merely about passing one option in t-p-s and adjusting the test suite a tiny bit. Thanks to your research, it appears it's not the case, so this qualifies more as a new feature rather than as a nice bonus improvement we can get cheaply merely by upgrading to Buster ⇒ let's come back to it once we can run the test suite on Buster or newer, i.e. when it'll really be cheap :) I'll adjust metadata accordingly.

#21 Updated by intrigeri 8 months ago

  • Blocked by Bug #15460: Test suite broken with Java 9+ added

#22 Updated by intrigeri 8 months ago

#23 Updated by intrigeri 8 months ago

  • Feature Branch set to feature/15450-switch-to-luks2, t-p-s:feature/15450-switch-to-luks2,

Renamed the t-p-s topic branch as this is not targeting 4.0 anymore.

#24 Updated by intrigeri 8 months ago

  • Feature Branch changed from feature/15450-switch-to-luks2, t-p-s:feature/15450-switch-to-luks2, to wip/feature/15450-switch-to-luks2, t-p-s:feature/15450-switch-to-luks2,

Also available in: Atom PDF