Re: Cryptsetup completion

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Paul Ruane wrote on Wed, 03 Jun 2020 13:52 +0100:
> On Wed, 3 Jun 2020 at 01:56, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> 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