-
Notifications
You must be signed in to change notification settings - Fork 17.4k
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: inner and outer signature algorithm identifiers don't match #47689
Comments
cc @FiloSottile @rolandshoemaker |
vs
Sigh. We might have to tolerate a NULL parameters mismatch? @rolandshoemaker FWIW the certificate can be patched to parse correctly, since the outer signature algorithm is not signed. |
We've managed to mostly excise special exceptions for broken certificates, and I'm loathe to add more. This seems relatively rare, unless we start to see a significant number of breakages (that cannot be reasonably worked around) I think we should just accept that we're not going to parse some malformed certificates. |
According to the spec (https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.1.2) the "parameter" field is optional, so you should treat both representations (with or without the optional "NULL" field) as equivalent. |
I've just run into this error myself. It is in relation to a third party cert I have no control over, and thus cannot change. Firefox has no issue loading the certificate (aside from the usual self-signed warning). It "just works". However I'm unable to establish a TLS connection within go at all because it flat-out refuses to parse the certificates. This seems less than desirable from a usability perspective. I don't even have a way to override (such as with InsecureSkipVerify or VerifyPeerCertificate). At this point I don't see how I can connect to this particular host from golang, period. Edit, 22 days later: As I have no way to use a
Now my TLS connection works! An awful lot of work to go to just in order to get it to accept a non-ideal certificate, whereas a web browser does it without any grief whatsoever. I'm all for standards-compliance - but a tls.config flag to allow it to be lax in this regard would have saved me hours and hours (and now, a much more difficult build process for my project). |
TL;DRdifferent ASN1 values can represent the same algorithm and golang wrongly compare the ASN1 bytes to check such similarity. WhyI believe that the issue relies upon the ambiguity in RFC 5280:
As you can see, it is not clear whether it refers to the entire sequence, including the parameters, or just the Regardless of this ambiguity, the RFC doesn't define same, and I can assume that the similarity check depends on the algorithm type.
The above implies that
and
Are the same, satisfying Section 4.1.1.2 and Section 4.1.2.3. This is also true for
In conclusion, it appears that different ASN1 values can represent the same algorithm and golang wrongly compare the ASN1 bytes to check such similarity. Thank you for all your help and effort. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Does not reproduce in
go version go1.16.6 darwin/arm64
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/-rDc14aTqgV
The
com.apple.systemdefault
certificate is a self signed root that Apple generates during macOS setup. It lives in most (all?) macOS user keychain. I recently discovered that go.17rc2 fails to parse some (but not all) of the Apple system keychain certificates. The one in the example was issued in 2015. It's possible Apple issued some of these certs erroneously, and fixed the mismatch in a follow-up release.I've only detected the failure on about 10 out of 100k macOS devices, but our environment has quick refresh cycles for devices. It's possible the problem would be more widespread for other users parsing the macOS System Keychain with Go.
What did you expect to see?
What did you see instead?
The text was updated successfully, but these errors were encountered: