password-hashes
password-hashes copied to clipboard
argon2: The documentation claims that associated data (and keyid) are deprecated but they are not
Currently, the documentation https://docs.rs/argon2/0.3.2/argon2/struct.Params.html#method.data claims that:
This field is not longer part of the argon2 standard (see: https://github.com/P-H-C/phc-winner-argon2/pull/173), and should not be used for any non-legacy work.
But I don't think that this is true. Notably, the referenced pull request only talks about the encoding, and the reference implementation still supports associated data: https://github.com/P-H-C/phc-winner-argon2/blob/master/include/argon2.h#L204-L205
Happily, #247 fixes this by making it possible to set the associated data.
That comment was added by @touilleMan in #249.
I'm a bit unclear what to make of the treatment in https://github.com/P-H-C/phc-winner-argon2/pull/173
It appears they removed some setters for data?
I'm a bit unclear what to make of the treatment in P-H-C/phc-winner-argon2#173
My take on that is that parameters with associated data (and keyid) have no string representation, but they can still be used programmatically.
Hi @teythoon ;-)
keyid&data can still be used programmatically, (see https://github.com/touilleMan/password-hashes/blob/a9471f0e898a000f36e60cde3a9aac4f6db6afad/argon2/tests/phc_strings.rs#L202-L203)
The reason to add the comment about deprecation in the documentation is due to those comments: https://github.com/P-H-C/phc-winner-argon2/issues/143#issuecomment-256152382
So keyid is an half-baked feature that got forgotten along the way (on top of that, the hashing algorithm never use this field so adding/modifying the keyid field to any existing hash has no effect)
data is a bit more tricky given it is actually used to compute the hash (so it cannot just be removed like keyid), see https://github.com/josephlr/phc-winner-argon2/blob/5d4f7552f0c2d8389c52ac9e42bce94f36ff4d91/src/core.c#L552-L553
That's why data is still present in the internal structure, but it is always set to empty.
Anyway those data and keyid fields really look like deprecated fields to me and a bad idea to rely on them (for instance you cannot use keyid/data from Python's PyNaCl: https://pynacl.readthedocs.io/en/latest/api/pwhash/?highlight=argon2#nacl.pwhash.argon2id.kdf)
I found two documents describing Argon2 in detail:
- https://github.com/P-H-C/phc-winner-argon2/blob/master/argon2-specs.pdf Section 3.1, "Inputs"
- RFC9106: https://www.rfc-editor.org/rfc/rfc9106.html#name-argon2-inputs-and-outputs
Both documents list associated data as inputs to the function. Neither document contains the word "deprecated", and there is no mention of the associated data in the PDF's changelog.
I cannot find credible evidence that authenticating associated data is in any way deprecated. If support for that is missing in PyNaCl, then that is a bug like it is here.
keyid&data can still be used programmatically
Ah, I didn't notice the ParamsBuilder. Maybe it would be helpful to link to it from Params. I'll update the issue's text.
@teythoon I think this argon2 thing can be divided into two layers:
- the algorithm as described by the specs PDF and the the RFC you provided
- the encoding format to turn a argon2 output and it input parameters into a self-contained string
Regarding 1), as you say nothing has been deprecated and the associated data field can still be used freely
However it appears that 2) never handled the associated data field (https://github.com/P-H-C/phc-winner-argon2/pull/173 is about the data part of the encoded hash is not taken into account). On top of that, a keyid field was first added but forgotten along the way.
If support for that is missing in PyNaCl, then that is a bug like it is here.
PyNaCl is a binding on libsodium which doesn't support keyid/associated data in there encoding processing.
This is in line with the official implementation that also doesn't support those field in the encoding processing
In the end the deprecation is about the use of keyid and associated data within the encoding format, not with the underlying algorithm. This is a subtle difference, however this can lead to compatibility issue if an user starts using this crate (and relies on the keyid/associated data fields in the encoded hash), then wants to migrate to another argon2 implementation such as libsodium (in this case the user would have to manually handle the hash encoding 😭 ).
I cannot find credible evidence that authenticating associated data is in any way deprecated
I agree with you that an official declaration is lacking here. However there is evidence that those fields are problematic and should be documented as such.
At this point I think instead of trying to document this, we should instead remove entirely the support to keyid and associated data fields from the encoding processing to comply with official (and libsodium) implementation. @tarcieri what do you think ?
PS: I've created an issue about that in the offcial implementation repo: https://github.com/P-H-C/phc-winner-argon2/issues/342
Lacking further upstream guidance I've retained them for now.
We can revisit this if we can get some proper upstream clarity.
You can also feel free to open a PR to remove them if you'd like to resume the discussion.