proposal: x/crypto/argon2: support secret value and associated data

[except]

Hello,

It seems though the exported functions of x/crypto/argon2 only permits for customised parameters relating to generation and obviously the salt and password for hashing. Being able to use a secret key would in theory increase security.

[bcmills]

CC @FiloSottile

[FiloSottile]

I assume you are referring to the inputs that the draft calls "Secret value K" and "Associated data X". The package API is already a handful as it is, I am not inclined to add support for them, unless there are protocols that require their use.

[except]

I just thought it was useful to be able to use the entirety of argon2, but I agree the package API does require many parameters already and it doesn’t seem as though any protocols need to use it.

[rsc]

Adding to proposal minutes.

[rsc]

There doesn't seem to be much interest in this feature. Based on the discussion above, this seems like a likely decline.

[rsc]

No change in consensus, so declined.

[marwan-at-work]

Hi 👋 @rsc @FiloSottile

Not sure if my +1 here makes any difference, but I'd love to see the secret (K) value exposed in x/crypto/argon2.

The motivation here is that other argon2 implementations such as Ruby/C, allow users to pass K, and the fact that Go's implementation doesn't creates friction for developers who are migrating their code from said languages to Go.

For example, I am tasked with re-writing some Ruby code to Go where in the transition period, the Go code will generate the argon2 hashes and the Ruby code should continue to be able to verify it (as both Go and Ruby will share the same secret in Vault) until the Ruby code is fully swapped out. Of course, this is not possible unless x/crypto/argon2 allowed me to pass K.

From what I can see, the secret K implementation actually already exists in x/crypto/argon2 and is tested as well, it just needs to be exposed?

Thank you!

[marwan-at-work]

Re-opening the issue for visibility. But please feel free to close again.

[except]

To avoid a breaking change, one could go down the route of argon2.IDKeyWithSecret.

[elgunhuseynli]

@rsc @FiloSottile

Deriving keys without secret allow attackers to crack passwords offline in case of database breach. Because salt and other parameters are stored in leaked database, and no secret is involved. Using secret and not storing it in database can add extra layer of security.

x/crypto/argon2 already implements this and all tests are passed. Why not expose secret parameter?

Also it is needed for compatibility with other libraries and existing projects.

Thanks.

[seankhliao]

Closing as I see the decision has been made to not include this.

[peter021]

This is a bad decision. These parameters are required to be able to implement NIST SP 800-108r1 and NIST SP 800-56C REV. 2. Also good password storage practise (https://www.ietf.org/archive/id/draft-ietf-kitten-password-storage-04.html) is to use a pepper, which is the purpose of the "secret"-parameter. I wanted to use a memory safe language for this and chose go initially because https://go.googlesource.com/proposal/+/master/design/cryptography-principles.md promised that I could. But it seems now that "safe" and "modern" are just pretty words. I guess my python code won't get ported to go at this time.

[dpifke]

I guess my python code won't get ported to go at this time.

While I agree with you that this decision is unfortunate, the less dramatic solution is to fork the Argon2 library out of /x/crypto. Speaking as someone who has had to do this (for compatibility with Argon2 hashes shared with a non-Go library—almost exactly the scenario described in #35481 (comment)), exposing those parameters is a pretty trivial modification, and shouldn't affect backporting any future changes from upstream.

Now that this decision is final, hopefully someone in the community will step up and maintain such a fork, similar to what happened with Proton Mail and /x/crypto/openpgp. I realistically don't have the bandwidth in the short-term, or else I would offer.

[peter021]

The bigger issue is that best practise, standards compliant featured doesn't seem to be asked for in this community or in the standard library. What other shortcuts and limitations is hidden in the standard library? If the standard library has no way of producing a peppered password, what conclusions can be drawn about resulting products? If there is a relevant module in the library then why shouldn't issues be fixed where they are? If forking is the natural reaction, what other parts of the standard library should not be used? Can this be clearly marked in the standard module documentation going forward?

[peter021]

You're right, I removed the sweeping accusations and generalisations. Sorry about that, got a little carried away.
Some of the remaining questions may be important to consider though. They don't need answers, and they represent a single point of view among others. Statistics show that if one person voice an oppinion there are generally another 24 people holding the very same oppinion. Also we are biased to think that everyone thinks the same way we do, which is usually only right about half of the time.
I write this not to downvalue the work that you guys do, I write this to try to convince people who are better than me to fix something important that is broken. I'm trying to help you guys improve the language and any resulting product that is built using it. I am a moonshine developer and a very recent beginner at Go and shouldn't be let near any standard library code. Security standards however is my back alley. My day job is about protecting critical infrastructure.