<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - RFE: implement --header option in systemd-cryptsetup/crypttab"
href="https://bugs.freedesktop.org/show_bug.cgi?id=66396#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - RFE: implement --header option in systemd-cryptsetup/crypttab"
href="https://bugs.freedesktop.org/show_bug.cgi?id=66396">bug 66396</a>
from <span class="vcard"><a class="email" href="mailto:zbyszek@in.waw.pl" title="Zbigniew Jedrzejewski-Szmek <zbyszek@in.waw.pl>"> <span class="fn">Zbigniew Jedrzejewski-Szmek</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=103783" name="attach_103783" title="systemd-cryptsetup patch">attachment 103783</a> <a href="attachment.cgi?id=103783&action=edit" title="systemd-cryptsetup patch">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=66396&attachment=103783'>[review]</a>
systemd-cryptsetup patch
Review of <span class=""><a href="attachment.cgi?id=103783" name="attach_103783" title="systemd-cryptsetup patch">attachment 103783</a> <a href="attachment.cgi?id=103783&action=edit" title="systemd-cryptsetup patch">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=66396&attachment=103783'>[review]</a>:
-----------------------------------------------------------------
::: src/cryptsetup/cryptsetup.c
@@ +140,5 @@
<span class="quote">> + } 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);</span >
That seems dangerous. The whole --header business is pretty dangerous, so any
misconfiguration should be treated as fatal.
@@ +143,5 @@
<span class="quote">> + 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);</span >
Let's make this fatal too. If there are two header= options (i.e. arg_header
was already set), refuse to continue.
@@ +405,4 @@
<span class="quote">> r = crypt_load(cd, CRYPT_LUKS1, NULL);
>
> + if (data_device)
> + r = crypt_set_data_device(cd, data_device);</span >
This assignment to r would overwrite the previous value, which hasn't been
checked yet.
@@ +578,5 @@
<span class="quote">> k = crypt_init(&cd, argv[3]);
> +
> + if(arg_header) {
> + log_info("LUKS header: %s", arg_header);
> + k = crypt_init(&cd, arg_header);</span >
Please make this log_debug.
@@ +634,4 @@
<span class="quote">> 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);</span >
I don't grok this. Why argv[3] is used if arg_header is set?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the QA Contact for the bug.</li>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>