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

crypto/x509: reject SHA-1 signatures in Verify #41682

Open
FiloSottile opened this issue Sep 28, 2020 · 17 comments
Open

crypto/x509: reject SHA-1 signatures in Verify #41682

FiloSottile opened this issue Sep 28, 2020 · 17 comments

Comments

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Sep 28, 2020

SHA-1 is weak: a SHA-1 collision was demonstrated and estimated to cost around $50k. https://shattered.io

Accepting SHA-1 signed certificates is a security issue, and lets attackers mount collision attacks if the CA is still signing SHA-1 certificates. crypto/x509 already rejects outright any MD5 signatures for the same reason.

The WebPKI has banned SHA-1 certificates for years now, and crypto/x509 targets a profile compatible with the WebPKI.

I propose we announce in Go 1.17 that we'll remove support in Go 1.18, and provide a GODEBUG opt-out until Go 1.19.

@FiloSottile FiloSottile added this to the Backlog milestone Sep 28, 2020
@FiloSottile FiloSottile removed this from the Backlog milestone Oct 20, 2020
@FiloSottile FiloSottile added this to the Go1.17 milestone Oct 20, 2020
@FiloSottile

This comment has been hidden.

@FiloSottile FiloSottile self-assigned this Mar 17, 2021
@FiloSottile FiloSottile changed the title crypto/x509: stop verifying SHA-1 signatures proposal: crypto/x509: stop verifying SHA-1 signatures Apr 7, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Apr 7, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Apr 7, 2021

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 rsc moved this from Incoming to Active in Proposals Apr 7, 2021
@ianlancetaylor

This comment has been hidden.

@rsc
Copy link
Contributor

@rsc rsc commented Apr 14, 2021

How many of the ancient servers being discussed in #45428 are serving SHA-1 signatures?

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Apr 14, 2021

SHA-1 in crypto/x509 is unrelated to crypto/tls, except to the extent that if you're running a legacy stack you're more likely to have both components be out of date. You can serve a SHA-1 certificate over TLS 1.3, if you felt like it.

There are no publicly trusted SHA-1 certificates anymore, so we pretty much have no numbers about them. (Well, we do, and they say zero, but they don't capture internal deployments.) Anyone using them is doing it with their own managed CA.

@rsc rsc changed the title proposal: crypto/x509: stop verifying SHA-1 signatures proposal: crypto/x509: reject SHA-1 signatures in Verify Apr 21, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Apr 21, 2021

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

@rsc rsc moved this from Active to Likely Accept in Proposals Apr 21, 2021
@rsc rsc moved this from Likely Accept to Accepted in Proposals Apr 28, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Apr 28, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: crypto/x509: reject SHA-1 signatures in Verify crypto/x509: reject SHA-1 signatures in Verify Apr 28, 2021
@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Jun 15, 2021

https://golang.org/cl/327811 has the pre-announcement, moving to Go 1.18 for implementation.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 15, 2021

Change https://golang.org/cl/327811 mentions this issue: doc/go1.17: add Go 1.18 pre-announcements

gopherbot pushed a commit that referenced this issue Jun 21, 2021
Updates #41682
Updates #45428

Change-Id: Ia31d454284f0e114bd29ba398a2858fc90454032
Reviewed-on: https://go-review.googlesource.com/c/go/+/327811
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 31, 2021

Change https://golang.org/cl/359777 mentions this issue: crypto/x509: disable SHA-1 signature verification

gopherbot pushed a commit that referenced this issue Nov 5, 2021
Updates #41682

Change-Id: Ib766d2587d54dd3aeff8ecab389741df5e8af7cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/359777
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 16, 2021

@FiloSottile Is there anything more to do here for 1.18? Thanks.

@andig
Copy link
Contributor

@andig andig commented Dec 12, 2021

I'm wondering if this is maybe premature. Trying to connect to my server I'm seeing

authentication handshake failed: x509: certificate signed by unknown authority (possibly because of \"x509: cannot verify signature: insecure algorithm SHA1-RSA (temporarily override with GODEBUG=x509sha1=1)\" while trying to verify candidate authority certificate \"serial:...\")

which seems related to this change. The fun part is that the ca and server certificates were only generated a year ago using OpenSSL with default setting afaikt. It seems as if this would hit a broader audience in a bad way if OpenSSL default certificated are rejected?

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 21, 2021

@andig did you see the opt-in using GODEBUG=x509sha1=1 go run main.go? That might resolve your problem as this issue is to intentionally opt out until opted in. I am also going to punt this to Go1.19 since we've fulfilled the goals per the original issue.

@odeke-em odeke-em removed this from the Go1.18 milestone Dec 21, 2021
@odeke-em odeke-em added this to the Go1.19 milestone Dec 21, 2021
@andig
Copy link
Contributor

@andig andig commented Dec 21, 2021

@odeke-em I'm aware of the switch. Nevertheless I would expect Go to successfully connect to something that was setup using default OpenSSL config.

I am also going to punt this to Go1.19 since we've fulfilled the goals per the original issue.

What does that mean? Right now gotip still requires opt-out, so this is part of 1.18. Should I open new issue to discuss moving it to 1.19?

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Dec 21, 2021

@andig this kind of change is a balancing act. You're right that a big part is what other important players in the ecosystem are doing. Regrettably, OpenSSL CLIs move too slowly to be a benchmark. Apparently, the OpenSSL project considers them testing, not production tools. If we were to wait until years have passed since the OpenSSL CLI defaults caught up with the world, we'd be always 10 years behind, which is an unacceptable cost. Moreover, the situation is made worse by long-cycle Linux distributions such as Debian, which will ship to users versions from several years ago. At the end of the day, the OpenSSL CLI made you a certificate that even at the time was not going to work with any browser. Letting it work with Go is a disservice, as if it didn't you'd probably have noticed that OpenSSL was not doing the right thing.

@mnordhoff
Copy link

@mnordhoff mnordhoff commented Dec 21, 2021

As an example, I think Debian unstable changed the default to SHA-256 in like 2013, inherited by later Debian and Ubuntu releases at some point. Many environments should be generating acceptable certificates by default, I think.

@andig
Copy link
Contributor

@andig andig commented Dec 22, 2021

I realise this is now absolutely my problem and not related to Go, but I'm totally failing to sign a CSR with subjectAlternateNames on OSX using SHA256. If anyone here has a pointer to a guide that would be greatly appreciated. Please excuse the OT response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants