cfrg / draft-irtf-cfrg-opaque

The OPAQUE Asymmetric PAKE Protocol
https://cfrg.github.io/draft-irtf-cfrg-opaque/draft-irtf-cfrg-opaque.html
Other
100 stars 21 forks source link

recommend argon2i instead of scrypt in configurations #376

Closed stef closed 1 year ago

stef commented 2 years ago

i would like to reopen this issue: https://github.com/cfrg/draft-irtf-cfrg-opaque/pull/155#discussion_r586922995 since the reason for this is now solved, as there is now a rfc for this: https://www.rfc-editor.org/rfc/rfc9106.txt

stef commented 2 years ago

quoting here the original answer from the above review comment, since github hides it:

This is not a great answer, but.... we can't properly cite Argon2i as it's not yet an RFC. (The same is true of the OPRF draft, but that's a dependency we can't workaround.)

We can certainly change this to argon2i, or whatever, provided there's a suitable reference and evidence that the new thing is more widely supported than scrypt. The purpose of this is to be a recommendation for applications should they not know what they're doing.

kevinlewi commented 1 year ago

@stef Do you have a recommendation for the exact parameters for Argon2?

Right now, the recommendation for scrypt reads as: Scrypt(32768,8,1)

What would be the equivalent for Argon2 / Argon2i (and what about Argon2id)?

Edit: Or perhaps we can just be ambiguous about this, and simply state Argon2 without specifying any parameters such as recommended salt length, etc. (which are already stated in the Argon2 RFC anyway)

stef commented 1 year ago

@stef Do you have a recommendation for the exact parameters for Argon2?

Right now, the recommendation for scrypt reads as: Scrypt(32768,8,1)

this seems to me approximately as an INTERACTIVE setup.

What would be the equivalent for Argon2 / Argon2i (and what about Argon2id)?

the paper only gives an algo how to determine these: https://github.com/P-H-C/phc-winner-argon2/raw/master/argon2-specs.pdf

looking at libsodium these are the parameters for the interactive versions of argon2:

crypto_pwhash_argon2id.h
66:#define crypto_pwhash_argon2id_OPSLIMIT_INTERACTIVE 2U
70:#define crypto_pwhash_argon2id_MEMLIMIT_INTERACTIVE 2**26

crypto_pwhash_argon2i.h
66:#define crypto_pwhash_argon2i_OPSLIMIT_INTERACTIVE 4U
70:#define crypto_pwhash_argon2i_MEMLIMIT_INTERACTIVE 2**25

while the argon2 rfc recommends two settings, one using 2GB ram (which i think is not workable for phones) and a second one using 64MB ram:

If much less memory is available, a uniformly safe option is Argon2id with t=3 iterations, p=4 lanes, m=2^(16) (64 MiB of RAM), 128-bit salt, and 256-bit tag size. This is the SECOND RECOMMENDED option.

Edit: Or perhaps we can just be ambiguous about this, and simply state Argon2 without specifying any parameters such as recommended salt length, etc. (which are already stated in the Argon2 RFC anyway)

i think the default variant should be Argon2id which is a hybrid protecting against sidechannels and is also the mandatory variant as per the RFC.

so i guess we can/should recommend also argon2id.

regarding what parameters, i have asked @jedisct1 why libsodium uses different params than the rfc (i guess because the latest sodium release predates the rfc) - i think it makes sense to choose parameters that are already widely used as defaults, thus the libsodium params as an alternative.

jedisct1 commented 1 year ago

Especially with interactive settings, using multiple lanes to compute a single hash can quickly turn into a DoS vector. Doubling memory usage instead provides a similar security level.

That being said, none of these recommended settings is really useful. It all depends on the resources the service is going to have, what latency is acceptable for an interactive session, how many concurrent sessions will run in parallel, and what else will be sharing the same resources.

kevinlewi commented 1 year ago

Thanks for the comments and for providing the analysis! Perhaps we can avoid going into this discussion in the draft by just mentioning "Argon2" instead of "Scrypt(...)" as a recommendation without going into further details.

See https://github.com/cfrg/draft-irtf-cfrg-opaque/pull/378 for the proposed change, and let me know if this looks OK, or if you think we should explicitly say "Argon2id" or something even more specific.

jedisct1 commented 1 year ago

Looking good!

For interoperability purposes, explicitly saying "Argon2id` may be a good idea, though.

stef commented 1 year ago

That being said, none of these recommended settings is really useful. It all depends on the resources the service is going to have, what latency is acceptable for an interactive session, how many concurrent sessions will run in parallel, and what else will be sharing the same resources.

@jedisct1 this KSF is only running on clients devices, not on servers.

jedisct1 commented 1 year ago

That was the justification for libsodium's parameters.

But on clients, doing recommendations is equally device-specific.

kevinlewi commented 1 year ago

Thanks. I changed the text to recommend Argon2id instead of Argon2 in the PR.

bytemare commented 1 year ago

Negotiating the parameters as part of the protocol means we need the client to be able to verify/authenticate the parameters coming from the server. As @jedisct1, there's a risk of DoS with malicious parameters. But the KSF is used as part of the OPRF-PRK output, so we can't do that.

I think parameters must therefore be defined or negotiated outside of the protocol, e.g. as application configuration, hardcoded parameters. @kevinlewi should we add that to the doc?

stef commented 1 year ago

actually it is not necessary to negotiate anything! the KSF and its params are completely client-side, and the server has no idea, and no way of verifying if a certain KSF configuration has been used by the client. as such the KSF and its configuration can be either just stored in a config file (next to the server address) on the client. or in many cases just use a default config which can be changed on the UI/preferences/configfile.

chris-wood commented 1 year ago

+1 to @stef -- no negotiation is needed here. These all should be configured by the application as needed.

chris-wood commented 1 year ago

Sorry for chiming in late here, folks. I want to gently push back on the general idea that we ought not to included recommended parameters for Argon2. I think applications should of course be able to choose the parameters that bet fit their use case and threat model. However, for the recommended profiles or configurations, which are meant to be turnkey options ready for use, I don't think we should burden the application with this choice. I think we ought to pick reasonable defaults here.

stef commented 1 year ago

since this is all gonna run on clients, and since a majority of the clients can be expected to be phones, and since i doubt that phones have 2GB of ram lying around for a KSF i recommend to use the 2nd recommendation from the rfc, which only needs 64MB of ram.

kevinlewi commented 1 year ago

Got it. So right now we have one vote from @chris-wood for the first recommended option, and one vote from @stef for the second recommended option from the Argon spec.

I think I am slightly in favor of the first recommended option, since I would say that we do not know what clients to expect, and if there are indeed clients with memory constraints, they can simply choose to use the second recommendation on their own.

Also, we already caveat the recommendations with the wording: "Absent an application-specific profile ..."

Let me know if you have a particularly strong opinion against this @stef. For now, I'll put up a PR which recommends the first recommendation.

stef commented 1 year ago

nah, go for it! thx! <3

kevinlewi commented 1 year ago

Re-opening this issue based on the topic in here: https://mailarchive.ietf.org/arch/msg/cfrg/7xBO1Yb8uYvb6-jPl7IKHbTqO8Q/

I am OK with restoring the previous recommended configuration:

P256-SHA256, HKDF-SHA-256, HMAC-SHA-256, SHA-256, scrypt(32768,8,1), P-256

in addition to the Argon one. Note that we don't supply test vectors for the slow hashing function anyway, so this would effectively just be a one-line change.

@stef, @jedisct1: wanted to get your thoughts on this as well, if you are against doing so. Thanks!

chris-wood commented 1 year ago

Closing as resolved.