[Cfrg] IRSG review of draft-irtf-cfrg-argon2-09

[Stephen Farrell <[e-mail]>]

Hiya,

I did an IRSG review for this. I think it's basically
ready but with one issue to address before publication.

As an aside, I set some password-cracking student
assignments a year or so ago, and the kids all hated
argon2 as it was the only algorithm (from those I
set) that they couldn't crack, despite chewing up
loads of CPU:-) IOW, I really like the algorithm,
the issues below are only about the documentation
and interop.

Cheers,
S.

The issue: What is a "primary variant" and what is an
implementer supposed to do?  I don't think this has quite
been resolved sufficiently. The interop requirement here
is mostly (I think) that independent password hashing and
verification implementations be able to work for the same
data. I'm guessing it's too late to just ditch all but one
set of parameters, but I think It'd be better to say e.g.
that argon2id with salt-len foo, m=whatever, p=something etc.
(fill in a currently reasonable value for *all* of the
parameters) MUST be implemented, MUST be usable (i.e.
exposed via command line or parameters) and SHOULD be
the default. If we don't have that, then I figure we'll end
up with interop problems. (See also below about test vectors.)
If what's reasonable changes over time, then an update to
the RFC or some new algorithm could address that but I'd hope
it ought be possible to choose one set of parameters that
we expect to be usable (as a default) for say 5+ years.

I've also two non-blocking suggestions/questions that
ought not get in the way of publication, but the authors
might wanna consider 'em:

- Test vectors: it'd have been nice (for me) if I could
verify one of the test vectors using either the debian
command line tool or python-cffi, the two implementations
I have to hand, but I can't. The test vectors all include
parameters that aren't exposed by those implementations
(at least, I think so, having looked for a few minutes).
The values used in the test vectors are also binary
which'd make it a bit more work even if e.g. there was no
secret or associated data. Could you add another test
vector (or two) that can be verified with current tools
(e.g. the debian command line tool that I got with "apt
install argon2" on Ubuntu)?  I think it'd be great if the
RFC and those tools were compatible, not great but fixable
if they are not, but it's a bit of a pity that I can't
tell;-)

- Do we know if someone has implemented from the draft and
gotten the correct result? I looked back at the cfrg list
and didn't see that anyone said they had. It'd be great if
we knew the text was good enough for that. (It looks fine
to me, but I've not tried to implement from the draft.)

Nits:

- intro: The URL for [ARGON2ESP] gets me a 404.
That's: https://www.cryptolux.org/images/0/0d/Argon2ESP.pdf

- intro: I don't know what "Argon2 summarizes the state of
the art" is meant to mean. Maybe you mean that argon2 is
intended to reflect the current state of the art?

- 3.1: can the nonce/salt (S) have zero length?  It looks
like that would work but be undesirable, so why not set
a non-zero minimum? (Implementations seem to do that
already, not sure if with the same minimum.)

- 3.1: would it be better to say that the nonce needs to
appear random and not reveal anything about the password
as well as being unique-per-password? MD5(pwd) would be
(sufficiently:-) unique-per-password but would not be a
good nonce. I guess it's just about possible that an
implementer might do something that silly;-(

- 3.1: are the optional K and X values considered "absent"
when they have length zero? (I guess so as the lengths are
input to H_0) It might be better to be explicit about
that.

- 3.4: 'S' was already defined as the salt, so S = 4 for
the slices is a bit unfortunate, though not a real issue.