-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: Certs with odd RDN layouts not handled, cause confusing errors #16836
Comments
Just for reference, the incorrectly structured cert is located at https://github.com/wbond/badtls.io/blob/e46322e240ca97c7316e0f6cbf0b31bba610e707/certs/domain-match.crt. Since this issue was created, I have regenerated the individual certs for badtls.io with a version of the ASN.1 library that generates the standard RDN structure. |
Three choices how to resolve this (as I see):
@agl thoughts? |
I ran a scan of the Pilot CT log for any unexpired chains that contained a certificate with more than one element in an RDN of either the Subject or Issuer. There are only 360 of them, the vast majority appearing to be certificates from the government of Singapore. I think the code should either reject them outright, or else scan them for known values and only reject cases where there are multiple common names or serial numbers etc. Since I suspect that this practice is more widespread outside the Web PKI, I'm leaning towards the latter. Any modern certificate should be including SANs and disabling the processing of common names anyway. (Indeed, the CA/B Forum requires this now.) |
A note on compatibility: in my testing none of Secure Transport, SChannel, OpenSSL 1.0.1 or LibreSSL complained about the abnormal RDNs in the certs that had been generated by certbuilder for badtls.io. I'm not suggesting that means go should take a particular stance, just figured I'd provide that data point. |
CL https://golang.org/cl/30810 mentions this issue. |
go version
)?go env
)?The program in in https://play.golang.org/p/I8S80_1P1p gave a confusing error (#16834 created about the message itself). This issue digs into why he ended up with such a parsed certificate, and what should have happened instead.
The cert in that play snippet is from https://github.com/wbond/badtls.io specifically https://github.com/wbond/badtls.io/blob/master/certs/domain-match.crt
What seems to have happened is that the cert there is structured like
seq(set(a=b, c=d, e=f))
instead of what I seem to be getting from elsewhere,seq(set(a=b), set(c=d), set(e=f))
. I may be very wrong about this, my asn.1 is 10 years rusty.Hopefully, either
or
go/src/crypto/x509/pkix/pkix.go
Line 67 in 7c5f33b
Most of the subject RDN content discarded silently. Confusing error message. Behavior that differs from
curl
on OS X and Debian Jessie.The text was updated successfully, but these errors were encountered: