From 4190212bbbf843e69ac4e48ce106ff4639b94441 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 42d4cf4b9..b9f5cd1f9 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 577e4f984..1caa050e3 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 000000000..d50c9bcb9 --- /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. +```