Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argon2 command line argument order sensitive (it should NOT) #279

Open
ye opened this issue Oct 16, 2019 · 3 comments
Open

argon2 command line argument order sensitive (it should NOT) #279

ye opened this issue Oct 16, 2019 · 3 comments

Comments

@ye
Copy link

ye commented Oct 16, 2019

As a robust command line program, the arguments should not be positional, e.g. tar. In argon2, the arguments are mostly non-positional except salt.

See example below.

Screen Shot 2019-10-16 at 12 54 19 PM

  1. it didn't work because I placed the type argument -id before the salt. Also this error message is useless because it doesn't give hint of which parameter went wrong. A good example is git, which will give you the context what the CLI program thinks you are trying to do and suggest the right way to do it.
  2. it worked because i followed the EXACT ordering
  3. it worked because all the arguments are not ordered (e.g -t placed last)
  4. argon2 -h or just argon2 gives the help text. But no way to show argon2 version.

To me, the usage line example is problematic:

Usage:  argon2 [-h] salt [-i|-d|-id] [-t iterations] [-m log2(memory in KiB) | -k memory in KiB] [-p parallelism] [-l hash length] [-e|-r] [-v (10|13)]
  • The help text usage (argon2 -h or just argon2 doesn't require salt), but all other cases salt argument is required. However, having -h argument or any arguments prefixed with - placed in front of salt made the impression that argon2 doesn't require arguments to be passed in order but in fact that's not true, salt must be the 1st positional argument.
  • Recommend have a separate line to indicate how to print help text, rather than consolidate all usages in one line in this current awkward way. Or make the salt argument optional as well [salt] and provide more detail error message when argon2 expects salt but not getting it.
  • add a print version command line argument --version. Right now the only way for me to check version is to do brew info argon2, always don't work if I compile from source directly.
@SparkDustJoe
Copy link

SparkDustJoe commented Oct 16, 2019 via email

@calestyo
Copy link

I think there’s nothing wrong with having positional arguments at a fixed position. This is especially necessary here to allow for salts that start with a - should there be ever options longer than 8 characters.

Other than that.. does #333 address your concerns with respect to the usage text?

@LoganDark
Copy link

For your point #4, the help text for the -v switch shows 10 or 13 (I'm assuming hex since in the other docs it mentions version 19), default is latest version (13). It's not obvious. My suggestion would be a banner above the "Usage:" text which explicitly shows the version to alleviate that concern.

The latest version is not 13, it is 0x13, or 19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants