[pkg-cryptsetup-devel] Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files

[Christoph Anton Mitterer]

Hey.

Thanks for you reply. :-)

I'm still a bit unsure how to do it right, in certain aspects:

1) crypttab_find_entry()
What would be a use case for that?

I mean in a keyscript, CRYPTTAB_* are anyway already set for the
"current" target, right?
And in a initramfs hook, I anyway need to loop over all of them... or
at least I wouldn't have a particular (target) name to search for?




2) crypttab_foreach_entry()
That's what I'd use in the hook. But you've mentioned that the
callbacks return value is ignored.
Could that be changed perhaps?

In my case it's probably not a big deal:
- if the callback would find that the "current" entry isn't meant for
  "my" script, then it could just return without doing anything
- if however an error occurs (e.g. no pathname= set) it would anyway
  just exit 1 the whole hook, since it's "catatrophic" failure and an
  initramfs level device couldn't be unlocked

Still it might be nice to determine outside of the foreach to determine
what to do.
Of course one could just set a "global" variable, indicating an error
and set from within the callback.


Also in crypttab_foreach_entry(), do I already have CRYPTTAB_OPTION_*?
Or really just the four CRYPTTAB_{NAME,SOURCE,KEY,OPTIONS} and I need
to call crypttab_parse_options to get the split up ones?

But in keyscripts I would already have CRYPTTAB_OPTION_*, too, right?



3) Escaping
My understanding is, that in both, the keyscript and the hook (when
using e.g. crypttab_foreach_entry()) any CRYPTTAB_* is already
unescaped, right?

You also mention above, that CRYPTTAB_OPTIONS is not safe to use, when
values contain ",".
I assume this is because, the unescaped CRYPTTAB_OPTIONS would contain
both, the "," from the values and the "," from the separators?
While CRYPTTAB_OPTION_* would take care of that properly?

So could I do basically something like this for the 4th field:
luks,keyscript=stupid\054name
and I'd get
CRYPTTAB_OPTIONS='luks,keyscript=stupid,name'
but
CRYPTTAB_OPTION_luks='yes'
CRYPTTAB_OPTION_keyscript='stupid,name'


For which fields are the octal escapes handled? The manpage only
mentions them for them for the key/3rd field.



4) Wishlist ;-)
Can we have something like the option splitting for the key/3rd field,
too?

You remember what I did? E.g.:
device=/dev/disk/by-label/boot-usb-stick:pathname=/path-on-that-device/key.gpg

Obviously there's the same issue, if some value would contain my
separator character (I've used : cause I wasn't sure if it troubles the
parsers from crypttab if I use , ... but I'd happily change to
something recommended from upstream).

I think there might be more keyscripts that benefit from this:
The fourth fields is rather for general crypttab options and I don't
think it would be wise if keyscript-specific options would be put in
there (mostly because they could collide with different keyscripts).

To me, the natural place or any options related to retrieving the key
is the 3rd field.
That would also include e.g. hostnames/ports for a keyscript that
retrieves the key via SSH,... or maybe if one uses a smartcard that can
hold multiple keys, the identifier of that cards keyslot.


I guess that would work similar to crypttab_parse_options? Maybe just a
different name crypttab_parse_keyfile_as_options?
The unsetting of variables you do there... that might be difficult to
do, since we have no idea how these options could be named, e.g.
CRYPTTAB_KEYFILEOPTION_device

I also don't think it’s easily possible to unset any
CRYPTTAB_KEYFILEOPTION_* variables.

"set" lists all name=value, but these may contain newlines and I think
it might not be easy to sanitize that.
$ dash
$ set
COLORTERM='truecolor'
DBUS_SESSION_BUS_ADDRESS='unix:path=/run/user/1000/bus'
...
HOME='/home/calestyo'
IFS=' 	
'
LANG='en_DE.UTF-8'


One could have a var like:
FOO='
BAR=baz
'


However,... it might actually work to do this generically:
If dash's syntax is really always:
name=value-with-some-quoting
with the 2nd ' being possibly in another line, we could do e.g.

set | sed -n 's/^\(CRYPTTAB_KEYFILEOPTION_[a-zA-Z_0-9]\+\)=.*$/\1/p'

and unset everything that results.
Sure this could contain some false positives, if e.g. someone had set
BAR='
CRYPTTAB_KEYFILEOPTION_foo='"'"'is not a var'"'"'
'

we would also get CRYPTTAB_KEYFILEOPTION_foo, but who cares? It's "our"
namespace.


Maybe you could also use that for unsetting CRYPTTAB_OPTION_*



In the end,... and you'll probably not like it ^^ ... I'd even suggest
to rename the 3rd filed to something more generic... just KEY or
KEYOPTIONS or so.
Simply to make it clear that this doesn't have to be a file.



Cheers,
Chris.