Boulder: Golang errors on extensions marked critical

[danjeffery]

bug appears to be this one:
https://groups.google.com/forum/#!topic/golang-codereviews/uMhu4DIstQI

To reproduce, create a CSR with an extension marked as critical. Golang will throw https://golang.org/src/crypto/x509/x509.go#L1896 when attempting to parse.

[jcjones]

Confirmed with CFSSL sign, so this affects Boulder.

This patch is slated for Golang 1.6, but applies cleanly to 1.5beta3, so we may need to maintain a patched copy of Golang right off the bat if we want to support such CSRs.

openssl genrsa -out my.key 2048
openssl req -new -days 1826 -key my.key -out my.req -config my.config

my.config:

[ req ]
distinguished_name  = subject
req_extensions      = req_ext
string_mask         = utf8only

# The Subject DN can be formed using X501 or RFC 4514 (see RFC 4519 for a description).
#   Its sort of a mashup. For example, RFC 4514 does not provide emailAddress.
[ subject ]
countryName         = Country Name (2 letter code)
countryName_default     = US

stateOrProvinceName     = State or Province Name (full name)
stateOrProvinceName_default = NY

localityName            = Locality Name (eg, city)
localityName_default        = New York

organizationName         = Organization Name (eg, company)
organizationName_default    = Example, LLC

commonName          = Common Name (e.g. server FQDN or YOUR name)
commonName_default      = Example Company

emailAddress            = Email Address
emailAddress_default        = [email protected]

[ req_ext ]
keyUsage            = critical, digitalSignature, keyEncipherment

[jmhodges]

I think we could push on them to get it in for 1.5 if we let them know. Want me to try?

[jcjones]

I think it'd be preferable to get that into 1.5.0, or at least something like 1.5.1. Otherwise we're probably going to be off-mainline. :/

Thanks in advance, @jmhodges .

[rolandshoemaker]

:+1:

[jsha]

It would be great to get it in for 1.5.

To be clear about the impact: We will not support any user-requested x509 extensions at launch, right? And this is a returned error, not a panic. So the problems are these:

• If a subscriber creates a CSR with critical extensions, but those extensions happen to be a subset of the extensions we planned to put in the cert anyhow, we would fail to issue when we should have succeeded.
• If a subscriber creates a CSR with critical extensions that we would reject, we would return to their client a "malformed CSR" error instead of an "unsupported critical extension in CSR" error.

Both of these are undesirable, but if the fix doesn't make it into Go 1.5, they might not be severe enough to merit maintaining an off-mainline Go fork.

[jmhodges]

I sent an email over the weekend and received a kind but definite no for 1.5.0. They'd like for us to update them with how many problems we see going forward because we're likely to get the brunt of them.

It would be nice to find a way to stat the "malformed CSR" failure (esp. relative to other failures) so we can make the call more easily later. (Of course, to be very specific, we'd want a stat for the "unsupported critical extension" but that'd require a fork to get that context out of the API which the stat is to help avoid doing in the first place.)

[jsha]

Is this fixed in 1.5.1?

[hlandau]

I've had problems with this. This seems like a GA bug, in my opinion.

This does not appear to be fixed in the 1.5.x branch.

For reference, the fix is here: https://github.com/golang/go/commit/e78e654c1de0a7bfe0314d6954d42b046f14f1bb

[jsha]

@hlandau: Can you describe more about the problems you're having? What tool are you using to create CSRs? Is it possible to configure them not to include the critical extensions?

[hlandau]

A certificate which makes LE's staging server balk can be generated using GnuTLS's certtool command with no particular customization.

certtool -q --load-privkey=test.key

        Attributes:
        Requested Extensions:
            X509v3 Subject Alternative Name: 
                DNS:example.com, DNS:www.example.com
            X509v3 Basic Constraints: critical
                CA:FALSE
            X509v3 Key Usage: critical
                Digital Signature, Key Encipherment
            X509v3 Extended Key Usage: critical
                TLS Web Server Authentication

[jsha]

Got it, thanks! Definitely valuable input, and will affect how we prioritize this.

In the meantime, as a stopgap, looks like you can use the --no-crq-extensions flag.

[borisovg]

Looks like I hit this issue with CSR generated by xca.

[legomind]

any update on this?

[jsha]

This is blocked on #1514, which in turn is blocked on a few other things.

[hlieberman]

We just had a user get bit by this issue.

Is there a possibility to improve the error handling around this circumstance so we can provide some indication of the proper fix, rather than a generic "oh god we can't parse things" error we give right now?

[jsha]

It's probably possible: We added some similar code in https://github.com/letsencrypt/boulder/pull/1972. However, that was a much simpler check: We just needed to check at a particular offset. For this we'd have to scan for a set of bytes uniquely identifying a critical extension.

Perhaps a better quick fix would be to add a comment to the detail returned from a mis-parsed CSR indicating that it can be caused by critical extensions. What do you think?

[silbe]

Has there any progress on this? AFAICT the Go issue should be resolved by now (#1514 has been closed, apparently after a "Switch to Golang 1.6.3" pull request was merged). Should it just work now or is there something to do on the certbot side as well? (I'm still waiting for 0.10.x to hit Debian backports)

[jsha]

This is fixed. Please reopen if you are seeing this problem.

[eltiare]

For anyone searching on this particular error, it can also happen if you submit a PEM formatted CSR instead of DER.