[systemd-bugs] [Bug 66396] RFE: implement --header option in systemd-cryptsetup/crypttab

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=66396

--- Comment #3 from Zbigniew Jedrzejewski-Szmek <zbyszek at in.waw.pl> ---
Comment on attachment 103783
  --> https://bugs.freedesktop.org/attachment.cgi?id=103783
systemd-cryptsetup patch

Review of attachment 103783:
-----------------------------------------------------------------

::: 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?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-bugs/attachments/20140803/f5dd1554/attachment-0001.html>