Original cryptsetup has an --header option: man cryptsetup: "--header <device or file storing the LUKS header> Use a detached (separated) metadata device or file where the LUKS header is stored. This options allows to store ciphertext and LUKS header on different devices." But there are no such option in systemd-cryptsetup. I cannot use it in crypttab too. What about of it's implementation? It can be used for creation "hidden" volumes, which are not contains any metadata about chipper end encryption type. From my point of view, it will be useful feature.
Created attachment 103783 [details] [review] systemd-cryptsetup patch I created patch. Please, can you review this?
/etc/crypttab example: mycryptdisk /dev/sdc /root/luks.key luks,header=/root/secure/luksheader.img
Comment on attachment 103783 [details] [review] systemd-cryptsetup patch Review of attachment 103783 [details] [review]: ----------------------------------------------------------------- ::: src/cryptsetup/cryptsetup.c @@ +140,5 @@ > + } else if (startswith(option, "header=")) { > + arg_type = CRYPT_LUKS1; > + > + if(!path_is_absolute(option+7)) > + log_error("Header path '%s' is not absolute. Ignoring.", option+7); That seems dangerous. The whole --header business is pretty dangerous, so any misconfiguration should be treated as fatal. @@ +143,5 @@ > + if(!path_is_absolute(option+7)) > + log_error("Header path '%s' is not absolute. Ignoring.", option+7); > + > + free(arg_header); > + arg_header = strdup(option+7); Let's make this fatal too. If there are two header= options (i.e. arg_header was already set), refuse to continue. @@ +405,4 @@ > r = crypt_load(cd, CRYPT_LUKS1, NULL); > > + if (data_device) > + r = crypt_set_data_device(cd, data_device); This assignment to r would overwrite the previous value, which hasn't been checked yet. @@ +578,5 @@ > k = crypt_init(&cd, argv[3]); > + > + if(arg_header) { > + log_info("LUKS header: %s", arg_header); > + k = crypt_init(&cd, arg_header); Please make this log_debug. @@ +634,4 @@ > if (streq_ptr(arg_type, CRYPT_TCRYPT)) > k = attach_tcrypt(cd, argv[2], key_file, passwords, flags); > else > + k = attach_luks_or_plain(cd, argv[2], key_file, ( arg_header ? argv[3] : NULL), passwords, flags); I don't grok this. Why argv[3] is used if arg_header is set?
I made some notes on the implementation. Looks reasonable in general, but please include changes to cryptsetup.xml and other man pages as necessary.
Created attachment 111105 [details] [review] systemd-cryptsetup patch
Created attachment 111106 [details] [review] crypttab.xml patch
(In reply to Zbigniew Jedrzejewski-Szmek from comment #3) > @@ +634,4 @@ > > if (streq_ptr(arg_type, CRYPT_TCRYPT)) > > k = attach_tcrypt(cd, argv[2], key_file, passwords, flags); > > else > > + k = attach_luks_or_plain(cd, argv[2], key_file, ( arg_header ? argv[3] : NULL), passwords, flags); > > I don't grok this. Why argv[3] is used if arg_header is set? argv[3] contains data_device if (and only if) arg_header is set. > + if (data_device) > + r = crypt_set_data_device(cd, data_device); From official docs: > int crypt_set_data_device (struct crypt_device * cd, const char * device) > Set data device For LUKS it is encrypted data device when LUKS header is separated. See also http://wiki.cryptsetup.googlecode.com/git/API/libcryptsetup_8h.html#aff3afff2a83a64f80962afa446eb9dca (
If arg_header is not set then argv[3] contains regular LUKS-device.
Applied in http://cgit.freedesktop.org/systemd/systemd/commit/?id=7376e83528.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.