ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
63 stars 60 forks source link

zkey crypttab generates invalid entries for plain type devices #55

Closed xnox closed 5 years ago

xnox commented 5 years ago

The correct syntax as implemented by systemd-cryptsetup and by debian's crypttab package crypttab process is ',plain' or ',luks2' etc to specify the type. 'hash=plain' is invalid for a plain type entry.

ifranzki commented 5 years ago

I understand about 'hash=plain' being invalid (I will fix this), but I don't understand about ',plain' or ',luks2'. Why the comma in front of plain or luks?

The generated crypttab entry currently looks as follows: enc-disk /dev/dasda1 /etc/zkey/repository/plain.skey plain,cipher=paes-cbc-plain64,size=512,hash=plain

So the 'plain' keyword is as first in the options, so I don't think that there should be a comma in front of it.

ifranzki commented 5 years ago

And is it really 'luks2' for a LUKS2 volume, or just 'luks' ? The man page of crypttab specifies only 'luks'. I would assume that it finds out by itself it is luks1 or luks2.

xnox commented 5 years ago

I understand about 'hash=plain' being invalid (I will fix this), but I don't understand about ',plain' or ',luks2'. Why the comma in front of plain or luks?

I was trying to point out that it is just a "key" by itself, not a "key=value" parameter. as in 'plain' by itself is the correct syntax. Sorry for confusion.

The generated crypttab entry currently looks as follows: enc-disk /dev/dasda1 /etc/zkey/repository/plain.skey plain,cipher=paes-cbc-plain64,size=512,hash=plain

So systemd-cryptsetup i believe does choke up on the ',hash=plain' bit and it should be removed. Otherwise it is valid way to declare the type the "plain," start of it.

However, on Ubuntu, we do not normally specify devices as /dev/kernelname instead we use UUID entries, eg. things like this:

nvme0n1p7_crypt UUID=a228a613-c3d2-42ec-8799-2b637afc4dce none luks,discard

is the structure of entries that I would like cryptsetup crypttab generate. ie. Use UUID= and always specify discard option.

So the 'plain' keyword is as first in the options, so I don't think that there should be a comma in front of it.

yes yes

xnox commented 5 years ago

And is it really 'luks2' for a LUKS2 volume, or just 'luks' ? The man page of crypttab specifies only 'luks'. I would assume that it finds out by itself it is luks1 or luks2.

I believe the crypttab parser in Debian/Ubuntu cryptsetup package for initramfs-tools unlocking wants a specifier. You are correct that cryptsetup can work out if things are luks1 or luks2 based on luks keyword, but i wonder if we want to be more specific or not. Let me get back on that.

ifranzki commented 5 years ago

However, on Ubuntu, we do not normally specify devices as /dev/kernelname instead we use UUID entries, eg. things like this: nvme0n1p7_crypt UUID=a228a613-c3d2-42ec-8799-2b637afc4dce none luks,discard is the structure of entries that I would like cryptsetup crypttab generate. ie. Use UUID= and always specify discard option.

Currently zkey crypttab generates entries with the device name exactly as it is associated with the key. When generating a key you can associate volumes with the key. The user is free to specify whatever device name there (as long as it exists). For using the UUID I would recommend to use /dev/disk/by-uuid/<uuid> . That way the device in the crypttab entry will also use that format.

Generating entries with UUID=<uuid> would need more logic inside zkey to get the UUID of the device associated with the key. Probably libblkid could be used to obtain the UUID from a device path. I will put that on the list for future enhancements...

… and always specify discard option.

Regarding the discard option: The man page of cryptsetup warns that this option has security risks. Because of that I would rather not include this option per default. The crypttab entries generated by zkey are meant more as a skeleton, and the user is free to change the generated entries as needed.