Cryptsetup completion

Cryptsetup completion

[Paul Ruane]

Hi,

For my own system I have modified the Zsh completion of cryptsetup to
include completion of the hash and cipher based upon those declared
available in /proc/crypto. I thought this might be useful for other
users, too, so I have included my changes below for your
consideration.

11,12c11,12
<   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
<   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
---
>   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification:_ciphers' \
>   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_hashes' \
157a158,183
>
> _ciphers() {
> typeset -a cipher_list
> local line
>
> _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+cipher" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> while read -A line
> do
> cipher_list+=($line[1])
> done
>
> _describe -t ciphers 'cipher' cipher_list "$@"
> }
>
> _hashes() {
> typeset -a hash_list
> local line
>
> _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+shash" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> while read -A line
> do
> hash_list+=($line[1])
> done
>
> _describe -t hashes 'hash' hash_list "$@"
> }

Paul

Re: Cryptsetup completion

[Mikael Magnusson]

On 6/2/20, Paul Ruane <paul.ruane@oniony.com> wrote:
> Hi,
>
> For my own system I have modified the Zsh completion of cryptsetup to
> include completion of the hash and cipher based upon those declared
> available in /proc/crypto. I thought this might be useful for other
> users, too, so I have included my changes below for your
> consideration.
>
> 11,12c11,12
> <   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
> <   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
> ---
>>   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher
>> specification:_ciphers' \
>>   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_hashes' \
> 157a158,183
>>
>> _ciphers() {
>> typeset -a cipher_list
>> local line
>>
>> _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1
>> "^type\s\+:\s\+cipher" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed
>> "s/^ //"' | \
>> while read -A line
>> do
>> cipher_list+=($line[1])
>> done
>>
>> _describe -t ciphers 'cipher' cipher_list "$@"
>> }
>>
>> _hashes() {
>> typeset -a hash_list
>> local line
>>
>> _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1
>> "^type\s\+:\s\+shash" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed
>> "s/^ //"' | \
>> while read -A line
>> do
>> hash_list+=($line[1])
>> done
>>
>> _describe -t hashes 'hash' hash_list "$@"
>> }

One could also imagine these substitutions instead of forking twelve times:
cipher_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*
cipher*}#*: }%% *} )
hash_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*
shash*}#*: }%% *} )

-- 
Mikael Magnusson

Re: Cryptsetup completion

[Daniel Shahaf]

Paul Ruane wrote on Tue, 02 Jun 2020 17:17 +0100:
> Hi,
> 
> For my own system I have modified the Zsh completion of cryptsetup to
> include completion of the hash and cipher based upon those declared
> available in /proc/crypto. I thought this might be useful for other
> users, too, so I have included my changes below for your
> consideration.
> 

Thanks.

The helper functions are in the global namespace so they should have
names that are less likely to clash.  Also, they aren't specific to
cryptsetup, so we could break them out to separate, autoloadable files
(with "#autoload" on their first line rather than "#compdef foo").

Would you be interested in updating the patch along these lines?  It'd
be best to do so by using zsh from git, but you needn't build zsh for
that (you can simply clone the git repository and, for testing, set
«fpath=( /path/to/zsh/Completions/**/*(/) )» prior to running compinit.).

Cheers,

Daniel

P.S.  For future reference, always pass the -u option to diff(1),
otherwise the patches cannot be applied except to the very same version
of the original file.


> 11,12c11,12
> <   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
> <   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
> ---
> >   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification:_ciphers' \
> >   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_hashes' \  
> 157a158,183
> >
> > _ciphers() {
> > typeset -a cipher_list
> > local line
> >
> > _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+cipher" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> > while read -A line
> > do
> > cipher_list+=($line[1])
> > done
> >
> > _describe -t ciphers 'cipher' cipher_list "$@"  
> > }  
> >
> > _hashes() {
> > typeset -a hash_list
> > local line
> >
> > _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+shash" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> > while read -A line
> > do
> > hash_list+=($line[1])
> > done
> >
> > _describe -t hashes 'hash' hash_list "$@"  
> > }  
> 
> Paul

Re: Cryptsetup completion

[Paul Ruane]

Thanks for the detailed reply.

I'll have a crack at making these changes and integrating Mikael's
enlightening parameter expansion approach rather than my primitive
piping.

Paul

