proposal: x/crypto: add crypt(3) password hash algorithms

[danderson]

I'm writing code that has to generate crypt(3) compatible password hashes, for installation in /etc/shadow. A Google search for a library currently offers two abandoned github repositories, at least one of which is unsafe (ignores returned errors in the crypto logic), and a stack overflow answer that uses cgo to wrap libcrypt.

I'd like to propose adding solid Go implementations of the more common crypt(3) algorithms to x/crypto. Specifically, I'd like to have support for the ${1,5,6}$ algorithms (resp. MD5, SHA256, SHA512), as well as the older DES-based algorithm for universality. The package documentation should include a recommendation against using the crypt(3) algorithms unless compatibility with crypt(3)-using code is necessary, since there exist much better KDFs already in x/crypto if you're working with a clean slate.

If this sounds reasonable, I'm volunteering to provide the implementation.

[ianlancetaylor]

Seems reasonable to me, but CC @agl.

[danderson]

Ping @agl , does this sound like something you'd accept if I send patches?

[danderson]

Ping.

[bradfitz]

With suitable documentation as you mentioned, this sounds reasonable. Feel free to send a CL.

If if it turns out @agl later objects passionately, you can put it under go4.org if you want to give it a non-github import path.

[eikenb]

@danderson Any progress on this? I'm currently using a libpam wrapper but would much prefer a native implementation.

[stapelberg]

Note that in the meantime, https://github.com/GehirnInc/crypt has appeared.

[stapelberg]

There’s another copy of what seems to be largely the same code at https://github.com/tredoe/osutil/tree/master/user/crypt and https://github.com/ncw/pwhash.

I’d say it makes sense to provide a canonical implementation in x/crypto :)

[abbot]

I can give this a stab. I've got some related FRs (e.g. abbot/go-http-auth#75, abbot/go-http-auth#48) and want to move the crypto-related code out of that package. And since there is already some requests to support other crypt(3) algorithms here, I'm happy to add this support.

[SuzukiHonoka]

Any progress now? I'm facing the same problem relating to crypt(3). There are in the GNU C, I have to write an extra C file to call that library, and it is not compatible with windows. I saw some third-party library already supported this now https://github.com/GehirnInc/crypt, would you guys take a look? Thanks.

[ianlancetaylor]

Somebody will need to write and contribute the code. This is not something the core Go team has the bandwidth to take on.

[rolandshoemaker]

Hi from the Go Security team 👋, sorry it has taken so long for someone from our team to address this, it completely flew under our radar. Since this proposal was accepted in 2016 a couple of things have changed that makes me think we should revisit the decision to accept it.

In particular, we've recently decided to freeze the x/crypto module, with the intention of moving most of its content into the standard library, so I don't think we should be adding anything new there. Additionally, adding password hashing mechanism to the standard library that relies on MD5 feels like the wrong choice in 2024, is there still a need to support this specific part of crypt(3)? Are people really still generating MD5 based password hashes?

I wouldn't be strictly opposed to including the SHA based variants, although we've also thought about working on a more unified password hashing API, rather than having a number of disparate implementations (of which, as mentioned in the initial comment, there are better choices unless you really need compat).

Another thing to consider is if this really needs to be a part of the standard library. The bar for what we include is somewhat higher than it was in 2016, and while there may be a need for a solid implementation of crypt(3), I'm not sure if it's something that we should be providing to all users of Go. The third-party Go module ecosystem is much more robust these days, and a community developed module that supports this functionality seems like a reasonable option. Is there a particular reason that I'm missing that would require us to put this in the standard library (other than it being stamped as approved by the Go team)?

[SuzukiHonoka]

Hi from the Go Security team 👋, sorry it has taken so long for someone from our team to address this, it completely flew under our radar. Since this proposal was accepted in 2016 a couple of things have changed that makes me think we should revisit the decision to accept it.

In particular, we've recently decided to freeze the x/crypto module, with the intention of moving most of its content into the standard library, so I don't think we should be adding anything new there. Additionally, adding password hashing mechanism to the standard library that relies on MD5 feels like the wrong choice in 2024, is there still a need to support this specific part of crypt(3)? Are people really still generating MD5 based password hashes?

I wouldn't be strictly opposed to including the SHA based variants, although we've also thought about working on a more unified password hashing API, rather than having a number of disparate implementations (of which, as mentioned in the initial comment, there are better choices unless you really need compat).

Another thing to consider is if this really needs to be a part of the standard library. The bar for what we include is somewhat higher than it was in 2016, and while there may be a need for a solid implementation of crypt(3), I'm not sure if it's something that we should be providing to all users of Go. The third-party Go module ecosystem is much more robust these days, and a community developed module that supports this functionality seems like a reasonable option. Is there a particular reason that I'm missing that would require us to put this in the standard library (other than it being stamped as approved by the Go team)?

Hi, thank you for your quick response.
The crypt(3) is commonly used for encrypting the user password in unix based system, as you can see the file in /etc/shadow.
Meanwhile, the latest crypt(3) supports sha256, sha512 and etc, for md5 security issues, you guys can set that as deprecated and there will be no harm. The only way to use crypt(3) now is to call the library in glibc, this make things very complex, since the support of glibc on windows server or darwin can be missing.
Please reconsider it, thank you.

[SuzukiHonoka]

Somebody will need to write and contribute the code. This is not something the core Go team has the bandwidth to take on.

Hi, thank you for your quick response.
I wish I could contribute the code to go core library, but I am not so familiar with it and have no faith to write the such thing to global used library.
I hopy you could understand, right now I can only use that third-party library.
It's still good if you guys can include it to the go library.
No rush, thank you.

[danderson]

Since filing this eight years ago, I've not needed this function. I can't quite remember what I was up to that I needed it in the first place. I assume I was trying to build a linux distro again, I tend to tilt at that windmill every few years.

The proposal to add it to x/crypto was driven by finding the existing 3p implementations unsafe (e.g. ignoring errors on fallible crypto primitive calls, segfaulting through incorrect use of cgo), and being worried that people end up plugging them into programs and shooting themselves in the foot.

Surveying the landscape again using the same method of "I'll just google the problem I have and import the first library it says", I found 2 implementations of crypt(3) above the fold, after a couple go/bad results pointing at std/crypto and x/crypto:

• https://github.com/simia-tech/crypt supporting md5crypt, sha1crypt, sha2crypt, bcrypt and Argon2. Not updated for several years, so wouldn't expect the KDF selection to expand.
• https://github.com/go-crypt/crypt supporting md5crypt, sha1crypt, sha2crypt, PBKDF2, bcrypt, scrypt and Argon2. Updates in the last few weeks, solo developer.

I don't immediately recognize the author names as well known Go crypto people, and I've not reviewed the code for correctness or safety. But purely feature-wise, this is a much better offering than what existed when I filed this proposal, and effort may be better spent on putting more eyes and tests on those implementations instead.

For cryptography code, my bias is to distrust random 3p implementations more than baseline, because implementing cryptography correctly is a somewhat niche skill, and it can take an expert to tell the difference between a robust and an unsafe offering. That's why I was arguing for an x/crypto implementation: std/ and x/ have a well-known and reasonably high bar for quality, so if an implementation exists there I feel much safer not digging into its guts to check how it's made.

Inclusion in std/crypto would follow similar logic, however it now needs to balance against adding weight to the stdlib, and adding API surface for "you really shouldn't be using this unless you have no other choice at all" type functionality, which isn't great. OTOH neither is being told to simply use Argon2id when the appliance you're trying to puppet with Go doesn't understand anything fancier than sha1crypt :) But my argument, if you accept it at all for x/crypto, is definitely weaker if applied to std/crypto, so I could go either way personally.

[rsc]

Adding back to the proposal process.

[rsc]

From the discussion it sounds like we could reasonably decline this, now that for example github.com/go-crypt/crypt exists.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group