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

x/crypto/argon2: expose all variants #23602

Closed
philtay opened this issue Jan 29, 2018 · 16 comments
Closed

x/crypto/argon2: expose all variants #23602

philtay opened this issue Jan 29, 2018 · 16 comments

Comments

@philtay
Copy link

philtay commented Jan 29, 2018

x/crypto/argon2 implements Argon2d, Argon2i and Argon2id variants but exports Argon2i only. The recommended and primary variant of Argon2 is Argon2id (default in libsodium). Moreover Argon2i is not the safest choice in certain scenarios (two published attacks). I propose to add to the package 3 exported functions (i.e. Argon2d, Argon2i and Argon2id). The Key function will be an alias for Argon2i for backward compatibility.

/cc @aead

@gopherbot gopherbot added this to the Unreleased milestone Jan 29, 2018
@aead
Copy link
Contributor

aead commented Jan 29, 2018

While I agree in general that crypto/argon2 should export the 2id variant I want to outline:

  • The recommendation (2id) of the authors is part of a RFC draft
  • I don't see any value in exporting 2d. Actually it does not provide a real advantage compared to 2id and is not side channel resistant.

@philtay
Copy link
Author

philtay commented Jan 29, 2018

Please export all of them. Don't assume. Let the world use all of your work! 2d provides real advantages over 2id in several scenarios. We should not limit ourselves to the "web service authentication" use case. Side channel attacks are simply not an issue sometimes. An example? KeePass selected the 2d variant. Their threat model is different. It depends...

@aead
Copy link
Contributor

aead commented Jan 30, 2018

2d provides real advantages over 2id in several scenarios.

Can you name one?
2id does the 2i block address computation over the first 2 slices of the memory during the first pass For all other block address computations it uses 2d. The only difference is that the first two slices are
computed using data-independent addressing. The mentioned attack cannot be applied to this construction since 2id computes at least (only 1 pass over mem) half of the memory by data-depended addressing.

We should not limit ourselves to the "web service authentication" use case. Side channel attacks are simply not an issue sometimes.

As mentioned already 2id contains the advantages of 2d but is side-channel resistant. There is just no advantage in using 2d instead of 2id. On the other side if 2d is exported people may use it in a scenario where side-channel resistance is required - even though 2id is available. From my point of view just saying "mention this in the doc" does not justify exporting 2d. IMO there should be a stronger argument to justify a "mis-usable" API.

KeePass selected the 2d variant. Their threat model is different. It depends...

Well, they support Argon2d (v1.0 - v1.3). So I guess that they simply took 2d because up to v1.2.1 there was no 2id version. If they would follow the recommendation they would have to prefer 2id over 2d. Probably they just stick to 2d because of simplicity - but that's just my assumption.

@philtay
Copy link
Author

philtay commented Jan 31, 2018

Can you name one?

I already did. I named an entire class of scenarios. 2d is better than 2id in any situation where side-channel attacks are not a threat. You need at least 2-pass 2id to achieve the same resistance of 1-pass 2d against TMTO attacks. Thus, by using 2id instead of 2d, you are forced to reduce the amount of memory for any given time constraint.

There is just no advantage in using 2d instead of 2id.

Advantages just explained above.

IMO there should be a stronger argument to justify a "mis-usable" API.

Of course I generally agree to avoid exposing dangerous primitives, but 2d is not one of them. It's simply targeted to different use cases, not less secure in any way. Please also note that in the current situation we have a "mis-usable" API where only 2i is exposed. This is a direct consequence of the "I know more than the average dev" line of thought. It turned out that 2i is the worst of the 3 variants. Let people choose for themselves.

Probably they just stick to 2d because of simplicity - but that's just my assumption.

I don't know if 2id was already published when they implemented Argon2, but they made the right choice anyway. There is a recommendation to prefer 2id in the general case, but if side-channel attacks are not a threat you should choose 2d.

