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
encoding/asn1: Empty integer checks fail on certain OpenSSL CSRs #15373
Comments
Can you provide the CSR? |
|
It seems like I forgot to actually finish writing the title of this issue :| |
It's probably too late for Go 1.7 at this point. I'd like to see it fixed but since Go 1.6 also had the problem it won't make things significantly worse to leave it in Go 1.7. I am assuming Let's Encrypt has already forked their own copy of ParseCertificateRequest (and asn1) in order to make forward progress. All the fields in x509.CertificateRequest are public so it looks like it should be possible to create them with custom code. |
I think the bar for adding workarounds to the base library should be very high. It's unfortunate that we didn't catch this in earlier Go versions and so these Let's Encrypt clients have managed to survive. But I don't like the idea of cementing in the expectation that an empty value for the version is ok here. |
Thinking more about this I agree with @agl that adding a exception to this is, in the long term, the wrong solution. Let's Encrypt is able to work around the problem relatively easily, and is happy enough to wait until the broken client population is small enough to just switch to throwing an error on the empty value as is the current behavior. |
go version
)?go1.6
go env
)?Ubuntu 15.10 x86_64
Build a CSR using pre-1.0.2 OpenSSL language bindings and attempt to parse it using
x509.ParseCertificateRequest
(Gist including short generator).No error
asn1: structure error: empty integer
is returned.(extra) 6. Cause
The pre-1.0.2 OpenSSL API (at least using the Python bindings) requires that the CSR version field be explicitly set otherwise it will not be populated (but the field will be included) causing the
empty integer
error.The Let's Encrypt team ran into this when attempting to migrate from 1.5 -> 1.6 and while we've been able to update our client code to solve the issue there are still a relatively large number being produced by users with old or different clients (~1.4% of CSRs we currently receive each week have this issue).
Removing the empty integer check, along the lines of the negative serial check, wouldn't be a great idea, since this is extremely useful elsewhere. One solution might be to add a workaround ASN1 tag,
brokenInt
or something, which knows how to parse anint
in this situation (i.e. if empty just use 0).cc @bradfitz @agl
The text was updated successfully, but these errors were encountered: