Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1070429 > unrolled thread

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

Back to article view | Back to linux.debian.bugs.dist

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-09 01:00 +0200
    Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-09 01:30 +0200
    Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-09 02:00 +0200
    Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 02:00 +0200
      Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 02:10 +0200
      Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 03:20 +0200
        Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 17:20 +0200
          Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 18:00 +0200
            Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 18:40 +0200
            Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 20:00 +0200
              Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-27 03:10 +0200
                Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-27 03:40 +0200
                Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-27 17:30 +0200
                  Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-27 18:30 +0200
                    Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-27 18:50 +0200
                      Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-27 19:30 +0200
                        Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-27 21:20 +0200
          Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 18:10 +0200
            Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 18:40 +0200
              Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 20:10 +0200
                Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 20:40 +0200
                  Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 22:00 +0200
                    Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Christoph Anton Mitterer <calestyo@scientia.net> - 2021-09-11 22:10 +0200
                      Bug#901795: cryptsetup-initramfs: please provide documented shell functions to validate/sanitize cryptroot entries in 3rd party hook files Guilhem Moulin <guilhem@debian.org> - 2021-09-11 22:20 +0200

Page 1 of 2  [1] 2  Next page →

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

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.

[toc] | [next] | [standalone]

#1070432

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.

[toc] | [prev] | [next] | [standalone]

#1070436

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Thu, 09 Sep 2021 at 00:54:51 +0200, Christoph Anton Mitterer wrote:
> I've just wondered whether the way you've mentioned above is still
> valid respectively considered "stable" now (as it: for use by
> keyscripts)? :-)

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.

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

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.

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

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

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

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1070712

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.

[toc] | [prev] | [next] | [standalone]

#1070714

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.

[toc] | [prev] | [next] | [standalone]

#1070718

[Multipart message — attachments visible in raw view] — view raw

On Sat, 11 Sep 2021 at 01:31:31 +0200, Christoph Anton Mitterer wrote:
> 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?

Right.

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

If desired, yes.

> 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

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

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

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.
 
> But in keyscripts I would already have CRYPTTAB_OPTION_*, too, right?

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

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

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

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

Correct.

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

My bad, it's supported in all fields.

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

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.

> 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.

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1070771

Hey Guilhem.


On Sat, 2021-09-11 at 03:13 +0200, Guilhem Moulin wrote:
> > 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?
> 
> If desired, yes.

Again, it's probably not so important for my own case, but with respect
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?


> > 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?
> 
> 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.
>  
> > But in keyscripts I would already have CRYPTTAB_OPTION_*, too,
> > right?
> 
> That's what's documented in crypttab(5).

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.



> > For which fields are the octal escapes handled? The manpage only
> > mentions them for them for the key/3rd field.
> 
> My bad, it's supported in all fields.

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



> > 4) Wishlist ;-)
> > Can we have something like the option splitting for the key/3rd
> > field,
> > too?
> 
> 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.

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=" "
}

    



> 
> > 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.
> 
> 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.

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.

[toc] | [prev] | [next] | [standalone]

#1070772

[Multipart message — attachments visible in raw view] — view raw

On Sat, 11 Sep 2021 at 17:12:17 +0200, Christoph Anton Mitterer wrote:
>>> For which fields are the octal escapes handled? The manpage only
>>> mentions them for them for the key/3rd field.
>> 
>> My bad, it's supported in all fields.
> 
> Are you going to correct it or shall I provide a patch for it?

I'll fix it, thanks for the notice!

>>> 4) Wishlist ;-)
>>> Can we have something like the option splitting for the key/3rd
>>> field,
>>> too?
>> 
>> 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
>> see fit.
> 
> I would really really ... really ;-) strongly hope you'd reconsider
> this:

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1070778

On Sat, 2021-09-11 at 17:55 +0200, Guilhem Moulin wrote:
> 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…

Well sure... or it could be base64 encoded, XML or whatever.

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.


> 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"].

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.

[toc] | [prev] | [next] | [standalone]

#1070794

On Sat, 2021-09-11 at 17:55 +0200, Guilhem Moulin wrote:
> I wouldn't mind documenting _CRYPTTAB_KEY for
> those who need the raw value from crypttab(5)

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.

[toc] | [prev] | [next] | [standalone]

#1073108

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.

[toc] | [prev] | [next] | [standalone]

#1073110

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.

[toc] | [prev] | [next] | [standalone]

#1073236

[Multipart message — attachments visible in raw view] — view raw

On Mon, 27 Sep 2021 at 02:56:26 +0200, Christoph Anton Mitterer wrote:
> Thus I cannot implement my own unescaping.

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1073245

On Mon, 2021-09-27 at 17:24 +0200, Guilhem Moulin wrote:
> 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.

Well it's kinda the same point like when I've asked for a parsing
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.



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

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.

[toc] | [prev] | [next] | [standalone]

#1073247

[Multipart message — attachments visible in raw view] — view raw

On Mon, 27 Sep 2021 at 18:21:47 +0200, Christoph Anton Mitterer wrote:
> But why on earth should one want to do any of that?

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

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1073253

On Mon, 2021-09-27 at 18:37 +0200, Guilhem Moulin wrote:
> 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.

[toc] | [prev] | [next] | [standalone]

#1073263

[Multipart message — attachments visible in raw view] — view raw

On Mon, 27 Sep 2021 at 19:21:45 +0200, Christoph Anton Mitterer wrote:
> On Mon, 2021-09-27 at 18:37 +0200, Guilhem Moulin wrote:
>> 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.

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.

> 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

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.

> Anyway... I guess that discussion is moot,

Yeah, and frankly also rather tiring.

> my whole point was whether we can get the raw variable exported?

As said in msg#163, yes.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1070775

[Multipart message — attachments visible in raw view] — view raw

On Sat, 11 Sep 2021 at 17:12:17 +0200, Christoph Anton Mitterer wrote:
>           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?

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

#1070780

On Sat, 2021-09-11 at 18:06 +0200, Guilhem Moulin wrote:
> Not wrong in my view, but incomplete and using undocumented escape
> sequences yields unspecified behavior.

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.

[toc] | [prev] | [next] | [standalone]

#1070800

[Multipart message — attachments visible in raw view] — view raw

On Sat, 11 Sep 2021 at 18:31:33 +0200, Christoph Anton Mitterer wrote:
> On Sat, 2021-09-11 at 18:06 +0200, Guilhem Moulin wrote:
>> Not wrong in my view, but incomplete and using undocumented escape
>> sequences yields unspecified behavior.
> 
> 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.

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.
 
> 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}'

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.

-- 
Guilhem.

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.debian.bugs.dist

csiph-web