Skip to content

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

Open
golang/crypto
#256
@cipherboy

Description

@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

Metadata

Metadata

Assignees

No one assigned

    Labels

    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions