diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go index b53eab4ba..a3fc05c2b 100644 --- a/cmd/ocsp-updater/main.go +++ b/cmd/ocsp-updater/main.go @@ -153,7 +153,7 @@ func (updater *OCSPUpdater) findStaleOCSPResponses(oldestLastUpdatedTime time.Ti now := updater.clk.Now() maxAgeCutoff := now.Add(-updater.ocspStaleMaxAge) - certStatusFields := "cs.serial, cs.status, cs.revokedDate, cs.notAfter" + certStatusFields := "cs.serial, cs.status, cs.revokedDate, cs.notAfter, cs.revokedReason" if features.Enabled(features.StoreIssuerInfo) { certStatusFields += ", cs.issuerID" } diff --git a/cmd/ocsp-updater/main_test.go b/cmd/ocsp-updater/main_test.go index 251d4d78b..689030d56 100644 --- a/cmd/ocsp-updater/main_test.go +++ b/cmd/ocsp-updater/main_test.go @@ -195,9 +195,9 @@ func TestFindStaleOCSPResponses(t *testing.T) { test.AssertNotError(t, err, "Couldn't update ocspLastUpdated") earliest := fc.Now().Add(-time.Hour) - certs, err := updater.findStaleOCSPResponses(earliest, 10) - test.AssertNotError(t, err, "Couldn't find certificate") - test.AssertEquals(t, len(certs), 1) + statuses, err := updater.findStaleOCSPResponses(earliest, 10) + test.AssertNotError(t, err, "Couldn't find status") + test.AssertEquals(t, len(statuses), 1) status, err := sa.GetCertificateStatus(ctx, core.SerialToString(parsedCert.SerialNumber)) test.AssertNotError(t, err, "Couldn't get the core.Certificate from the database") @@ -207,9 +207,41 @@ func TestFindStaleOCSPResponses(t *testing.T) { err = updater.storeResponse(meta) test.AssertNotError(t, err, "Couldn't store OCSP response") - certs, err = updater.findStaleOCSPResponses(earliest, 10) + statuses, err = updater.findStaleOCSPResponses(earliest, 10) test.AssertNotError(t, err, "Failed to find stale responses") - test.AssertEquals(t, len(certs), 0) + test.AssertEquals(t, len(statuses), 0) +} + +func TestFindStaleOCSPResponsesRevokedReason(t *testing.T) { + updater, sa, dbMap, fc, cleanUp := setup(t) + defer cleanUp() + + reg := satest.CreateWorkingRegistration(t, sa) + parsedCert, err := core.LoadCert("test-cert.pem") + test.AssertNotError(t, err, "Couldn't read test certificate") + issued := fc.Now().UnixNano() + _, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{ + Der: parsedCert.Raw, + RegID: ®.ID, + Ocsp: nil, + Issued: &issued, + }) + test.AssertNotError(t, err, "Couldn't add test-cert.pem") + + // We need to set a fake "ocspLastUpdated" value for the cert we created + // in order to satisfy the "ocspStaleMaxAge" constraint. + fakeLastUpdate := fc.Now().Add(-time.Hour * 24 * 3) + _, err = dbMap.Exec( + "UPDATE certificateStatus SET ocspLastUpdated = ?, revokedReason = 1 WHERE serial = ?", + fakeLastUpdate, + core.SerialToString(parsedCert.SerialNumber)) + test.AssertNotError(t, err, "Couldn't update ocspLastUpdated") + + earliest := fc.Now().Add(-time.Hour) + statuses, err := updater.findStaleOCSPResponses(earliest, 10) + test.AssertNotError(t, err, "Couldn't find status") + test.AssertEquals(t, len(statuses), 1) + test.AssertEquals(t, int(statuses[0].RevokedReason), 1) } func TestFindStaleOCSPResponsesStaleMaxAge(t *testing.T) { @@ -257,10 +289,10 @@ func TestFindStaleOCSPResponsesStaleMaxAge(t *testing.T) { // certificates, parsedCertA. The second should be excluded by the // `ocspStaleMaxAge` cutoff. earliest := fc.Now().Add(-time.Hour) - certs, err := updater.findStaleOCSPResponses(earliest, 10) + statuses, err := updater.findStaleOCSPResponses(earliest, 10) test.AssertNotError(t, err, "Couldn't find stale responses") - test.AssertEquals(t, len(certs), 1) - test.AssertEquals(t, certs[0].Serial, core.SerialToString(parsedCertA.SerialNumber)) + test.AssertEquals(t, len(statuses), 1) + test.AssertEquals(t, statuses[0].Serial, core.SerialToString(parsedCertA.SerialNumber)) } func TestOldOCSPResponsesTick(t *testing.T) { diff --git a/test/integration/revocation_test.go b/test/integration/revocation_test.go index dd1b9b060..b5920985c 100644 --- a/test/integration/revocation_test.go +++ b/test/integration/revocation_test.go @@ -157,9 +157,11 @@ func TestRevokeWithKeyCompromise(t *testing.T) { res, err := authAndIssue(c, certKey, []string{random_domain()}) test.AssertNotError(t, err, "authAndIssue failed") + cert := res.certs[0] + err = c.RevokeCertificate( c.Account, - res.certs[0], + cert, c.Account.PrivateKey, ocsp.KeyCompromise, ) @@ -169,6 +171,11 @@ func TestRevokeWithKeyCompromise(t *testing.T) { _, err = c.NewAccount(certKey, false, true) test.AssertError(t, err, "NewAccount didn't fail with a blacklisted key") test.AssertEquals(t, err.Error(), `acme: error code 400 "urn:ietf:params:acme:error:badPublicKey": public key is forbidden`) + + // Check the OCSP response. It should be revoked with reason = 1 (keyCompromise) + response, err := ocsp_helper.ReqDER(cert.Raw, ocsp.Revoked) + test.AssertNotError(t, err, "requesting OCSP for revoked cert") + test.AssertEquals(t, response.RevocationReason, 1) } func TestBadKeyRevoker(t *testing.T) {