From 249c472b5b294a29924245d817d743a504936589 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Mon, 17 Apr 2023 12:40:26 -0400 Subject: [PATCH] Remove extraneous certificate from OCSP response (#20201) * Remove extraneous certificate from OCSP response 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 * Add changelog entry Signed-off-by: Alexander Scheel --------- Signed-off-by: Alexander Scheel --- builtin/logical/pki/path_ocsp.go | 8 +++++++- builtin/logical/pki/path_ocsp_test.go | 6 ------ changelog/20201.txt | 3 +++ 3 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 changelog/20201.txt diff --git a/builtin/logical/pki/path_ocsp.go b/builtin/logical/pki/path_ocsp.go index 42d4cf4b95..b9f5cd1f9f 100644 --- a/builtin/logical/pki/path_ocsp.go +++ b/builtin/logical/pki/path_ocsp.go @@ -499,13 +499,19 @@ func genResponse(cfg *crlConfig, caBundle *certutil.ParsedCertBundle, info *ocsp revSigAlg = x509.SHA512WithRSA } + // Due to a bug in Go's ocsp.ParseResponse(...), we do not provision + // Certificate any more on the response to help Go based OCSP clients. + // This was technically unnecessary, as the Certificate given here + // both signed the OCSP response and issued the leaf cert, and so + // should already be trusted by the client. + // + // See also: https://github.com/golang/go/issues/59641 template := ocsp.Response{ IssuerHash: reqHash, Status: info.ocspStatus, SerialNumber: info.serialNumber, ThisUpdate: curTime, NextUpdate: curTime.Add(duration), - Certificate: caBundle.Certificate, ExtraExtensions: []pkix.Extension{}, SignatureAlgorithm: revSigAlg, } diff --git a/builtin/logical/pki/path_ocsp_test.go b/builtin/logical/pki/path_ocsp_test.go index 577e4f9840..1caa050e3d 100644 --- a/builtin/logical/pki/path_ocsp_test.go +++ b/builtin/logical/pki/path_ocsp_test.go @@ -365,7 +365,6 @@ func TestOcsp_MultipleMatchingIssuersOneWithoutSigningUsage(t *testing.T) { require.Equal(t, crypto.SHA1, ocspResp.IssuerHash) require.Equal(t, 0, ocspResp.RevocationReason) require.Equal(t, testEnv.leafCertIssuer1.SerialNumber, ocspResp.SerialNumber) - require.Equal(t, rotatedCert, ocspResp.Certificate) requireOcspSignatureAlgoForKey(t, rotatedCert.SignatureAlgorithm, ocspResp.SignatureAlgorithm) requireOcspResponseSignedBy(t, ocspResp, rotatedCert) @@ -442,7 +441,6 @@ func TestOcsp_HigherLevel(t *testing.T) { require.NoError(t, err, "parsing ocsp get response") require.Equal(t, ocsp.Revoked, ocspResp.Status) - require.Equal(t, issuerCert, ocspResp.Certificate) require.Equal(t, certToRevoke.SerialNumber, ocspResp.SerialNumber) // Test OCSP Get request for ocsp @@ -463,7 +461,6 @@ func TestOcsp_HigherLevel(t *testing.T) { require.NoError(t, err, "parsing ocsp get response") require.Equal(t, ocsp.Revoked, ocspResp.Status) - require.Equal(t, issuerCert, ocspResp.Certificate) require.Equal(t, certToRevoke.SerialNumber, ocspResp.SerialNumber) } @@ -527,7 +524,6 @@ func runOcspRequestTest(t *testing.T, requestType string, caKeyType string, caKe require.Equal(t, ocsp.Good, ocspResp.Status) require.Equal(t, requestHash, ocspResp.IssuerHash) - require.Equal(t, testEnv.issuer1, ocspResp.Certificate) require.Equal(t, 0, ocspResp.RevocationReason) require.Equal(t, testEnv.leafCertIssuer1.SerialNumber, ocspResp.SerialNumber) @@ -552,7 +548,6 @@ func runOcspRequestTest(t *testing.T, requestType string, caKeyType string, caKe require.Equal(t, ocsp.Revoked, ocspResp.Status) require.Equal(t, requestHash, ocspResp.IssuerHash) - require.Equal(t, testEnv.issuer1, ocspResp.Certificate) require.Equal(t, 0, ocspResp.RevocationReason) require.Equal(t, testEnv.leafCertIssuer1.SerialNumber, ocspResp.SerialNumber) @@ -572,7 +567,6 @@ func runOcspRequestTest(t *testing.T, requestType string, caKeyType string, caKe require.Equal(t, ocsp.Good, ocspResp.Status) require.Equal(t, requestHash, ocspResp.IssuerHash) - require.Equal(t, testEnv.issuer2, ocspResp.Certificate) require.Equal(t, 0, ocspResp.RevocationReason) require.Equal(t, testEnv.leafCertIssuer2.SerialNumber, ocspResp.SerialNumber) diff --git a/changelog/20201.txt b/changelog/20201.txt new file mode 100644 index 0000000000..d50c9bcb9d --- /dev/null +++ b/changelog/20201.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Decrease size and improve compatibility of OCSP responses by removing issuer certificate. +```