@aead
Copy link
Contributor

aead commented Jan 31, 2018

TL;DR: I leave that here for @agl to decide. If it get's approval - ping me if I should send a CL.
Of course @philtay feel free to send a CL if you want to take it 😃

Thus, by using 2id instead of 2d, you are forced to reduce the amount of memory for any given time constraint.

I agree on that since for the first half of the memory (of the first pass) the low-storage attack can be applied. However the best attack on 2id with n passes is an approximation of the best TMTO attack against 2d (I think the factor is 1.3 or so). Nevertheless it's correct that for a constant time defender 2d is the best choice in a scenario where side channel attacks are not an issue.

Of course I generally agree to avoid exposing dangerous primitives, but 2d is not one of them.

Well, that's the general problem I'm concerned about here. Every primitive (if not broken) is not dangerous if it is used correctly. The same applies here for 2d. The problem is that this is often not the case. However I think explicitly documenting the pitfalls can work here.

Please also note that in the current situation we have a "mis-usable" API where only 2i is exposed
This is a direct consequence of the "I know more than the average dev" line of thought

Just to defend myself on that:

  1. Any argon2 version is vulnerable to a TMTO attack - the TMTO attack against 2i is "just" more effective than against 2d and 2id.
  2. 2i is side-channel-resistant. So it would be usable in any scenario - of course it would require more resources (time,memory) than 2d. The same is not true for 2d.

So in that sense I do not agree that the API is "mis-usable" right know but I do agree that exporting 2i as the default was a mistake. However the discussion about "mis-usable" is more interpretation then fact-based here...

I don't know if 2id was already published when they implemented Argon2

FWIW That wouldn't be possible. As mentioned the Argon2 spec only named 2i and 2d in e.g. version 1.2. Since KeePass supports 2d (version 1.0) they have not been aware of 2id.

@aead
Copy link
Contributor

aead commented Jan 31, 2018

I guess Proposal and NeedsDecision labels ?!

@agl
Copy link
Contributor

agl commented Feb 4, 2018

Exposing 2id seems to be sensible and without objection. I would suggest a change to do just that in order to avoid tangling it with the question of whether 2d is a good idea.

The idea that everything should be supported and we should let the developers decide is explicitly not what x/crypto aims for. We have also never exposed a "safe but slower" and "unsafe against timing side-channels but faster" pair of functions like this. (We have plenty of problems with primitives that should be side-channel free that aren't, but that's a different matter.)

The argument on the KeePass page that side-channel attacks are irrelevant “because KeePass is a local application, not a remote server” doesn't make sense to me. The side-channel attacks in question are the usual cache (and thus timing) issues, right? So why couldn't local Javascript attack that?

We crippled Javascript's access to timers as best as we could after Meltdown/Spectre so maybe it's not so practical now, but the fact that KeePass may have misunderstood things doesn't support the idea that exposing 2d is a net positive.

@philtay
Copy link
Author

philtay commented Feb 5, 2018

@aead could you please send a CL exposing 2id only? We can further discuss about 2d later. I think that the work you've already done here is basically what we need. Please let me know if I can be of any help.

@gopherbot
Copy link

Change https://golang.org/cl/91935 mentions this issue: argon2: add Argon2id and update parameter recommendations

@MalteJ
Copy link

MalteJ commented Feb 10, 2018

Argon2d is/will be used for proof of work in blockchains.
That's why it should be exposed by this library. Otherwise we will see modified copies of this code in the blockchain client repos.

@gawmanarnar
Copy link

gawmanarnar commented Mar 19, 2018

I'm in agreement with @MalteJ. The draft Argon2 RFC (link to section) specifically recommends Argon2d for cryptographic mining. I was disappointed to see it was not exported when I reached for this library.

@bt
Copy link

bt commented Apr 21, 2018

Any update to this?

Same as @MalteJ and @gimmeasandwich; trying to implement a PoW miner in Golang is impossible because of lacking Argon2d support.

@aead
Copy link
Contributor

aead commented Apr 21, 2018

/cc @FiloSottile

@cenkalti
Copy link

cenkalti commented Jul 9, 2018

Please export 2d variant.

@djkazic
Copy link

djkazic commented Oct 29, 2018

Is there any intent to export Argon2D currently?

@FiloSottile
Copy link
Contributor

I don't think we should break the expectation that crypto/ and x/crypto provide safe primitives for most use cases to enable blockchain PoW algorithms. In particular, I don't want the fastest option in a set of hard to choose amongst options to be the least safe.

A high-performance Argon2d implementation with no security requirements is maybe something the blockchain community should develop in an external repository.

@golang golang locked and limited conversation to collaborators Oct 29, 2019
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
This change exports the Argon2 variant Argon2id and improves documenation.
The following parameter recommendations are added:
 - Argon2i:
   time=3 and max. memory for non-interactive scenarios as recommended by the
   RFC draft https://tools.ietf.org/html/draft-irtf-cfrg-argon2-03#section-9.3
 - Argon2id:
   time=2 and memory=64MB for interactive scenarios as used by libsodium >= 1.0.9
   https://download.libsodium.org/doc/password_hashing/the_argon2i_function.html

   time=1 and max. memory for non-interactive scenarios as recommended by the
   RFC draft linked above.

Fixes golang/go#23602

Change-Id: Ia4d537e6126e5aff1243f2b5579df6bc8edb851a
Reviewed-on: https://go-review.googlesource.com/91935
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
This change exports the Argon2 variant Argon2id and improves documenation.
The following parameter recommendations are added:
 - Argon2i:
   time=3 and max. memory for non-interactive scenarios as recommended by the
   RFC draft https://tools.ietf.org/html/draft-irtf-cfrg-argon2-03#section-9.3
 - Argon2id:
   time=2 and memory=64MB for interactive scenarios as used by libsodium >= 1.0.9
   https://download.libsodium.org/doc/password_hashing/the_argon2i_function.html

   time=1 and max. memory for non-interactive scenarios as recommended by the
   RFC draft linked above.

Fixes golang/go#23602

Change-Id: Ia4d537e6126e5aff1243f2b5579df6bc8edb851a
Reviewed-on: https://go-review.googlesource.com/91935
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
This change exports the Argon2 variant Argon2id and improves documenation.
The following parameter recommendations are added:
 - Argon2i:
   time=3 and max. memory for non-interactive scenarios as recommended by the
   RFC draft https://tools.ietf.org/html/draft-irtf-cfrg-argon2-03#section-9.3
 - Argon2id:
   time=2 and memory=64MB for interactive scenarios as used by libsodium >= 1.0.9
   https://download.libsodium.org/doc/password_hashing/the_argon2i_function.html

   time=1 and max. memory for non-interactive scenarios as recommended by the
   RFC draft linked above.

Fixes golang/go#23602

Change-Id: Ia4d537e6126e5aff1243f2b5579df6bc8edb851a
Reviewed-on: https://go-review.googlesource.com/91935
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
This change exports the Argon2 variant Argon2id and improves documenation.
The following parameter recommendations are added:
 - Argon2i:
   time=3 and max. memory for non-interactive scenarios as recommended by the
   RFC draft https://tools.ietf.org/html/draft-irtf-cfrg-argon2-03#section-9.3
 - Argon2id:
   time=2 and memory=64MB for interactive scenarios as used by libsodium >= 1.0.9
   https://download.libsodium.org/doc/password_hashing/the_argon2i_function.html

   time=1 and max. memory for non-interactive scenarios as recommended by the
   RFC draft linked above.

Fixes golang/go#23602

Change-Id: Ia4d537e6126e5aff1243f2b5579df6bc8edb851a
Reviewed-on: https://go-review.googlesource.com/91935
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants