ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
63 stars 60 forks source link

Passphrase options #59

Closed xnox closed 5 years ago

xnox commented 5 years ago

This adds the common passphrase options to zkey cryptsetup subcommand, as well as adds the -batch-mode option.

Unfortunately, I was not able to reuse the args with the zkey-cryptsetup command, due to -l clash... it's both --volumes and --keyfile-size, hence made it use -L short arg.

Fixes https://github.com/ibm-s390-tools/s390-tools/issues/58

xnox commented 5 years ago

The command line options mimicked here, are those of cryptsetup / zkey-cryptsetup, as the options (both short and long) are the ones used by the cryptsetup tool.

Ideally, I would not want to introduce short options at all, and use long options only, but that's not really an option I don't think. If it is, my preference would be to force long args only.

I find it unfortunate that, zkey cryptsetup and zkey crypttab subcommands are part of zkey tool, rather than the zkey-cryptsetup tool. (I do understand that currently zkey-cryptsetup tool compilation is conditional, but it could compile cryptsetup/crypttab subcommands always).

It is annoying that '-l' option is already in use by the zkey cryptsetup hence I cannot really use -l again to specify the --keyfile-size. I guess I could enforce positional subflag parsing, but using '-L' seemed like the least work/surprise.

I also do not understand the reuse of short args concern raised, given that cryptsetup subcommand does not use the other flags - and unlikely to start doing so in the future. Enforcing short args uniqueness across the whole tool, is desirable, but counter-intuitive when cryptsetup itself will be called.

I understand that using same short option with different meanings, depending on which subcommand is doing parsing is awkward, but imho it is ok here, as imho cryptsetup well-known args should take precedence.

To be honest, I'd rather move the 'crypsetup|crypttab' subcommands into 'zkey-cryptsetup' and use the OPT_PASSPHRASE_ENTRY(cmd) from there, which does use cryptsetup compatible long and short options.

ifranzki commented 5 years ago

Ideally, I would not want to introduce short options at all, and use long options only, but that's not really an option I don't think. If it is, my preference would be to force long args only.

Yes I guess that would be me preference, too, and this is possible:

#define OPT_KEYFILE 256    // <-- use values in the range above 256 for different options....

    {
        .option = {"key-file", required_argument, NULL, OPT_KEYFILE},
        .argument = "FILE-NAME",
        .desc = "Read the passphrase from the specified file",
        .command = COMMAND_CRYPTSETUP,
        .flags = UTIL_OPT_FLAG_NOSHORT,
    },

In the switch you check for OPT_KEYFILE instead of the short option character.

I find it unfortunate that, zkey cryptsetup and zkey crypttab subcommands are part of zkey tool, rather than the zkey-cryptsetup tool.

To be honest, I'd rather move the 'crypsetup|crypttab' subcommands into 'zkey-cryptsetup' and use the OPT_PASSPHRASE_ENTRY(cmd) from there, which does use cryptsetup compatible long and short options.

One reason is that zkey operates on the key repository, which has all the uinformation about the to be generated commands and crypttab entriers. zkey-cryptsetup does not know the repository at all, and just acts on LUKS2 volumes. I would like to keep that separation.

ifranzki commented 5 years ago

In the meantime I took your patches and adjusted them to meet my requirements. I also found a few additional small things that needed to be changed.

Are you OK when I push the modified patches and add you as Suggested-by: Dimitri John Ledkov <xnox@ubuntu.com> to the commit message?

xnox commented 5 years ago

@ifranzki sure