On Wed, 3 Jun 2020 at 01:56, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Paul Ruane wrote on Tue, 02 Jun 2020 17:17 +0100:
> > Hi,
> >
> > For my own system I have modified the Zsh completion of cryptsetup to
> > include completion of the hash and cipher based upon those declared
> > available in /proc/crypto. I thought this might be useful for other
> > users, too, so I have included my changes below for your
> > consideration.
> >
>
> Thanks.
>
> The helper functions are in the global namespace so they should have
> names that are less likely to clash.  Also, they aren't specific to
> cryptsetup, so we could break them out to separate, autoloadable files
> (with "#autoload" on their first line rather than "#compdef foo").
>
> Would you be interested in updating the patch along these lines?  It'd
> be best to do so by using zsh from git, but you needn't build zsh for
> that (you can simply clone the git repository and, for testing, set
> «fpath=( /path/to/zsh/Completions/**/*(/) )» prior to running compinit.).
>
> Cheers,
>
> Daniel
>
> P.S.  For future reference, always pass the -u option to diff(1),
> otherwise the patches cannot be applied except to the very same version
> of the original file.
>
>
> > 11,12c11,12
> > <   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
> > <   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
> > ---
> > >   '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification:_ciphers' \
> > >   '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_hashes' \
> > 157a158,183
> > >
> > > _ciphers() {
> > > typeset -a cipher_list
> > > local line
> > >
> > > _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+cipher" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> > > while read -A line
> > > do
> > > cipher_list+=($line[1])
> > > done
> > >
> > > _describe -t ciphers 'cipher' cipher_list "$@"
> > > }
> > >
> > > _hashes() {
> > > typeset -a hash_list
> > > local line
> > >
> > > _call_program grep 'grep "^\(name\|type\)\s\+:" /proc/crypto | grep -B1 "^type\s\+:\s\+shash" | grep "^name\s\+:" | cut -d ":" -f 2 | uniq | sed "s/^ //"' | \
> > > while read -A line
> > > do
> > > hash_list+=($line[1])
> > > done
> > >
> > > _describe -t hashes 'hash' hash_list "$@"
> > > }
> >
> > Paul
>

Re: Cryptsetup completion

[Paul Ruane]

On Wed, 3 Jun 2020 at 01:56, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> The helper functions are in the global namespace so they should have
> names that are less likely to clash.  Also, they aren't specific to
> cryptsetup, so we could break them out to separate, autoloadable files

I have renamed these with prefix "_crypto". I noticed commands and
types are now split into their respective directories so I have placed
them accordingly.

> that (you can simply clone the git repository and, for testing, set
> «fpath=( /path/to/zsh/Completions/**/*(/) )» prior to running compinit.).

I was unable to get the fpath with the globbing to work. I ended up
using "fpath=( /home/paul/projects/zsh/Completion )". Even so I had
some errors ("_arguments:comparguments:393: too many arguments")
completing with many commands, not just cryptsetup, but I assume that
this is some incompatibility between my 5.8 and HEAD.

I've used a slightly modified version of Mikael's parameter expansion
to eliminate the need to fork processes.

The new patch--in correct format :)--is here:

diff --git a/Completion/Linux/Command/_cryptsetup
b/Completion/Linux/Command/_cryptsetup
index 45159d0be..3e3bd5ba0 100644
--- a/Completion/Linux/Command/_cryptsetup
+++ b/Completion/Linux/Command/_cryptsetup
@@ -8,8 +8,8 @@ _arguments -s \
   '(-v --verbose)'{-v,--verbose}'[enable verbose mode]' \
   '--debug[show debug messages]' \
   '--debug-json[show debug messages including JSON metadata]' \
-  '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
-  '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
+  '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher
specification:_crypto_ciphers' \
+  '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_crypto_hashes' \
   '(-y --verify-passphrase)'{-y,--verify-passphrase}'[query for
password twice]' \
   '(-d --key-file)'{-d+,--key-file=}'[set keyfile]:key file:_files' \
   '--master-key-file=[set master key]:key file:_files' \
diff --git a/Completion/Linux/Type/_crypto_ciphers
b/Completion/Linux/Type/_crypto_ciphers
new file mode 100644
index 000000000..ab9138a11
--- /dev/null
+++ b/Completion/Linux/Type/_crypto_ciphers
@@ -0,0 +1,7 @@
+#autoload
+
+cipher_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*:
cipher*}#*: }%% *} )
+
+_describe -t ciphers 'cipher' cipher_list
+
+return ret
diff --git a/Completion/Linux/Type/_crypto_hashes
b/Completion/Linux/Type/_crypto_hashes
new file mode 100644
index 000000000..56b06b5f0
--- /dev/null
+++ b/Completion/Linux/Type/_crypto_hashes
@@ -0,0 +1,7 @@
+#autoload
+
+hash_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*:
shash*}#*: }%% *} )
+
+_describe -t hashes 'hash' hash_list
+
+return ret

Thanks,
Paul

Re: Cryptsetup completion

[Daniel Shahaf]

Paul Ruane wrote on Wed, 03 Jun 2020 13:52 +0100:
> On Wed, 3 Jun 2020 at 01:56, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> 
> > The helper functions are in the global namespace so they should have
> > names that are less likely to clash.  Also, they aren't specific to
> > cryptsetup, so we could break them out to separate, autoloadable files  
> 
> I have renamed these with prefix "_crypto". I noticed commands and
> types are now split into their respective directories so I have placed
> them accordingly.

They've always been split, but there's a configure option to combine
them at installation time.  (That's one of several reasons to base
patches on git master.)

> > that (you can simply clone the git repository and, for testing, set
> > «fpath=( /path/to/zsh/Completions/**/*(/) )» prior to running compinit.).  
> 
> I was unable to get the fpath with the globbing to work. I ended up
> using "fpath=( /home/paul/projects/zsh/Completion )".

Right, you do need Completion/ there, and what I wrote above won't do
that.  Sorry about that.  (I based that glob on the $ZDOTDIR/.zshenv
I use for development, but reduced it too much.)

> Even so I had some errors ("_arguments:comparguments:393: too many
> arguments") completing with many commands, not just cryptsetup, but I
> assume that this is some incompatibility between my 5.8 and HEAD.

Yes, that's correct: _arguments is coupled to the C implementation of
the "compargumetns" builtin.  Sorry again; I'd forgotten about that.
(To use 5.8 binaries with master's _arguments, the third argument in
calls to «comparguments -W» needs to be removed.)

> I've used a slightly modified version of Mikael's parameter expansion
> to eliminate the need to fork processes.
> 
> The new patch--in correct format :)--is here:
> 
> diff --git a/Completion/Linux/Command/_cryptsetup
> b/Completion/Linux/Command/_cryptsetup

Your MUA inserted hard line breaks in the patch, which corrupted it.
For future reference, either configure your MUA not to wrap line breaks
and not to strip trailing spaces, or send patches as an attachment named
*.txt [that's the most portable way to ensure a texty MIME type, which
helps recipients' MUAs].

> +++ b/Completion/Linux/Type/_crypto_ciphers
> @@ -0,0 +1,7 @@
> +#autoload
> +
> +cipher_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*:
> cipher*}#*: }%% *} )  

This variable is set globally.  It should be declared local («typeset -a»).

On my system the array has several elements with the value "aes" (as
well as one "__aes").  Is that intentional?  If not, adding -U to the
array declaration will squash duplicates.

> +_describe -t ciphers 'cipher' cipher_list
> +
> +return ret

There's no variable «ret» in this function.  However, the call to
_describe is in any case the last thing in the function, so it'll be
easier to delete this line entirely.

Paul Ruane wrote on Wed, 03 Jun 2020 13:52 +0100:
> +++ b/Completion/Linux/Command/_cryptsetup
> @@ -8,8 +8,8 @@ _arguments -s \
> -  '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification' \
> -  '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm' \
> +  '(-c --cipher)'{-c+,--cipher=}'[set cipher]:cipher specification:_crypto_ciphers' \
> +  '(-h --hash)'{-h+,--hash=}'[hash algorithm]:hash algorithm:_crypto_hashes' \
> +++ b/Completion/Linux/Type/_crypto_ciphers
> @@ -0,0 +1,7 @@
> +cipher_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*: cipher*}#*: }%% *} )  
> +++ b/Completion/Linux/Type/_crypto_hashes
> @@ -0,0 +1,7 @@
> +hash_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*: shash*}#*: }%% *} )  

Further question.  I smoke tested this by trying to format a loop device
with various hashes/ciphers from the given lists, but got some errors:

% print -r -- ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*: shash*}#*: }%% *}
crc32c crct10dif crc32 crc32c __ghash hmac(sha256) hmac(sha1) crct10dif sha224 sha256 sha1 md5 digest_null
% /sbin/cryptsetup luksFormat $PWD/loop key -c aes -h crc32c
⋮
Requested hash crc32c is not supported.
Failed to set pbkdf parameters.
zsh: exit 1     /sbin/cryptsetup luksFormat $PWD/loop key -c aes -h crc32c
% /sbin/cryptsetup luksFormat $PWD/loop key  -c aes -h "hmac(sha1)" 
⋮
Requested hash hmac(sha1) is not supported.
Failed to set pbkdf parameters.
zsh: exit 1     /sbin/cryptsetup luksFormat $PWD/loop key -c aes -h "hmac(sha1)"
% 

(That said, I haven't been able to get luksFormat to succeed with _any_
set of arguments, so there might be some flaw in my testing.)

There's also a note in cryptsetup(8):

> > > NOTES ON SUPPORTED CIPHERS, MODES, HASHES AND KEY SIZES
> > >        The available combinations of ciphers, modes, hashes and key sizes
> > >        depend on kernel support. See /proc/crypto for a list of available
> > >        options. You might need to load additional kernel crypto modules in
> > >        order to get more options.
> > > 
> > >        For the --hash option, if the crypto backend is libgcrypt, then all
> > >        algorithms supported by the gcrypt library are available.  For other
> > >        crypto backends, some algorithms may be missing.

So, two questions:

- Are there 'shash' entries in /proc/crypto that _shouldn't_ be offered
  as completions?

- Are there hash algorithms that would be accepted as arguments to the
  -h option that aren't listed in /proc/crypto?

Cheers,

Daniel

Re: Cryptsetup completion

[Oliver Kiddle]

Paul Ruane wrote:
> The new patch--in correct format :)--is here:

Thanks

> +++ b/Completion/Linux/Type/_crypto_ciphers

These are just the cyphers handled by the Linux kernel, right? Is this
naming perhaps too generic? _linux_ciphers perhaps? We wouldn't want
them to be used in other cases where something else such as openssl
ciphers are wanted. Does anyone know for what other commands these might
be relevant? If there are none, then I would contradict Daniel about
factoring them out.

> +cipher_list=( ${${${(M)${(s:name:)${(M)${(f)"$(</proc/crypto)"}:#(name|type)*}}:#*:
> cipher*}#*: }%% *} )
> +
> +_describe -t ciphers 'cipher' cipher_list

The _describe function is there to make it easier to format lists
containing both matches and descriptions. From my testing, this
cipher_list array is only a list of matches. So it is much more
efficient to just do:

    local expl
    _description ciphers expl cipher
    compadd "$@" "$expl[@]" -a cipher_list

I know that looks slightly longer but it is doing much less underneath
so is more efficient.

Oliver

Re: Cryptsetup completion

[Daniel Shahaf]

Oliver Kiddle wrote on Thu, 04 Jun 2020 13:50 +0200:
> Paul Ruane wrote:
> > The new patch--in correct format :)--is here:  
> 
> Thanks
> 
> > +++ b/Completion/Linux/Type/_crypto_ciphers  
> 
> These are just the cyphers handled by the Linux kernel, right? Is this
> naming perhaps too generic? _linux_ciphers perhaps?

+1

> We wouldn't want
> them to be used in other cases where something else such as openssl
> ciphers are wanted. Does anyone know for what other commands these might
> be relevant? If there are none, then I would contradict Daniel about
> factoring them out.

For the record, my rationale was simply that a function that completes
"'cipher' elements of /proc/crypto" seemed like it would be inherently
reusable.  Nevertheless, I'm not aware of a specific place where it
would be reused, and I wouldn't mind one way or the other whether it
should be factored out or not.

Separate issue: cryptsetup(8) says:
.
       --cipher, -c <cipher-spec>
	      … The current default in the distributed sources is
	      "aes-cbc-essiv:sha256" for plain dm-crypt and
	      "aes-xts-plain64" for LUKS. …
.
So it seems _linux_ciphers should not be used directly as _the_
completion function for arguments to the -c option.

Cheers,

Daniel