Skip to content

x/crypto/ocsp: "OCSP response contains bad number of certificates" error #21527

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

Closed
joeshaw opened this issue Aug 18, 2017 · 8 comments
Closed

Comments

@joeshaw
Copy link
Contributor

joeshaw commented Aug 18, 2017

Currently in the ocsp.ParseResponseForCert function is a block:

	if len(basicResp.Certificates) > 1 {
		return nil, ParseError("OCSP response contains bad number of certificates")
	}

I can't find any justification for this check (it dates back to @rsc's initial commit). RFC 6960 section 4.2.1 says,

   The value for response SHALL be the DER encoding of
   BasicOCSPResponse.

   BasicOCSPResponse       ::= SEQUENCE {
      tbsResponseData      ResponseData,
      signatureAlgorithm   AlgorithmIdentifier,
      signature            BIT STRING,
      certs            [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }

   The value for signature SHALL be computed on the hash of the DER
   encoding of ResponseData.  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.

This leads me to believe that more than one cert is valid here, and in fact the http://sureseries-ocsp.cybertrust.ne.jp/OcspServer responder sends certs chaining up to a root.

The Go OCSP implementation doesn't verify the response certificate to a root, it just checks the signature against the issuer. (Is this sufficient? OpenSSL seems to do more.) But it seems like it shouldn't be an error to receive more than one certificate; it could simply do the check it currently does against the first one in the list.

/cc @agl

@gopherbot gopherbot added this to the Unreleased milestone Aug 18, 2017
@joeshaw
Copy link
Contributor Author

joeshaw commented Aug 18, 2017

Sorry, misattributed the initial commit to Russ. He just moved it to a subrepo from the Go repo. Initial commit was 8286ee4 by Adam.

@kreichgauer
Copy link
Contributor

This leads me to believe that more than one cert is valid here

That is correct, the Go ocsp package is just a bit limited and doesn't have support for more complex responses. From https://godoc.org/golang.org/x/crypto/ocsp#ParseResponse:

ParseResponse parses an OCSP response in DER form. It only supports responses for a single certificate.

/cc @agl

@joeshaw
Copy link
Contributor Author

joeshaw commented Aug 18, 2017

From https://godoc.org/golang.org/x/crypto/ocsp#ParseResponse:

ParseResponse parses an OCSP response in DER form. It only supports responses for a single certificate.

I interpreted this to mean a single certificate sent in the request. Not a single OCSP response certificate.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/57510 mentions this issue: ocsp: remove error for > 1 certificate in response

@joeshaw
Copy link
Contributor Author

joeshaw commented May 1, 2018

@agl @FiloSottile I would love to get this in for 1.11 if the approach makes sense. It causes us to get "OCSP response contains bad number of certificates" for any certificates that use cybertrust.ne.jp's various OCSP responders. For example, https://www.superchoice.bet/

@joeshaw
Copy link
Contributor Author

joeshaw commented May 1, 2018

🤦‍♂️ duh, I forgot this was in x/crypto and doesn't follow the release process. Still, please take a look once the 1.11 freeze fury has died down!

@joeshaw
Copy link
Contributor Author

joeshaw commented May 2, 2018

It was mentioned in the CL, but for better visibility: At the time this was merged there were many OCSP responders that triggered this. 209 by my count: https://crt.sh/ocsp-responders?get=OCSP+response+contains+bad+number+of+certificates&sort=2&dir=v

@robstradling
Copy link
Contributor

Just FYI, crt.sh now has this fix, and all of the "bad number of certificates" errors have disappeared.

@golang golang locked and limited conversation to collaborators May 9, 2019
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
jasonwvh pushed a commit to jasonwvh/ocsp that referenced this issue Jul 13, 2022
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
desdeel2d0m added a commit to desdeel2d0m/crypto that referenced this issue Jul 1, 2024
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1).  This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.

Fixes golang/go#21527

Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants