Skip to content

Commit

Permalink
Remove extraneous certificate from OCSP response (#20201)
Browse files Browse the repository at this point in the history
* 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 <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
  • Loading branch information
cipherboy authored Apr 17, 2023
1 parent d518846 commit 249c472
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 7 deletions.
8 changes: 7 additions & 1 deletion builtin/logical/pki/path_ocsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
6 changes: 0 additions & 6 deletions builtin/logical/pki/path_ocsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions changelog/20201.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Decrease size and improve compatibility of OCSP responses by removing issuer certificate.
```

0 comments on commit 249c472

Please sign in to comment.