Hi.

Fritst thanks for work you've done in the recent new versions. Sooo many
nice things have been implemented/fixed :-)

Unfortunately, it breaks booting with my personal openpgp keyscripts.

The problem seems that in earlier versions, the initramfs got this file:
main/conf/conf.d/cryptroot with:
target=system,source=/dev/disk/by-uuid/97d2d814-72f6-11e8-a274-742b62897688,rootdev,keyscript=/lib/cryptsetup/scripts/decrypt_openpgp,tries=0,key=device=/dev/disk/by-label/keyFilePart:pathname=/etc/dm-crypt/keys/keyfile_for_system

As you can see, I use the 3rd field of the crypttab, for giving addtional
options to the keyscript:
device=/dev/disk/by-label/keyFilePart		=> the device on which the keyfile is to be found at boot
pathname=/etc/dm-crypt/keys/keyfile_for_sysstem	=> the name of the keyfile on the rootfs of that device

This file is gone, but now there is
main/cryptroot/crypttab with:
system UUID=31a2a126-2947-47ad-a87e-f5b9cb0b6c8a device=/dev/disk/by-label/gss-boot-data_ec713fc2-901a-4f51-8ffe-b9f4df02537b:pathname=/etc/dm-crypt/keys/heisenberg.scientia.net_system loud,luks,keyscript=decrypt_openpgp,tries=0


1) Such a file/format change should go to the NEWS file ;-)
   This is one of the main reasons I reported #826122 back then
   to get that "interface" stable for 3rd party users
2) I assume main/cryptroot/crypttab can have multiple lines, right?
   How can I find out in my keyscript, which one is the right line
   for it right now (i.e. for the device the keyscript currently
   tries to open)?
3) Is there any documentaion of the (stable) format of main/cryptroot/crypttab?
   Cause it doesn't seem to be the same than the normal /etc/crypttab


Thanks,
Chris.

Hi Christoph,

:-)

I disagree, the location of this file and its format are internal
(undocumented) implementation details, so third-party keyscripts
shouldn't rely on this.  Please use the interface documented in
crypttab(5) to determine which device your keyscript is processing.
You should find the following in the keyscript's environment:

    CRYPTTAB_NAME=system
    CRYPTTAB_SOURCE=/path/to/source/device
    CRYPTTAB_KEY=device=/dev/disk/by-label/keyFilePart:pathname=/etc/dm-crypt/keys/heisenberg.scientia.net_system
    CRYPTTAB_OPTION_loud=yes
    CRYPTTAB_OPTION_luks=yes
    CRYPTTAB_OPTION_keyscript=/lib/cryptsetup/scripts/decrypt_openpgp
    CRYPTTAB_OPTION_tries=0

IMHO this bug should either be closed (not a bug) or, if there is a need
to improve the documentation, its severity lowered to wishlist, and its
title changed accordingly.  It's not a regression in either case; it's
in no way ‘important’ since you were the one shooting yourself in the
foot by relying on undocumented behavior :-P

Cheers,

Hey :-)

Sorry, I completely messed up my bug report ^^...

Actually, in the keyscript I already use CRYPTTAB_* ... it's the
initramfs hook which fails.

During the initramfs hook, I obviously won't have any of CRYPTTAB_*, or
do I?
But I do have the main/conf/conf.d/cryptroot (respectively now it's
main/cryptroot/crypttab - within the initramfs image).


So why do I need stuff from crypttab during initramfs generation?

Well the first thing is, I do some sanity checking already in the
initramfs hook, e.g. if there is no pathname= option my keyscript would
never be able to succeed, thus I already warn during initramfs
generation, that this initramfs will fail to boot.

Second thing is, in addition to device=, I alternatively offer an
option to simply include the key inside the initramfs, which is however
quite questionable from a security PoV,... so the hook script needs to
know that option and only if that is set, include the key (which is
taken again from pathname=).
Further, only if the key is read from some device, and not contained in
the initramfs image,... my hook scripts includes passdev (which I use
for reading) in the initramfs, otherwise it's not needed.
Last but not least if both, device= and "key_file_in_initramfs_images"
options are set, the initramfs hook script gives a warning, that one
will be ignored.



So in order to make the initramfs generation a bit more powerful, I
need the options for the device currently being processed.

Cheers,
Chris.

Control: severity -1 wishlist
Control: tag -1 - moreinfo
Control: retitle -1 cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files

I see, thanks for the explanation, sounds like a valid use case indeed.

Thus lowering the bug severity to ‘wishlist’ and retiling the bug
accordingly.

FWIW, what we're currently doing (so far undocumented and subject to
change) is to go through the system crypttab(5) and copy each entry
requiring unlocking at initramfs stage to $DESTDIR/cryptroot/crypttab
(the format of which is therefore analogous to crypttab(5)).  So the
following template should work when the hook file has ‘cryptroot’ as
prerequisite:

    . /lib/cryptsetup/functions

    [ -s "$DESTDIR/cryptroot/crypttab" ] || return 0
    while read CRYPTTAB_NAME CRYPTTAB_SOURCE CRYPTTAB_KEY CRYPTTAB_OPTIONS; do
        if [ "${CRYPTTAB_NAME#\#}" = "$CRYPTTAB_NAME" ] && \
                crypttab_parse_options "$CRYPTTAB_OPTIONS" n; then
            […]
        fi
    done <"$DESTDIR/cryptroot/crypttab"

But please note that this is subject to change until we document the
snippet and close this bug :-P

Cheers,

Well... it still broke some existing setups... it was always advertised
that people could make their own keyscripts and they simply used what
seemed usable and provided by cryptsetup ;-)

Guess that's why you saw quite a number of reports of people that saw
their stuff breaking... but anyway I don't really care which severity a
bug has, as long as there's going to be a nice solution :-)

Will that process only those crypttab entries which actually require to
be processed during initramfs?

Any idea already when this "API" is going to be stabilised? I'd prefer
to only adapt my stuff (&upgrade) once this is done. :D



One off topic things...
Why the whole split into cryptsetup-initramfs? Seems a bit overkill and
cosmetically less nice to me.

Wouldn't it have been the best if the cryptsetup initramfs hooks simply
only add stuff to the initramfs, if this was required for booting (or
didn't they do this already?), without any need to "configure/enable"
this by installing a package.
Right now this seems a bit error prone and kinda abusing the package
management for what should be rather configuration.

And if someone would have wanted to disable cryptsetup's initramfs-
tools integration, one could have simply provided a config file which
is sourced by the hooks and then exit if desired (or even better,
initramfs-tools should provide some masking mechanism,... like systemd
does when one adds symlinks to /dev/null in /etc/... with the same name
as the unit file shipped by the package in /usr/...


Cheers,
Chris.

Sure, people are welcome to write their own keyscripts.  But rather than
using an undocumented interface and assume it'll never break, the right
thing to do is to ask us to document said interface and commit to
maintain it.

Uh, are you referring to the regressions that were filed last week?  You
can't compare your own #901795 with these regressions, where we broke
setups following the documented interface…

Beside your bug, the only similar issue we heard of was reported to the
list by Marc Haber, who wrote he was “aware that [he's] using an
internal interface, hence not a bug report but this message”.

That being said, 2 reports doesn't mean there are only 2 users affected.
But I think it's fair to assume that the very vast majority of users
were not using our internal interface.

Yes.

Right now we'd like things to settle a bit, and fixing actual regression
have higher priority.  I'll plan to start working on this once the
package enters testing, but I'm not promising anything.

See the changelog entry for 2:2.0.3-1, and the 2 merged bug it closed.

See /etc/cryptsetup-initramfs/conf-hook.  We're deprecating
CRYPTSETUP=[y|n] now that the we split the initramfs integration in its
own package, though.

Hey. :-)

Well, back then when I "developed" my gnupg keyscript there was one in
cryptsetup which, IIRC, was even dysfunctional.
I tried to get Jonas mine included, but he didn't like it, though I
consider(ed) it pretty sophisticated (but I haven't looked at the
current decrypt_gnupg,... so maybe this is now fine, too).

So I kinda had "upstream contact" but things just got lost over time
;-)

If you like I can send you the full set of scripts&hooks for review.

There was no real criticism in my words :-) I just wanted to point out
that these things simply happen... people use such stuff if it's
visible ;-)
No blame on you guys!

Nice :-)

I'd now simply start to use the "interface" you suggested me in your
last mail. Question on that:

Why is this necessary? I assume when I PREREQ=cryptroot, than
$DESTDIR/cryptroot/crypttab finished(!) and contains all devices needed
to be unlocked during initramfs, right?
And I'd guess this is for the case that there are no such devices and
thus no "$DESTDIR/cryptroot/crypttab" would be even written (and my
hook wouldn't need to do anything either). Right?
Do you guys do any quoting in "$DESTDIR/cryptroot/crypttab"?
Cause read without -r will interpret \ as quoting character... and this
is IMO always a bit dangerous if the same is then used...
What is this intended for?
(Oh and did you guys notice that this is a bashism? ${var#word} is not POSIX sh compatbile)
I'd assume I don't need this, if I don't use the options like SIZE,
CIPHER and such in my hookscript, right? I basically use only what's in
the 3rd field of crypttab, and that should correspond to CRYPTTAB_KEY
here?!
When I don't actually need crypttab_parse_options, I also shouldn't
need to source /lib/cryptsetup/functions above.



So apart from not understanding yet what the "${CRYPTTAB_NAME#\#}" =
"$CRYPTTAB_NAME" is meant for, I'd say it's enough for my initramfs
hook to do:
CRYPTTAB_OPTIONS; do

assuming each line of "$DESTDIR/cryptroot/crypttab" will continue to
adhere to this format.

Hmm... I've looked over it, and it seems the only reason was to be able
to depend on busybox (only if cryptsetup within initramfs is used)?
Cause the other mess Michael Biebl described seems to be rather either
other bugs or somehow solvable at a first glance.

Well in the end I personally still consider such "configuring" packages
 rather ugly.
One could have made a warning if initramfs-tools.conf says BUSYBOX=N
but cryptsetup needs to enable them nevertheless.
Or vice versa let cryptsetup not forcibly enable it, and give a
warning/error during initramfs creation if it's not there.
The package management system shouldn't be therefore configuration -
config files should.

Anyway,... just cosmetic thoughts from my personal PoV... I'm happy as
it is and with the great efforts you guys do for the benefit of all of
us!


Cheers & thanks,
Chris.

Just open a wishlist bugs and everybody will be able to look at it? :-)
meant to document the interface, but to understand what your needs are.

No, it's for the case where cryptsetup's initramfs integration is not
processed (because the package isn't installed, because /etc/crypttab is
empty or nonexistent, or because /etc/cryptsetup-initramfs/conf-hook
specifies CRYPTSETUP=n).

Hmm good point, you can have spaces and tabs (and options values
containing ‘,’) by prefixing them with ‘/’ in your /etc/crypttab, but
lines are unquoted when the hook reads /etc/crypttab, and
$DESTDIR/cryptroot/crypttab is quote-free so special characters are
lost.  (Note that the handling of special characters in /etc/crypttab
was not documented — thus not supported — before 2:2.0.3-2.)

It removes comments (not necessary currently as the hook removes them
already).

It very much is, see http://pubs.opengroup.org/onlinepubs/9699919799/
sec. 2.6.2 “Parameter Expansion”.  Anyway we're not targeting POSIX
shell but dash (which has a handful of features not in POSIX shell) for
the hook files, and busybox's ash (which is a superset of dash) in the
scripts.

Hi,

In the upcoming 2:2.0.3-5 I refactored the crypttab(5) parsing logic [0].
Would the following interface suit your needs?

    crypttab_find_entry([--quiet], $target)

        Search the crypttab(5) for the given $target and set
        CRYPTTAB_NAME, CRYPTTAB_SOURCE, CRYPTTAB_KEY, and
        CRYPTTAB_OPTIONS accordingly.  (These variables are not exported
        to the environment.)  If there are duplicates target names then
        only the first one is considered.  Return 0 if a match is found,
        and 1 otherwise.

    crypttab_foreach_entry($callback)

        Iterate through the crypttab(5) and run the given $callback for
        each entry found.  The entry currently being processed is
        refered to by the values of CRYPTTAB_{NAME,SOURCE,KEY,OPTIONS}.
        (These variables are not exported to the environment.)
        Note: $callback's return value doesn't affect the loop
        currently, but if breaking out is desired it shouldn't be hard
        to add.

    crypttab_parse_options([--export], [--quiet])

        Parse the options of a crypttab(5) mapping, defined by values of
        variables CRYPTTAB_{NAME,SOURCE,KEY,OPTIONS}, and set variables
        variables CRYPTTAB_OPTION_<option>=<value> accordingly.  These
        variables are exported to the environment if --export is set.
        Return 1 on parsing error, 0 otherwise (incl. if unknown options
        were encountered).

The crypttab(5) to use is suitably chosen depending on the context: main
system, initramfs hook scripts, or initramfs boot scripts.

See the `cryptgnupg` hook script for an example of this interface:

https://salsa.debian.org/cryptsetup-team/cryptsetup/blob/master/debian/initramfs/hooks/cryptgnupg

I should also point out that the value of CRYPTTAB_OPTIONS is not
reliable if there are options with values containing ‘,’ characters.
So to get the value of a particular <option> one shouldn't parse
$CRYPTTAB_OPTIONS, but rather use $CRYPTTAB_OPTION_<option>.  For
instance if the crypttab(5) line being processed is

    target /dev/source none luks,header=/my/header\054swap

(minus the 4 leading spaces) then after parsing options one gets

    CRYPTTAB_NAME="target"
    CRYPTTAB_SOURCE="/dev/source"
    CRYPTTAB_KEY="none"
    CRYPTTAB_OPTIONS="luks,header=/my/header,swap"
    CRYPTTAB_OPTION_luks="yes"
    CRYPTTAB_OPTION_header="/my/header,swap"

Hey Guilhem.


Looks awesome at a first glance,.. I'll have a more thorough look at it
the next days...

One thing however comes just to my mind, i.e. another use case (that my
hook script would already do):

In my crypttab 3rd field, I allow some option to be included like
"include_key_in_initramfs".
That's basically a mutually exclusive alternative to the
device=...:path=... option where the keyscript loads+gpg-decrypts the
key from that device in that relative path.

As the name implies, it includes the gpg-encrypted key in the initramfs
images at their creation time (taking *just* path=... as its location
in the filesystem).
(For security reasons this is what I'd not suggest, though.)


Now when my initramfs hook copies that to the initramfs, it needs to
store it in some place where it can be later found by the keyscript.
Currently, the path is hardcoded in my hook script (only).
Until the big changes in the recent upgrades to cryptsetup, the hook
then modified the:
.../conf/conf.d/cryptroot
file to contain path=<pathWithinTheInitramfs> in the 3rd field (instead
of the original path=<pathWithinTheNormalFilesystem>.


So the use case is, that people may also wish to modify parts of the
.../cryptroot/crypttab file from their hook scripts... and that it
could be nice to have an interface for that as well.

What do you think? :-)

Cheers,
Chris.

That adds too much complexity for the benefit of a too specific use
case, IMHO.  Since you already “abuse” CRYPTTAB_KEY and parse/split its
value in your hook and keyscript, you could as well but both the source
and destination path there:

    device=…:pathsrc=…:pathdst=…

Or have a static destination path that uniquely depends on $CRYPTTAB_NAME,
for instance [$DESTINATION]/cryptroot/keyfiles/$CRYPTTAB.key.

Hey Guilhem.

I've just wondered whether the way you've mentioned above is still
valid respectively considered "stable" now (as it: for use by
keyscripts)? :-)

And further, did I understand it right, that
$DESTDIR/cryptroot/crypttab would contain one line (in the crypttab
syntax) per device that needs unlocking in the initramfs?

Which could then of course mean, that each line uses a different
keyscript.


Thanks,
Chris.

Oh and perhaps a bit more O:-)

The basic idea, AFAIU, would be the following, for my case:



1) The keyscript is repeatedly invoked for each device and has
CRYPTTAB_* already set (each time with the respective values).
So in that case I don't have to loop over all devices myself, nor to I
have to use anything from /lib/cryptsetup/functions .

The only exception would be, if - in my specific use case - they device
with the gpg-encrypted key file is one of those unlocked during
initramfs, in which I could end up in a deadlock.




2) For the hook the idea is, AFAIU the following:
I (!) have to loop over all the devices listed in
"$DESTDIR/cryptroot/crypttab", which had previously been populated by
the cryptoroot hook (upon which I depend).

I have no options set, but I can use crypttab_parse_options to get
them.

Then for each entry I first need to check, whether it's me (i.e.
whether the $CRYPTTAB_OPTION_keyscript is mine).
Then I can go on and parse CRYPTTAB_KEY, which in turn contains the
various options for my script.

And obviously I should e.g. do the copy_exec only once.

Does that seem about right?


Is TABFILE to be used by 3rd party hooks/keyscripts?


Or is there a better way? I've seen crypttab_foreach_entry()...

Could I use that like this:

myhook()
{
#- parses CRYPTTAB_KEY
#- set variables whether it needs to copy stuff in
}

crypttab_foreach_entry(myhook)

if [ $foo = "yes" ]; then
	copy_exec whatever
fi


AFAIU, that function would also automatically detect whether it's in a
hook context, and set the right TABFILE?



Thanks,
Chris.

Hi,

Well we've never received any follow-up regarding a stable interface
and/or documented functions, so this bug is still open and the situation
is not more stable than 3 years ago :-P  The interface suggested in
https://bugs.debian.org/901795#53 likely still works, but is still
subject to change while this bug is open.

Yes, however the exact TABFILE value may not be relied upon, and the
file content is mangled to make it suitable for initramfs stage, so it
might not match /etc/crypttab.

copy_exec() is a no-op when the destination exists.

Yes, again see https://bugs.debian.org/901795#53 , that's what
/usr/share/initramfs-tools/hooks/cryptgnupg and some of our other hooks
do.

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.

Oh and one more thing which is a bit unrelated to this, but also a bit
related ;-)

Could you clarify:
       tries=<num>
           Try to unlock the device <num> before failing. It's particularly
           useful when using a passphrase or a keyscript that asks for
           interactive input. If you want to disable retries, pass “tries=1”.
           Default is “3”. Setting “tries=0” means infinitive retries.


which AFAIU is how often cryptsetup invokes the keyscript and *not* how
often the keyscript itself tries (e.g. asking for a passphrase).

And that's also what should be clarified / "defined", like by saying:
           Try to unlock the device <num> before failing. Its how often
           the keyscript is invoked when it fails.
           If you want to disable retries, pass “tries=1”.
           Default is “3”. Setting “tries=0” means infinitive retries.
           Note that keyscripts themselves may do their own tries in
           addition.


What I describe above makes IMO actually sense, i.e. having two
different kind of tries:
Take my keyscript as an example, which waits for a device, reads a gpg-
enced key from it with passdev, then waits for a passphrase with
askpass and uses that to decrypt the data with gpg.

Currently, when I enter a wrong key (e.g. at boot time) I have to plug
the USB again (retry made by cryptsetup's 4th field tries=0), because
the keyscript exited and the already read stuff is gone.

With an additiones tries=, specific to the keyscript (and set again in
the 3rd field that I abuse so belovedly) I could do the following:

The "internal" tries is e.g. 3, so my own keyscript will already try
reading the passphrase and decrypting the previously read gpg-enced
file 3 times before giving up.

I could surround the asskpass with a timeout, just to make sure that
they keyscipt (with the precious key in memory, allowing for coldboot
attacks) doesn't stay there forever (e.g. if I forgot about the
computer and went shopping).
If the timeout would be e.g. 15s per default, and the internal tries 3,
they keyscript would wait at most 45s... and then exit.

Then the cryptsetup tries=n comes again (for the initramfs it probably
only makes sense with =0), but now, it would need the USB stick again.


Thanks,
Chris.

Right.

If desired, yes.

Right, that's what we're doing too.

You need to call crypttab_parse_options() in the callback, see the
cryptgnupg keyscript for an example.  IIRC this is intentional because
te callback need to have the ability to bail out before option
validation.

That's what's documented in crypttab(5).

Yes.  FWIW the original unescaped values can be found in _CRYPTTAB_*,
but this is undocumented and thus may not be relied upon.

Correct.

My bad, it's supported in all fields.

That's too much a niche case IMHO.  When you use a key script the 3rd
field is an opaque value passed along and you might treat it any way you
see fit.

That would have have been a valid suggestion at the time the interface
was designed, but many releases later I'm afraid renaming is not an
option.

Hey Guilhem.
to getting the whole thing (eventually) stabilised, I'd probably
recommend it.

I'm just not sure what should be returned then:
- always either 0 (all succeeded) or 1 (a failure happened)
or
- always either 0 (all succeeded) or <the non-zero exit status of the
  failing callback>

What would you suggest?

Yes, than all is correctly documented... I just wasn't sure whether
this might be either a documentation issue or undesired behaviour cause
the two (keyscript / hook) differ, or the one it's already there, for
the others not.

But it's alright then.

Are you going to correct it or shall I provide a patch for it?

I would really really ... really ;-) strongly hope you'd reconsider
this:

- With the keyscript we have such a nice and powerful way that let
  people nearly arbitrarily extend the functionality.
  But since stuff is pre-processed by cryptsetups own scripts (e.g.
  escaping and all that stuff) it could happen quite easily, that
  keyscripts and hooks break, if they'd all do their own stuff.
  Just take my example:
  - I already use ":" as separator, making the configuration style
    inhomogeneous with the rest.
  - Right now I have no escaping support at all, so people using weird
    values would just break my script.
  - And even if I repeat the unescaping step by using _CRYPTTAB_* I
    again use "inofficial" API, which could break, should you ever
    decide to change things.

- What are the possible (reasonable) cases of keyscripts, that just
  need a plain pathname as 3rd-field parameter?
  If such keyscript knows nothings else (which it cannot, because
  there's no way to configure it), then the keyfile itself needs
  already be readily available in the filesystem, or at least it must
  be possible to try through all possible candidates (like e.g. all
  slots of a crypto smartcard).

  That rules out any keyscript where the key is taken from somewhere
  that doesn't look like a file:
  - from network (where host/port/remote-server SSH key or so would be
    needed)
  - from crypto smartcards/etc., when LUKS is *not* used. Here it might
    not be possible to determine, whether one had tried the right key.
    Sure, there is "check=" but consider a double encrypted plain dm-
    crypt volume ... there would be no check there.
  - or like my case, where it's read from a non available device, where
    some device ID is needed)
  and all that would need to be configured somewhere, and should
  ideally use the same rules for quoting, separators, etc..

- If cryptsetup would provide a function like crypttab_parse_options
  for the 3rd filed, and maybe in addition makes the CRYPTTAB_*
  variables stable, I'd also call #901795 done, and everything that
  a keyscript maker could possible need, provided via some proper API.

- Also I think the change I'm asking for is not so invasive, or is it?
  Would e.g. the following do it already (two questions inside the
  code)?


# crypttab_parse_key_as_options([--export], [--quiet])
#   Parses $_CRYPTTAB_KEY, as a comma-separated option string from the
#   crypttab(5) 3th column, and sets corresponding variables
#   CRYPTTAB_KEYOPTION_<option>=<value> (which are added to the environment
#   if --export is set).
#   For error and warning messages, CRYPTTAB_NAME, (resp. CRYPTTAB_KEY)
#   should be set to the (unmangled) mapped device name (resp. key
#   option string).
#   Return 1 on parsing error, 0 otherwise (incl. if unknown options
#   were encountered).
crypttab_parse_key_as_options() {
    local quiet="n" export="n"
    while [ $# -gt 0 ]; do
        case "$1" in
            --quiet) quiet="y";;
            --export) export="y";;
            *) cryptsetup_message "WARNING: crypttab_parse_key_as_options(): unknown option $1"
        esac
        shift
    done

    local IFS=',' x OPTION VALUE

    # unset any CRYPTTAB_KEYOPTION_* variables
    # This may also determine and unset some CRYPTTAB_KEYFILEOPTION_* names which
    # were not even set (namely in cases, where such strings were contained in
    # variable values with newlines at the right place), but it doesn't harm, since
    # we anyway claim the whole CRYPTTAB_KEYOPTION_* "namespace" as "ours".
    # Note that [:alnum:] isn't used as it depends on the locale.
    unset -v $( set | sed -n 's/^\(CRYPTTAB_KEYFILEOPTION_[a-zA-Z_0-9]\+\)=.*$/\1/p' |  tr '\n' ' ' )

    # use $_CRYPTTAB_KEY not $CRYPTTAB_KEY as options values may
    # contain '\054' which is decoded to ',' in the latter
    for x in $_CRYPTTAB_KEY; do
        OPTION="${x%%=*}"
        VALUE="${x#*=}"
        if [ "$x" = "$OPTION" ]; then
            unset -v VALUE
        else
            VALUE="$(printf '%b' "$VALUE")"
###=> is this the place where you unescape?
###   then the documentation is wrong, casue %b doesn't only unescape octal sequences, right?
        fi
        if ! crypttab_validate_option; then
###=> not exactly sure what we should do here:
###
###   do you test anywhere whether OPTION is a vaild trailing part of variable names?
###   cause that's what I'd do instead of crypttab_validate_option, which wouldn't make sense,
###   since we cannot really check the values of keyscript options
###   maybe I could either just error out if something not [a-zA-Z_0-9]+ is encountered, or
###   replace any of those with "_"?
###
            return 1
        elif [ -z "${OPTION+x}" ]; then
            continue
        fi
        if [ "$export" = "y" ]; then
            export "CRYPTTAB_OPTION_$OPTION"="${VALUE-yes}"
        else
            eval "CRYPTTAB_OPTION_$OPTION"='${VALUE-yes}'
        fi
    done
    IFS=" "
}

Well it's just a name, so I don't care so much about it.
But if you'd choose to accept my above proposal, it would at least make
sense to add to the manpage, that keyscripts might use the 3rd field
not as a keyfile pathname, but also as comma-separated and printf %b
unescaped options.


Thanks,
Chris.

I'll fix it, thanks for the notice!

