agherzan / yubikey-full-disk-encryption

Use YubiKey to unlock a LUKS partition
Apache License 2.0
795 stars 50 forks source link

Cannot enable `discard` using YFDE #35

Closed b0ch3nski closed 5 years ago

b0ch3nski commented 5 years ago

With YFDE, I cannot get the discard to work in any way...

According to the documentation, YFDE should read configuration from cryptdevice kernel parameter. This works, but not for allow-discards part.

Grub config - /etc/default/grub:

GRUB_CMDLINE_LINUX="cryptdevice=UUID=...:some-luks-vol:allow-discards ..."

Following line was enough to enable discard without YFDE.

So I've tried to force it in /etc/ykfde.conf:

# Optional flags passed to 'cryptsetup'. Example: "--allow-discards" for TRIM
# support. Leave empty to use 'cryptdevice' boot parameter.
YKFDE_LUKS_OPTIONS="--allow-discards"

Regenerated the initramfs, rebooted, and still no go. Executing dmsetup table doesn't show that discard is supported.

Vincent43 commented 5 years ago

Thank you for the report. I'm not using discard myself but I'll try to find the culprit. Maybe @stuffo who is the author of discard support have any idea?

agherzan commented 5 years ago

I can't see the obvious issue here either. Let's wait for the author for a while. We can dig further if need be.

Vincent43 commented 5 years ago

@b0ch3nski can you try if https://github.com/agherzan/yubikey-full-disk-encryption/commit/b8e148a574a27f1a9926051a54de4656cc5bd6de fixed this for you?

b0ch3nski commented 5 years ago

I've tried the current code base. With debugging enabled, I can see that cryptsetup luksOpen is, at least in theory, invoked with --allow-discards parameter.

Unfortunately, it has no effect - dmsetup table still doesn't show that discard is enabled.

Just to make sure that nothing has broken in between, I've regenerated initramfs again, with ykfde hook replaced with encrypt, rebooted, and then discard is back again. So the problem must be somewhere in YFDE code :cry:

Vincent43 commented 5 years ago

Hm, I switched to the same code that Arch encrypt hook uses. If --allow-discards is already passed for cryptsetup luksOpen then I don't know what else we could do.

What type of LUKS container do you use? Is it LUKS2? Did you formatted it with --integrity option?

You may try removing allow-discards from GRUB_CMDLINE_LINUX and add it only for YKFDE_LUKS_OPTIONS in ykfde.conf.

You may also try booting from external media and open LUKS container through ykfde-open with YKFDE_LUKS_OPTIONS="--allow-discards" in ykfde.conf to see if it works this way.

b0ch3nski commented 5 years ago

Simply switching from ykfde hook to encrypt enables discard again, without any other changes, so there must be some difference over the code :thinking:

I'm using LUKS1. I've also tried previously forcing it by setting YKFDE_LUKS_OPTIONS="--allow-discards" - it also has no effect... Booting from another media is quite a lot of work and I might try it in spare time, but I cannot promise when that would be.

Vincent43 commented 5 years ago

When you set YKFDE_LUKS_OPTIONS="--allow-discards", did you also remove allow-discards from GRUB_CMDLINE_LINUX? If not then try removing it from there.

You may also try creating LUKS container in a file (or unused partition, usb device) for test with ykfde-format and open it with ykfde-open with YKFDE_LUKS_OPTIONS="--allow-discards" in ykfde.conf.

If nothing works then you may also treat it as a feature as using discard in LUKS is generally recommended against as it leaks data usage patterns :smile:.

HacKanCuBa commented 5 years ago

I'm not sure if this is going to help you, but the setting to allow discards in /etc/crypttab is simply discard.

Vincent43 commented 5 years ago

We don't use /etc/crypttab. We pass --allow-discards to cryptsetup directly on unlock.

b0ch3nski commented 5 years ago

I've solved my issue by converting to LUKS2 and using persistent flags as described in Arch wiki.

The root cause is still unknown, but at least it works now :smiley:

Vincent43 commented 5 years ago

@b0ch3nski it took quite long time but I finally fixed this issue with https://github.com/agherzan/yubikey-full-disk-encryption/commit/50e45b6c219b2acea89760fef4abbae938661262

Here's explanation from commit description:

'allow-discards wasn't working for both automatic reading 'cryptdevice' kernel parameter and 'YKFDE_LUKS_OPTIONS' variable from 'ykfde.conf' due to following reasons:

  • YKFDE_LUKS_OPTIONS was unconditinally overwritten by 'cryptargs' from 'cryptdevice' kernel parameter even when the latter was empty.

  • Original 'cryptargs' paramter was initialized with an empty value which resulted with an additional whitespace character before '--allow-discards' which therefore wasn't properly passed to cryptsetup thus was ignored.

This commit fixes both cases.