x/crypto/ocsp: Verify ProducedAt, ThisUpdate, and NextUpdate against cert validity window

[twifkak]

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

$ go version
go version go1.15.8 linux/amd64

Does this issue reproduce with the latest release?

Almost certainly. I don't see anything in https://github.com/golang/crypto/blob/master/ocsp/ocsp.go that would change the situation.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/twifkak/.cache/go-build"
GOENV="/home/twifkak/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/twifkak/.go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/twifkak/.go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.15/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build079748278=/tmp/go-build -gno-record-gcc-switches"

What did you do?

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

ocsp.ParseResponse() does not validate that the response's ProducedAt is within the NotBefore/NotAfter range of the provided `cert.

ocsp.ParseResponseForCert(), if given an OCSP response with multiple SingleResponses, does not necessarily pick the one with the correct ProducedAt per the same logic.

What did you expect to see?

&ocsp.Response{Status:2, ...
panic: no response matching the supplied certificate

What did you see instead?

&ocsp.Response{Status:0, ...
&ocsp.Response{Status:0, ...

[seankhliao]

cc @agl

[twifkak]

It looks like the library ought to check ThisUpdate and NextUpdate against the cert's validity window, too. LMK if you want me to provide a minified test case for you like I did with ProducedAt.

[twifkak]

/cc @sleevi

[sleevi]

/cc @FiloSottile @rolandshoemaker

Note: I think historically, Go has taken a conservative approach, even though specific certificate profiles (e.g. Web PKI) may make more stringent requirements. I know @FiloSottile tries to find the right balance, but I did want to explicitly acknowledge he may decide to WontFix this and leave it to implementations to enforce :)

So, enforcing thisUpdate, nextUpdate, or producedAt is <= validity.notAfter should definitely not be done, because they are valid OCSP responses providing data for historic certificates ("archival purposes" in RFC 6960). Certain certificate profiles and application classes may totally want to prohibit such a scenario, but that's not something that generalizes to OCSP.

Enforcing thisUpdate is >= validity.notBefore is also something that should not be done. It's a statement of the responder's knowledge (as covered in https://tools.ietf.org/html/rfc6960#section-2.4 ), and it's conceivable they have PKI-specific context (e.g. it may be a reflection of an underlying credential type used by the CA to issue the actual certificate).

For the other bits, I think it may make sense specific to the application? We've not yet profiled these in the Web PKI, and the closest profile I'm aware of is the (mandatory-but-not-mandatory for Web PKI) RFC 5019, Section 2.2.4

[twifkak]

Good to know. For ParseResponse it seems OK to me if the solution is documentation to explain that callers may need to check other things for specific profiles. (And even better if it provides specific advice or links.)

For ParseResponseForCert, it means that callers looking to enforce a specific profile may have to reimplement that function, because the ocsp lib doesn't provide a way to iterate over the SingleResponses of a Response. Hypothetically, a Response may contain two SingleResponses for the same cert, one of which has invalid dates and one of which has valid dates. If this case is more than hypothetical, then one solution is a new function, ParseResponsesForCert that returns a []*Response of all matching SingleResponses.