I still stand by what I wrote here 3 years ago, it's a useniche case and
we have no reason to assume that the opaque 3rd field value is
$FOO-delimited.  For all I know there might be keyscripts which expect a
JSON string here instead…  I wouldn't mind documenting _CRYPTTAB_KEY for
those who need the raw value from crypttab(5), but even without the
ambiguity can easily be eliminated by double-escaping or simply using
other escape sequences: “foo\040bar:ba%3Az” in crypttab yields
CRYPTTAB_KEY="foo bar:baz%3Abar" which you can trivially decode into a
pair ["foo bar", "ba:z"].  Moreover, doing this makes it possible to
manipulate binary strings which is not something we can do with
pre-mangled environment variables.

Not wrong in my view, but incomplete and using undocumented escape
sequences yields unspecified behavior.  The intent was to mimic fstab(5)
behavior, and IIRC I noticed at the time that \xHH was supported
although not documented either.  Either way, better stick to documented
escape sequences in both files.

Question is though: What makes sense to provide to programmers of
keyscripts?

Will such developer insist on using format XYZ if he's already provided
with out-of-the-box tools for some other format ABC and that's enough
for his needs?

I would have thought that the whole system benefits if one tries to
keep things as homogeneous as possible with an API that handles things
in a stable way and users don't have to adapt their keyscripts
everytime something changes in the back.

I doubt it's so beneficial to treat it as fully opaque and let
everything like escaping be done differently per keyscript.

Well sure I can one can do it the keyscript. I could simply take the
example crypttab_parse_key_as_options() given before and put that into
the keyscript.
My point was rather that this ain't a wheel every keyscript developer
should need to re-invent.

But anyway... I guess that leads to nothing.


I guess documenting _CRYPTTAB_* as stable API would be helpful, from
there all keyscript developers could re-do the some similar kind of
parsing.
But it probably only makes sense if the crypttab_*() functions would
also get ever official, which currently doesn't seem on the near
horizon.



Well, from my side we could probably close the bug or at least I can
unsubscribe it, since there doesn't seem to be any clear path forward
to really stabilise that API and there's no desire to extend it either.

I will just "unofficially" use what's already and adapt should it ever
break :D


Thanks for you help,
Chris.

Well the problem is simply that anyone who uses in any of the fields
e.g. \n will end up getting a true newline and not the literal \n, as
one would assume from the documentation, which just mentions the octal
escapes.


Btw, there might also be a subtle security issue:

If, for some reason, normal users are allowed to directly or indirectly
control the contents of crypttab, they could probably inject shell code
here:
            eval "CRYPTTAB_OPTION_$OPTION"='${VALUE-yes}'

Cheers,
Chris.

btw: it might make sense for you to instead create and document a copy
of the _CRYPTTAB_* e.g. RAW_CRYPTTAB_* .

That would give you the freedom to change/mangle/etc. the underlying
_CRYPTTAB_* should you ever wish to.


Cheers,
Chris.

I was about to reply “like fsttab(5)” but it seems fstab-decode(8)
doesn't mangle ‘\xHH’, ‘\t’ or ‘\n’.  So either I misremembered testing
this at the time, or something changed meanwhile :-)  I'd argue that ‘\’
is a special character which per documentation “needs to be escaped
using octal sequences”, so both ‘\n’ and ‘\xHH’ yield unspecified
behavior, but I guess that can be made explicit.

We assume that unprivileged users do not have write access to
/etc/crypttab (actually, $TABFILE), keyscripts, or initramfs hook/
scripts.  Otherwise one can replace askpass with `mail me@example.net`,
append ‘keyscript=gimme_your_password’ to crypttab entries, or simply
ship compromised executables in the initramfs image.

AFAICS, at least \xHH is not specified for that in at least POSIX:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html

dash, doesn't seem to do it either, but bash does.
Every field of crypttab is unescaped using printf(1)’s “%b” conversion
specification, which unescapes the \-escapes (\n, \t, \0num, etc.) as
provided by the echo utility.

Than we're always automatically on the safe side.

I'd still suggest to document that (and not just assume it silently)...
otherwise some smartypants admins might thinkt it's ok to allow users
to just append entries to crypttab in some way they think it would be
secure.



Cheers,
Chris.

The use of `printf %b` to decode escape sequences is an internal
implementation detail; documenting it would tie our hands for
implementation changes…

Also there is no guaranty that /bin/sh is dash; don't forget that we
also run at initramfs stage where /bin/sh is typically `busybox ash`
(but again no guaranty, it can be dash, bash, klibc's sh, or anything
else) for which `printf %b` *does* decode \xHH.  In my view
overspecifying is all but ideal here.

