x/crypto/ocsp: ParseResponse makes incorrect choices about response and issuer verification

[cipherboy]

x/crypto/ocsp makes an incorrect assumption about the contents of the BasicOCSPResponse's certs field and has a broken API as a result.

---

Per RFC 6960 Section 4.2.1 ASN.1 Specification of the OCSP Response:

The responder MAY include certificates in
the certs field of BasicOCSPResponse that help the OCSP client verify
the responder's signature. If no certificates are included, then
certs SHOULD be absent.

Notably, the RFC makes no assertions about the contents of these certificates, whether they are strictly delegated OCSP issuers (as discussed in another Go issue or whether they can include or not include intermediate CAs necessary to tie the OCSP responder's cert back to a concrete root.

However, x/crypto/ocsp's ParseResponse and ParseResponseForCertificate incorrectly assume that any certificates included in the response are a delegated OCSP signer and its chain, assuming the delegated OCSP signer is the first certificate. This is not guaranteed by the RFC and is not assumed by other OCSP libraries. Go is unique and non-conformant in this regard.

In particular, if the caller passes an issuer parameter value, Go incorrectly assumes that this issuer must have either directly issued the OCSP response or issued the first certificate in the BasicOCSPResponse's certs field.

This has no basis in the RFC.

Further, and more risky is the beavhior around issuer=nil: here, while the BasicOCSPResponse's certs field is used to verify the signature, no grounding to a trusted certificate. This is only safe over a secure transport for OCSP requests/responses and the docs make no mention of this. (cc @golang/security and @rolandshoemaker).

---

As discussed at a previous issue, this assumption about the structure of BasicOCSPResponse is at least historically invalid for public CAs.

For private CAs:

• Vault's PKI OCSP responser provisions the Certificates field pointing to the leaf's issuing CA (which also issued the OCSP response).
• Dogtag PKI's OCSP responder does similarly, but supports external (delegated) OCSP certificates in addition to provisioning the full chain of the leaf.
• Without running, it appears the Packetfence OCSP responder follows the same approach wherein the CA certificate is again provisioned here in the Certificate field.

I've not checked other CA libraries, but given we've now found three independent private CAs with this behavior that aren't forbidden by spec, and given Go seems alone in this behavior (and other tools like openssl do the right thing), it seems like fixing Go is the correct approach.

On the usage side, the story is more complicated.

• Square's certigo library uses the issuer in the response field, meaning if they'd be affected by this issue in Go's OCSP parsing if contacting one of these private CAs.
• Snowflake's ocsp.go library has the same issue.
• Caddy's certmagic has the same issue.
• Google's webpacker, might appear to use this in a security-critical context with a nil issuer, though it looks like the chain-related version below has correctly specified an issuer.
• Digitorus's pdfsign appears to read OCSP info out of the PDF context and attempt to verify the info there, which might be considered a security problem.

This shows that consumers of Go's OCSP library are likely unaware of the implications of the API design of passing a non-nil issuer.

Most usages of nil parameters seem safe on first glance, but their authors should probably investigate further. Since I didn't see any really obvious incorrect usages, and the other behavior is a fail-closed behavior, I did not think this warranted a security issue.

---

I'd propose the following fixes:

1. Update the docs to more adequately warn about specifying a nil issuer here.
2. Update the API to correctly return all certs fields, allowing callers to perform more advanced chain building with a nil issuer parameter if they desire. See conversation below.
3. Fix the library to not err if issuer == certs[0] (i.e., if issuer != certs[0], do the signature check that exists today).
4. Iterate over passed certificates to find the correct one that signed this request.

This appears to be the minimally invasive sets of changes that allow more complex libraries tao get more correct behavior while affecting the library the least.

As I get time, and if people here are happy with this, I'll open a CL with these changes.

---

See also: #21527

See also: #40017

Reproducer: https://go.dev/play/p/Nr-VKOD_fxH

---

What version of Go are you using (go version)?

$ go version
go version go1.20.3 linux/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cipherboy/.cache/go-build"
GOENV="/home/cipherboy/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/cipherboy/go/pkg/mod"
GONOPROXY="github.com/hashicorp"
GONOSUMDB="github.com/hashicorp"
GOOS="linux"
GOPATH="/home/cipherboy/go"
GOPRIVATE="github.com/hashicorp"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/cipherboy/GitHub/cipherboy/vault/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build652088920=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Above, including reproducer.

What did you expect to see?

Go should not have erred on the first ocsp.ParseResponse call with the issuer.

What did you see instead?

Go erred with bad OCSP signature: crypto/rsa: verification error

[seankhliao]

cc @golang/security

[rolandshoemaker]

Notably, the RFC makes no assertions about the contents of these certificates, whether they are strictly delegated OCSP issuers (as discussed in #40017 or whether they can include or not include intermediate CAs necessary to tie the OCSP responder's cert back to a concrete root.

I think the RFC is a little vague and confusing (in an extremely PKIX-y way) here. There is no restriction put on what can be in certs, just saying that they help the OCSP client verify the responder's signature. That said RFC 6960 Section 4.2.2.2. requires that if a delegated signing certificate is used it should be issued by the certificate that signed the certificate being checked. In the delegation case certs then only really needs to contain a single certificate, but the RFC leaves the door open to put whatever else you want in there.

In theory you can put the rest of the chain (e.g. delegated signer -> intermediate issuer -> root) in the response, but we have no interest in verifying the rest of that chain (or any other chain in the response) since we only care that the delegated signer links back to the issuer of the certificate being checked (https://go.dev/cl/57510 contains some previous discussion of this behavior, and why we do what we do).

I think one of the reasons the API is designed in the way it is, and assumes that the caller has the issuer on hand, is that if you are verifying the validity of a certificate, you are, 99% of the time, going to have its issuer on hand. In theory there could be value in verifying OCSP responses that contain the fully chain from OCSP signer to certificate issuer for cases where you do not have the issuer already, but I'm not sure there are any real world cases where this is the case (at least ones I'd want to explicitly support).

For private CAs

These are interesting examples, and given there are implementations in the wild we probably need to consider supporting them, but I'm not entirely clear what value is being added here.

From the Vault perspective, what is the rationale for including the issuing certificate (which isn't delegated) to the response? Is it expected that the relying party won't already have it? It seems like this unnecessarily inflates the size of the response.

1. Update the docs to more adequately warn about specifying a nil issuer here.

Seems reasonable to say this should only be done if the response was provided over an otherwise trusted channel. Even with that said though we probably also should be explicit that if issuer == nil you need to check that the response was signed by a certificate issued by the certificate that signed the response (I wish we'd never included signature verification in parsing, this API is cursed).

2. Update the API to correctly return all certs fields, allowing callers to perform more advanced chain building with a nil issuer parameter if they desire.

Not sure I'm convinced this is something we really want to support. Just exposing all of the certificates sent isn't particularly complicated, but it encourages users to use OCSP in a way that isn't really relevant to web PKI (which is the subset of PKIX that we target, in an attempt to prune away the overwhelming complexity that is PKIX), and that I don't think we have any particular interest in providing broader support for elsewhere (i.e. in the standard library).

3. Fix the library to not err if issuer == certs[0] (i.e., if issuer != certs[0], do the signature check that exists today).

Seems reasonable to handle this redundancy, since it does appear in the wild.

[cipherboy]

@rolandshoemaker writes:

That said RFC 6960 Section 4.2.2.2. requires that if a delegated signing certificate is used it should be issued by the certificate that signed the certificate being checked. In the delegation case certs then only really needs to contain a single certificate, but the RFC leaves the door open to put whatever else you want in there.

I don't think this is quite a valid read, though I agree in practice.

Notably, that paragraph in RFC 6960 goes on to write:

The CA SHOULD use the same issuing key to issue a delegation certificate as that used to sign the certificate being checked for revocation.

and further, the extended note is of relevance:

Note: For backwards compatibility with RFC 2560, it is not
prohibited to issue a certificate for an Authorized
using a different issuing key than the key used to issue the
certificate being checked for revocation. However, such a
practice is strongly discouraged, since clients are not
required to recognize a responder with such a certificate as an
Authorized Responder.

Which is to say, this is likely another case where the CA the Entity (with a capital E) is different than "the concrete certificate with the IsCA basis constraint assert) and something legacy did happen, I agree.

But, more to the point, nothing guarantees the first item is also the responder.

---

Finishing the investigation, I think EJBCA has the same issue:

• responseChain is built from fullCertificateChain.
• It builds the OCSP response with this full chain -- either of the signing CA or with the delegated signer AND the signing CA's chain.
• BouncyCastle will happily include that chain in the ASN.1 response.

Following that BouncyCastle lead:

• Maritime Connectivity PKI seems to have the same behavior as Vault, assuming it is configured to directly use the ICA and not a delegated signer (unverified whether this is possible or not).
• If my read of otoroshi is correct this directly uses the root CA and again places it in the certs field.

So yeah, we can fix this on the Vault side, but this is a lot of additional private CAs--including EJBCA--that have this same behavior.

Go is broken validating all of them.

[rolandshoemaker]

Yeah, I think it'd probably also be reasonable to just iterate across all of the certificates in certs to find one that validates the signature, rather than always picking the first one 🤷.

[cipherboy]

Thanks Roland! I'll pick this up on Monday with a patch set then. - A

[gopherbot]

Change https://go.dev/cl/485055 mentions this issue: ocsp: better validate OCSP response's certificates