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

Remove extraneous certificate from OCSP response #20201

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Apr 17, 2023

Since the issuer used to sign the certificate also signs the OCSP response, no additional information is added by sending the issuer again in the certs field of the BasicOCSPResponse structure. Removing it saves bytes and avoids confusing Go-based OCSP verifiers which cannot handle the cert issuer being duplicated in the certs field.


This complements #20181; if anyone uses a different cluster for Vault PKI from Cert Auth and are running Vault 1.12.x, they could update that cluster to a future 1.12 Vault release, instead of (or in addition to) updating the cluster running Cert Auth to a newer 1.13 version. This additionally improves compatibility with any other Go OCSP validation that may be occurring (e.g., Snowflake or others as reported on the upstream Go issue).

As another justification, note that when validating under cross-signed pairs, if we provision this certificate without including both cross-signed pairs, we're essentially gambling as to which one the (OCSP) client is validating under since they're otherwise equivalent.

Since the issuer used to sign the certificate also signs the OCSP
response, no additional information is added by sending the issuer again
in the certs field of the BasicOCSPResponse structure. Removing it saves
bytes and avoids confusing Go-based OCSP verifiers which cannot handle
the cert issuer being duplicated in the certs field.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants