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

badtls.io certificates created with an old version of asn1crypto? #1

Open
jviide opened this issue Aug 25, 2016 · 9 comments
Open

badtls.io certificates created with an old version of asn1crypto? #1

jviide opened this issue Aug 25, 2016 · 9 comments

Comments

@jviide
Copy link
Contributor

jviide commented Aug 25, 2016

wbond/asn1crypto@e1fa6cd fixed how asn1crypto.x509.Name.build() encodes distinguished names. However badtls.io might still serve certificates created with an older version of asn1crypto.

A bit of background: ouspg/trytls#231 stumbled onto how go1.7 fails some badtls.io tests with rather surprising error messages. @joneskoo created issue golang/go#16834, which in turn spawned issue golang/go#16836, which in turn hints that the trigger might have been the way x509.Name.build() used to work.

@wbond
Copy link
Owner

wbond commented Aug 26, 2016

Yes, I should regenerate them. I suppose this means that hard-coded CA certs will break in existing integrations.

Any thoughts on what you would consider an alternative to posting a notice in the readme that the root CA changed?

@joneskoo
Copy link

joneskoo commented Aug 26, 2016 via email

@jviide
Copy link
Contributor Author

jviide commented Aug 26, 2016

One option would be to serve the regenerated certs from new ports, and keep the old tests/ports functional but undocumented for a reasonable period of time (maybe until some valid cert expires).

That would be more of a burden to maintain, though.

@wbond
Copy link
Owner

wbond commented Aug 26, 2016

It may be sufficient for now to regenerate all but the CA certificate. I suppose it depends on whether any of the clients are running into issues with the CA. Generally the CA isn't validated in any way, and is usually used just for the public key, key identifier, and comparing the subject to the issuer of issued certs.

I will generate new certs for all but the CA and we can see how Go does with it. So far, OpenSSL, SChannel and SecureTransport all seem okay with the non-standard RDN encoding. I'll probably end up moving over to new ports and keeping the old ones running with the old CA for backwards compatibility. However this will probably happen next week since I am on holiday now.

@joneskoo
Copy link

joneskoo commented Aug 26, 2016

Once you regenerate the cert you can check the cert by pasting it to this
test code in go playground and run it online in browser.

https://play.golang.org/p/I8S80_1P1p

Hope this helps.

According to @tv42 openssl 1.0.2d (with curl I believe) did not accept the unusual cert
either; I did not confirm.

@wbond
Copy link
Owner

wbond commented Aug 28, 2016

I've regenerated everything but the CA cert, and https://play.golang.org/p/Uv4r0RFkGr is now successful.

I've also run the oscrypto test suite on Travis and Appveyor, and they both like the new certs.

I'm going to leave this open for now to remind me to come up with a migration plan to have a correct CA certificate.

@joneskoo
Copy link

Confirmed new certs validate now fine with go net/http. Thanks for the fix. 🍰

I think you're right about CA not being validated. I think where it may come up is if e.g. the conclusion in Go ends up being to reject the certificate as bad when parsing, fail fast principle. In that case it's plausible a future version might reject the whole certificate.

@jviide
Copy link
Contributor Author

jviide commented Aug 28, 2016

Great, thank you!

@woodruffw
Copy link

Apologies for the necropost here, but I'm seeing a variant of this issue for PyCA Cryptography's X.509 parser (which is strict about DER SET OF ordering):

  File "/Users/william/devel/x509-limbo/env/lib/python3.12/site-packages/cryptography/x509/base.py", line 594, in load_der_x509_certificate
    return rust_x509.load_der_x509_certificate(data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: error parsing asn1 value: ParseError { kind: InvalidSetOrdering, location: ["Certificate::tbs_cert", "TbsCertificate::issuer", 0, 2] }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants