Reland "Use GetRevocationStatus instead of GetCertificateStatus"

core/objects.go

@@ -83,6 +83,11 @@ var OCSPStatusToInt = map[OCSPStatus]int{
 	OCSPStatusRevoked: ocsp.Revoked,
 }
 
+const (
+	RevocationStatusGood    int64 = 0
+	RevocationStatusRevoked int64 = 1
+)
+
 // DNSPrefix is attached to DNS names in DNS challenges
 const DNSPrefix = "_acme-challenge"
 

mocks/sa.go

@@ -213,7 +213,7 @@ func (sa *StorageAuthorityReadOnly) GetCertificateStatus(_ context.Context, req
 
 // GetRevocationStatus is a mock
 func (sa *StorageAuthorityReadOnly) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {
-	return nil, nil
+	return nil, errors.New("no revocation status")
 }
 
 // SerialsForIncident is a mock

ra/ra.go

@@ -1683,12 +1683,12 @@ func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert
 // certificates that were previously revoked for a reason other than
 // keyCompromise, and which are now being updated to keyCompromise instead.
 func (ra *RegistrationAuthorityImpl) updateRevocationForKeyCompromise(ctx context.Context, serialString string, issuerID issuance.NameID) error {
-	status, err := ra.SA.GetCertificateStatus(ctx, &sapb.Serial{Serial: serialString})
+	status, err := ra.SA.GetRevocationStatus(ctx, &sapb.Serial{Serial: serialString})
 	if err != nil {
 		return berrors.NotFoundError("unable to confirm that serial %q was ever issued: %s", serialString, err)
 	}
 
-	if status.Status != string(core.OCSPStatusRevoked) {
+	if status.Status != core.RevocationStatusRevoked {
 		// Internal server error, because we shouldn't be in the function at all
 		// unless the cert was already revoked.
 		return fmt.Errorf("unable to re-revoke serial %q which is not currently revoked", serialString)

ra/ra_test.go

@@ -3511,20 +3511,20 @@ type mockSARevocation struct {
 	sapb.StorageAuthorityClient
 
 	known   map[string]*x509.Certificate
-	revoked map[string]*corepb.CertificateStatus
+	revoked map[string]*sapb.RevocationStatus
 	blocked []*sapb.AddBlockedKeyRequest
 }
 
 func newMockSARevocation(known *x509.Certificate) *mockSARevocation {
 	return &mockSARevocation{
 		known:   map[string]*x509.Certificate{core.SerialToString(known.SerialNumber): known},
-		revoked: make(map[string]*corepb.CertificateStatus),
+		revoked: make(map[string]*sapb.RevocationStatus),
 		blocked: make([]*sapb.AddBlockedKeyRequest, 0),
 	}
 }
 
 func (msar *mockSARevocation) reset() {
-	msar.revoked = make(map[string]*corepb.CertificateStatus)
+	msar.revoked = make(map[string]*sapb.RevocationStatus)
 	msar.blocked = make([]*sapb.AddBlockedKeyRequest, 0)
 }
 
@@ -3552,14 +3552,13 @@ func (msar *mockSARevocation) GetLintPrecertificate(_ context.Context, req *sapb
 	return nil, berrors.UnknownSerialError()
 }
 
-func (msar *mockSARevocation) GetCertificateStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.CertificateStatus, error) {
+func (msar *mockSARevocation) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {
 	if status, present := msar.revoked[req.Serial]; present {
 		return status, nil
 	}
-	if cert, present := msar.known[req.Serial]; present {
-		return &corepb.CertificateStatus{
-			Serial:   core.SerialToString(cert.SerialNumber),
-			IssuerID: int64(issuance.IssuerNameID(cert)),
+	if _, present := msar.known[req.Serial]; present {
+		return &sapb.RevocationStatus{
+			Status: core.RevocationStatusGood,
 		}, nil
 	}
 	return nil, berrors.UnknownSerialError()
@@ -3598,14 +3597,12 @@ func (msar *mockSARevocation) RevokeCertificate(_ context.Context, req *sapb.Rev
 	if _, present := msar.revoked[req.Serial]; present {
 		return nil, berrors.AlreadyRevokedError("already revoked")
 	}
-	cert, present := msar.known[req.Serial]
+	_, present := msar.known[req.Serial]
 	if !present {
 		return nil, berrors.UnknownSerialError()
 	}
-	msar.revoked[req.Serial] = &corepb.CertificateStatus{
-		Serial:        req.Serial,
-		IssuerID:      int64(issuance.IssuerNameID(cert)),
-		Status:        string(core.OCSPStatusRevoked),
+	msar.revoked[req.Serial] = &sapb.RevocationStatus{
+		Status:        core.RevocationStatusRevoked,
 		RevokedReason: req.Reason,
 	}
 	return &emptypb.Empty{}, nil
@@ -3772,7 +3769,7 @@ func TestRevokeCertByKey(t *testing.T) {
 
 	// Reset and have the Subscriber revoke for a different reason.
 	// Then re-revoking using the key should work.
-	mockSA.revoked = make(map[string]*corepb.CertificateStatus)
+	mockSA.revoked = make(map[string]*sapb.RevocationStatus)
 	_, err = ra.RevokeCertByApplicant(context.Background(), &rapb.RevokeCertByApplicantRequest{
 		Cert:  cert.Raw,
 		Code:  int64(revocation.Unspecified),

sa/db-next/boulder_sa/20251002000000_AddRevokedSerialsIndex.sql

@@ -0,0 +1 @@
+../../db/boulder_sa/20251002000000_AddRevokedSerialsIndex.sql

sa/db/boulder_sa/20251002000000_AddRevokedSerialsIndex.sql

@@ -0,0 +1,9 @@
+-- +migrate Up
+-- SQL in section 'Up' is executed when this migration is applied
+
+ALTER TABLE `revokedCertificates` ADD KEY `serial` (`serial`);
+
+-- +migrate Down
+-- SQL section 'Down' is executed when this migration is rolled back
+
+ALTER TABLE `revokedCertificates` DROP KEY `serial`;

sa/model.go

@@ -161,41 +161,6 @@ func SelectCertificateStatus(ctx context.Context, s db.OneSelector, serial strin
 	return model.toPb(), err
 }
 
-// RevocationStatusModel represents a small subset of the columns in the
-// certificateStatus table, used to determine the authoritative revocation
-// status of a certificate.
-type RevocationStatusModel struct {
-	Status        core.OCSPStatus   `db:"status"`
-	RevokedDate   time.Time         `db:"revokedDate"`
-	RevokedReason revocation.Reason `db:"revokedReason"`
-}
-
-// SelectRevocationStatus returns the authoritative revocation information for
-// the certificate with the given serial.
-func SelectRevocationStatus(ctx context.Context, s db.OneSelector, serial string) (*sapb.RevocationStatus, error) {
-	var model RevocationStatusModel
-	err := s.SelectOne(
-		ctx,
-		&model,
-		"SELECT status, revokedDate, revokedReason FROM certificateStatus WHERE serial = ? LIMIT 1",
-		serial,
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	statusInt, ok := core.OCSPStatusToInt[model.Status]
-	if !ok {
-		return nil, fmt.Errorf("got unrecognized status %q", model.Status)
-	}
-
-	return &sapb.RevocationStatus{
-		Status:        int64(statusInt),
-		RevokedDate:   timestamppb.New(model.RevokedDate),
-		RevokedReason: int64(model.RevokedReason),
-	}, nil
-}
-
 var mediumBlobSize = int(math.Pow(2, 24))
 
 type issuedNameModel struct {

sa/sa_test.go

@@ -396,15 +396,19 @@ func TestAddPrecertificate(t *testing.T) {
 	defer cleanUp()
 
 	reg := createWorkingRegistration(t, sa)
+	regID := reg.Id
 
-	// Create a throw-away self signed certificate with a random name and
-	// serial number
+	// Add a cert to the DB to test with.
 	serial, testCert := test.ThrowAwayCert(t, clk)
-
-	// Add the cert as a precertificate
-	regID := reg.Id
+	_, err := sa.AddSerial(ctx, &sapb.AddSerialRequest{
+		RegID:   regID,
+		Serial:  serial,
+		Created: timestamppb.New(testCert.NotBefore),
+		Expires: timestamppb.New(testCert.NotAfter),
+	})
+	test.AssertNotError(t, err, "failed to add test serial")
 	issuedTime := mustTimestamp("2018-04-01 07:00")
-	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
+	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
 		Der:          testCert.Raw,
 		RegID:        regID,
 		Issued:       issuedTime,
@@ -413,11 +417,9 @@ func TestAddPrecertificate(t *testing.T) {
 	test.AssertNotError(t, err, "Couldn't add test cert")
 
 	// It should have the expected certificate status
-	certStatus, err := sa.GetCertificateStatus(ctx, &sapb.Serial{Serial: serial})
+	certStatus, err := sa.GetRevocationStatus(ctx, &sapb.Serial{Serial: serial})
 	test.AssertNotError(t, err, "Couldn't get status for test cert")
-	test.AssertEquals(t, certStatus.Status, string(core.OCSPStatusGood))
-	now := clk.Now()
-	test.AssertEquals(t, now, certStatus.OcspLastUpdated.AsTime())
+	test.AssertEquals(t, certStatus.Status, core.RevocationStatusGood)
 
 	// It should show up in the issued names table
 	issuedNamesSerial, err := findIssuedName(ctx, sa.dbMap, reverseFQDN(testCert.DNSNames[0]))
@@ -1787,10 +1789,9 @@ func TestRevokeCertificate(t *testing.T) {
 	sa, fc, cleanUp := initSA(t)
 	defer cleanUp()
 
-	reg := createWorkingRegistration(t, sa)
 	// Add a cert to the DB to test with.
+	reg := createWorkingRegistration(t, sa)
 	serial, testCert := test.ThrowAwayCert(t, fc)
-	issuedTime := sa.clk.Now()
 	_, err := sa.AddSerial(ctx, &sapb.AddSerialRequest{
 		RegID:   reg.Id,
 		Serial:  core.SerialToString(testCert.SerialNumber),
@@ -1801,14 +1802,14 @@ func TestRevokeCertificate(t *testing.T) {
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
 		Der:          testCert.Raw,
 		RegID:        reg.Id,
-		Issued:       timestamppb.New(issuedTime),
+		Issued:       timestamppb.New(testCert.NotBefore),
 		IssuerNameID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test cert")
 
-	status, err := sa.GetCertificateStatus(ctx, &sapb.Serial{Serial: serial})
+	status, err := sa.GetRevocationStatus(ctx, &sapb.Serial{Serial: serial})
 	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusGood)
+	test.AssertEquals(t, status.Status, core.RevocationStatusGood)
 
 	fc.Add(1 * time.Hour)
 
@@ -1824,12 +1825,11 @@ func TestRevokeCertificate(t *testing.T) {
 	})
 	test.AssertNotError(t, err, "RevokeCertificate with no OCSP response should succeed")
 
-	status, err = sa.GetCertificateStatus(ctx, &sapb.Serial{Serial: serial})
+	status, err = sa.GetRevocationStatus(ctx, &sapb.Serial{Serial: serial})
 	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusRevoked)
+	test.AssertEquals(t, status.Status, core.RevocationStatusRevoked)
 	test.AssertEquals(t, status.RevokedReason, reason)
 	test.AssertEquals(t, status.RevokedDate.AsTime(), now)
-	test.AssertEquals(t, status.OcspLastUpdated.AsTime(), now)
 
 	_, err = sa.RevokeCertificate(context.Background(), &sapb.RevokeCertificateRequest{
 		IssuerID: 1,
@@ -1840,67 +1840,13 @@ func TestRevokeCertificate(t *testing.T) {
 	test.AssertError(t, err, "RevokeCertificate should've failed when certificate already revoked")
 }
 
-func TestRevokeCertificateWithShard(t *testing.T) {
-	sa, fc, cleanUp := initSA(t)
-	defer cleanUp()
-
-	// Add a cert to the DB to test with.
-	reg := createWorkingRegistration(t, sa)
-	eeCert, err := core.LoadCert("../test/hierarchy/ee-e1.cert.pem")
-	test.AssertNotError(t, err, "failed to load test cert")
-	_, err = sa.AddSerial(ctx, &sapb.AddSerialRequest{
-		RegID:   reg.Id,
-		Serial:  core.SerialToString(eeCert.SerialNumber),
-		Created: timestamppb.New(eeCert.NotBefore),
-		Expires: timestamppb.New(eeCert.NotAfter),
-	})
-	test.AssertNotError(t, err, "failed to add test serial")
-	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:          eeCert.Raw,
-		RegID:        reg.Id,
-		Issued:       timestamppb.New(eeCert.NotBefore),
-		IssuerNameID: 1,
-	})
-	test.AssertNotError(t, err, "failed to add test cert")
-
-	serial := core.SerialToString(eeCert.SerialNumber)
-	fc.Add(1 * time.Hour)
-	now := fc.Now()
-	reason := int64(1)
-
-	_, err = sa.RevokeCertificate(context.Background(), &sapb.RevokeCertificateRequest{
-		IssuerID: 1,
-		ShardIdx: 9,
-		Serial:   serial,
-		Date:     timestamppb.New(now),
-		Reason:   reason,
-	})
-	test.AssertNotError(t, err, "RevokeCertificate with no OCSP response should succeed")
-
-	status, err := sa.GetCertificateStatus(ctx, &sapb.Serial{Serial: serial})
-	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusRevoked)
-	test.AssertEquals(t, status.RevokedReason, reason)
-	test.AssertEquals(t, status.RevokedDate.AsTime(), now)
-	test.AssertEquals(t, status.OcspLastUpdated.AsTime(), now)
-	test.AssertEquals(t, status.NotAfter.AsTime(), eeCert.NotAfter)
-
-	var result revokedCertModel
-	err = sa.dbMap.SelectOne(
-		ctx, &result, `SELECT * FROM revokedCertificates WHERE serial = ?`, core.SerialToString(eeCert.SerialNumber))
-	test.AssertNotError(t, err, "should be exactly one row in revokedCertificates")
-	test.AssertEquals(t, result.ShardIdx, int64(9))
-	test.AssertEquals(t, result.RevokedReason, revocation.KeyCompromise)
-}
-
 func TestUpdateRevokedCertificate(t *testing.T) {
 	sa, fc, cleanUp := initSA(t)
 	defer cleanUp()
 
 	// Add a cert to the DB to test with.
 	reg := createWorkingRegistration(t, sa)
 	serial, testCert := test.ThrowAwayCert(t, fc)
-	issuedTime := fc.Now()
 	_, err := sa.AddSerial(ctx, &sapb.AddSerialRequest{
 		RegID:   reg.Id,
 		Serial:  core.SerialToString(testCert.SerialNumber),
@@ -1911,7 +1857,7 @@ func TestUpdateRevokedCertificate(t *testing.T) {
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
 		Der:          testCert.Raw,
 		RegID:        reg.Id,
-		Issued:       timestamppb.New(issuedTime),
+		Issued:       timestamppb.New(testCert.NotBefore),
 		IssuerNameID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test cert")
@@ -1944,10 +1890,10 @@ func TestUpdateRevokedCertificate(t *testing.T) {
 	test.AssertNotError(t, err, "RevokeCertificate failed")
 
 	// Double check that setup worked.
-	status, err := sa.GetCertificateStatus(ctx, &sapb.Serial{Serial: serial})
+	status, err := sa.GetRevocationStatus(ctx, &sapb.Serial{Serial: serial})
 	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusRevoked)
-	test.AssertEquals(t, revocation.Reason(status.RevokedReason), revocation.CessationOfOperation)
+	test.AssertEquals(t, status.Status, core.RevocationStatusRevoked)
+	test.AssertEquals(t, status.RevokedReason, int64(revocation.CessationOfOperation))
 	fc.Add(1 * time.Hour)
 
 	// Try to update its revocation info with no backdate
@@ -3000,10 +2946,10 @@ func TestGetRevokedCerts(t *testing.T) {
 	test.AssertNotError(t, err, "failed to add test cert")
 
 	// Check that it worked.
-	status, err := sa.GetCertificateStatus(
+	status, err := sa.GetRevocationStatus(
 		ctx, &sapb.Serial{Serial: core.SerialToString(eeCert.SerialNumber)})
 	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusGood)
+	test.AssertEquals(t, status.Status, core.RevocationStatusGood)
 
 	// Here's a little helper func we'll use to call GetRevokedCerts and count
 	// how many results it returned.
@@ -3107,10 +3053,10 @@ func TestGetRevokedCertsByShard(t *testing.T) {
 	test.AssertNotError(t, err, "failed to add test cert")
 
 	// Check that it worked.
-	status, err := sa.GetCertificateStatus(
+	status, err := sa.GetRevocationStatus(
 		ctx, &sapb.Serial{Serial: core.SerialToString(eeCert.SerialNumber)})
 	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusGood)
+	test.AssertEquals(t, status.Status, core.RevocationStatusGood)
 
 	// Here's a little helper func we'll use to call GetRevokedCertsByShard and count
 	// how many results it returned.