Do we warn users not to make /usr/bin, /root or /etc/ld.so.conf writable
by unprivileged users? :-)  Or not to remove the sticky bit on /tmp?

True.
Well then best is probably to e.g. document the \0xxx and mention that
any other use of \ needs to have that quoted or it may have a special
meaning?

That should neither over- nor under-specify it.


Cheers,
Chris.

Right, that's what I was hinting at in https://bugs.debian.org/901795#128 :-)

| I'd argue that ‘\’ is a special character which per documentation
| “needs to be escaped using octal sequences”, so both ‘\n’ and ‘\xHH’
| yield unspecified behavior, but I guess that can be made explicit.

Hey again.

Next to stabilising "_CRYPTTAB_*", could you also export it to the
keyscripts?

I can see it in the initramfs hook, when using crypttab_foreach_entry,
but it's not there in the keyscript.

Thus I cannot implement my own unescaping.


Cheers,
Chris.

Seems to be doable with a simply oneliner:

--- a/lib/cryptsetup/functions	2021-09-27 03:30:31.928052985 +0200
+++ b/lib/cryptsetup/functions	2021-09-27 03:30:28.387976358 +0200
@@ -278,6 +278,7 @@
     local keyscriptarg="$1" CRYPTTAB_TRIED="$2" keyscript;
     export CRYPTTAB_NAME CRYPTTAB_SOURCE CRYPTTAB_OPTIONS
     export CRYPTTAB_TRIED
+    export _CRYPTTAB_NAME _CRYPTTAB_SOURCE _CRYPTTAB_KEY _CRYPTTAB_OPTIONS

     if [ -n "${CRYPTTAB_OPTION_keyscript+x}" ] && \
             [ "$CRYPTTAB_OPTION_keyscript" != "/lib/cryptsetup/askpass" ]; then


But I'm not sure how you'd want to handle _CRYPTTAB_KEY. It seems to be
there at this point, but CRYPTTAB_KEY is set below in the conditional
from $keyscriptarg .

Cheers,
Chris.

Why not?  _CRYTTAB_* is useful to copy a crypttab snippet to another
location, but as said before you don't need it to produce your own
parsing logic.  You can use another character than ‘\’ to start your
escape sequence, or double escape the ‘\’s.  And again you'll need
something like that to pass NUL bytes anyway.

I don't mind exporting these but it's incorrect to claim that not having
the verbatim strings are preventing massaging.

function for the 3rd field...

crypttab has some given format, which follows that of fstab.
This format is (as you know of course):
- one entry per row
- fields separated by whitespace
- options within a field separated by ","
- options either standalone or with =value
- values quoted with \0ooo

Of course one could somehow hack in anything else too, JSON, XML,
base64 encoded binary ASN1, etc., just as one could double-escape (or
triple?) or us another separator char or ≔ instead of =

But why on earth should one want to do any of that?


It would just make editing of the config files more complex and deviate
from the given format style without any good reason.

Well at least not a "clean" one that simply follows the standard format
without any hacks and workarounds like double-escaping. :-D


In case you haven't seen, I've made a PR which seems to do that
exporting :-)


Thanks,
Chris.

Because the field is opaque, and the key=value list format might not
make sense for keyscripts.

Well sure you can define it that way... but with respect to the fstab-
like-format that makes simply not that much sense:

fstab quite clearly assumes a format as described above. It also
doesn't

There’s no single filesystem type which would expect any options in
fstab’s fourth field which wouldn't follow the actual main format but
take e.g. suvol={JSON} or so.


Why should crypttab go down this road, when it's anyway not really
possible, as neither filed can ever be truly opaque?!

Without encoding respectively quoting an double-quoting, you cannot
have binary data in it nor you can you have JSON/XML in it.


Actually, if it would be opaque for keyscripts, as you say, then it
wouldn't perform any unencoding on it and:
CRYPTTAB_KEY == _CRYPTTAB_KEY



Anyway... I guess that discussion is moot, my whole point was whether
we can get the raw variable exported?


Cheers,
Chris.

I agree that fstab's *4th column* (option) does, and crypttab's *4th
column* (option) follow the same format.  AFAIK fstab itself makes no
assumption on how the 1st field is formatted; like mount(8)'s ‘device’
argument its interpretation depends on the FS type.  Looks pretty opaque
to me.

No because the value may contain space and tabs which are used as field
separator hence need to be escaped.  For that field I see no need to use
any other octal sequences other than these two.

Yeah, and frankly also rather tiring.

As said in msg#163, yes.