Skip to content
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

Backport of Add fix for Go x/crypto/ocsp failure case into release/1.13.x #20199

Conversation

hc-github-team-secure-vault-core
Copy link
Collaborator

Backport

This PR is auto-generated from #20181 to be assessed for backporting due to the inclusion of the label backport/1.13.x.

The below text is copied from the body of the original PR.


When calling ocsp.ParseRequest(req, issue) with a non-nil issuer on a ocsp request which unknowingly contains an entry in the BasicOCSPResponse's certs field, Go incorrectly assumes that the issuer is a direct parent of the first certificate in the certs field, discarding the rest.

As documented in the Go issue, this is not a valid assumption and thus causes OCSP verification to fail in Vault with an error like:

bad OCSP signature: crypto/rsa: verification error

which ultimately leads to a cert auth login error of:

no chain matching all constraints could be found for this login certificate

We address this by using the unsafe issuer=nil argument, taking on the task of validating the OCSP response's signature as best we can in the absence of full chain information on either side (both the trusted certificate whose OCSP response we're verifying and the lack of any additional certs the OCSP responder may have sent).

See also: golang/go#59641


Overview of commits

@hc-github-team-secure-vault-core hc-github-team-secure-vault-core force-pushed the backport/cipherboy-fix-ocsp-query-handling/precisely-major-mallard branch 2 times, most recently from 3cbdbc6 to 0495ac8 Compare April 17, 2023 15:48
@cipherboy cipherboy enabled auto-merge (squash) April 17, 2023 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants