Delete sa.GetRevokedCerts

Author: aarongable

docs/CRLS.md

@@ -39,8 +39,7 @@ contains an entry for every certificate, explicitly recording that newly-issued
 certificates are not revoked. The latter is less explicit but more scalable,
 containing rows only for certificates which have been revoked.
 
-The SA exposes the two different types of recordkeeping in two different ways:
-`GetRevokedCerts` returns revoked certificates whose NotAfter dates fall within
-a requested range. `GetRevokedCertsByShard` returns revoked certificates whose
-`shardIdx` matches the requested shard. The crl-updater uses only the latter
-method, and the former will be removed in the future.
+The SA only exposes the latter of these two mechanisms via the
+`GetRevokedCertsByShard` method, which returns revoked certificates whose
+`shardIdx` matches the requested shard. The `certificateStatus` table will be
+removed in the near future.

mocks/sa.go

@@ -252,16 +252,6 @@ func (sa *StorageAuthority) GetPausedIdentifiers(_ context.Context, _ *sapb.Regi
 	return nil, nil
 }
 
-// GetRevokedCerts is a mock
-func (sa *StorageAuthorityReadOnly) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetRevokedCertsClient, error) {
-	return &ServerStreamClient[corepb.CRLEntry]{}, nil
-}
-
-// GetRevokedCerts is a mock
-func (sa *StorageAuthority) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthority_GetRevokedCertsClient, error) {
-	return &ServerStreamClient[corepb.CRLEntry]{}, nil
-}
-
 // GetRevokedCertsByShard is a mock
 func (sa *StorageAuthorityReadOnly) GetRevokedCertsByShard(ctx context.Context, _ *sapb.GetRevokedCertsByShardRequest, _ ...grpc.CallOption) (grpc.ServerStreamingClient[corepb.CRLEntry], error) {
 	return &ServerStreamClient[corepb.CRLEntry]{}, nil

sa/sa_test.go

@@ -2974,113 +2974,6 @@ func TestSerialsForIncident(t *testing.T) {
 	test.AssertNotError(t, err, "Error getting serials for incident")
 }
 
-func TestGetRevokedCerts(t *testing.T) {
-	sa, _, cleanUp := initSA(t)
-	defer cleanUp()
-
-	// Add a cert to the DB to test with. We use AddPrecertificate because it sets
-	// up the certificateStatus row we need. This particular cert has a notAfter
-	// date of Mar 6 2023, and we lie about its IssuerNameID to make things easy.
-	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")
-
-	// Check that it worked.
-	status, err := sa.GetCertificateStatus(
-		ctx, &sapb.Serial{Serial: core.SerialToString(eeCert.SerialNumber)})
-	test.AssertNotError(t, err, "GetCertificateStatus failed")
-	test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusGood)
-
-	// Here's a little helper func we'll use to call GetRevokedCerts and count
-	// how many results it returned.
-	countRevokedCerts := func(req *sapb.GetRevokedCertsRequest) (int, error) {
-		stream := make(chan *corepb.CRLEntry)
-		mockServerStream := &fakeServerStream[corepb.CRLEntry]{output: stream}
-		var err error
-		go func() {
-			err = sa.GetRevokedCerts(req, mockServerStream)
-			close(stream)
-		}()
-		entriesReceived := 0
-		for range stream {
-			entriesReceived++
-		}
-		return entriesReceived, err
-	}
-
-	// The basic request covers a time range that should include this certificate.
-	basicRequest := &sapb.GetRevokedCertsRequest{
-		IssuerNameID:  1,
-		ExpiresAfter:  mustTimestamp("2023-03-01 00:00"),
-		ExpiresBefore: mustTimestamp("2023-04-01 00:00"),
-		RevokedBefore: mustTimestamp("2023-04-01 00:00"),
-	}
-	count, err := countRevokedCerts(basicRequest)
-	test.AssertNotError(t, err, "zero rows shouldn't result in error")
-	test.AssertEquals(t, count, 0)
-
-	// Revoke the certificate.
-	_, err = sa.RevokeCertificate(context.Background(), &sapb.RevokeCertificateRequest{
-		IssuerID: 1,
-		Serial:   core.SerialToString(eeCert.SerialNumber),
-		Date:     mustTimestamp("2023-01-01 00:00"),
-		Reason:   1,
-		Response: []byte{1, 2, 3},
-		ShardIdx: 1,
-	})
-	test.AssertNotError(t, err, "failed to revoke test cert")
-
-	// Asking for revoked certs now should return one result.
-	count, err = countRevokedCerts(basicRequest)
-	test.AssertNotError(t, err, "normal usage shouldn't result in error")
-	test.AssertEquals(t, count, 1)
-
-	// Asking for revoked certs with an old RevokedBefore should return no results.
-	count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
-		IssuerNameID:  1,
-		ExpiresAfter:  basicRequest.ExpiresAfter,
-		ExpiresBefore: basicRequest.ExpiresBefore,
-		RevokedBefore: mustTimestamp("2020-03-01 00:00"),
-	})
-	test.AssertNotError(t, err, "zero rows shouldn't result in error")
-	test.AssertEquals(t, count, 0)
-
-	// Asking for revoked certs in a time period that does not cover this cert's
-	// notAfter timestamp should return zero results.
-	count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
-		IssuerNameID:  1,
-		ExpiresAfter:  mustTimestamp("2022-03-01 00:00"),
-		ExpiresBefore: mustTimestamp("2022-04-01 00:00"),
-		RevokedBefore: mustTimestamp("2023-04-01 00:00"),
-	})
-	test.AssertNotError(t, err, "zero rows shouldn't result in error")
-	test.AssertEquals(t, count, 0)
-
-	// Asking for revoked certs from a different issuer should return zero results.
-	count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
-		IssuerNameID:  5678,
-		ExpiresAfter:  basicRequest.ExpiresAfter,
-		ExpiresBefore: basicRequest.ExpiresBefore,
-		RevokedBefore: basicRequest.RevokedBefore,
-	})
-	test.AssertNotError(t, err, "zero rows shouldn't result in error")
-	test.AssertEquals(t, count, 0)
-}
-
 func TestGetRevokedCertsByShard(t *testing.T) {
 	sa, _, cleanUp := initSA(t)
 	defer cleanUp()

sa/saro.go

@@ -972,59 +972,6 @@ func (ssa *SQLStorageAuthorityRO) GetRevokedCertsByShard(req *sapb.GetRevokedCer
 	})
 }
 
-// GetRevokedCerts returns revoked certificates based on temporal sharding.
-//
-// Based on a request specifying an issuer and a period of time,
-// it writes to the output stream the set of all certificates issued by that
-// issuer which expire during that period of time and which have been revoked.
-// The starting timestamp is treated as inclusive (certs with exactly that
-// notAfter date are included), but the ending timestamp is exclusive (certs
-// with exactly that notAfter date are *not* included).
-func (ssa *SQLStorageAuthorityRO) GetRevokedCerts(req *sapb.GetRevokedCertsRequest, stream grpc.ServerStreamingServer[corepb.CRLEntry]) error {
-	if core.IsAnyNilOrZero(req.IssuerNameID, req.RevokedBefore, req.ExpiresAfter, req.ExpiresBefore) {
-		return errIncompleteRequest
-	}
-	atTime := req.RevokedBefore.AsTime()
-
-	clauses := `
-		WHERE notAfter >= ?
-		AND notAfter < ?
-		AND issuerID = ?
-		AND status = ?`
-	params := []any{
-		req.ExpiresAfter.AsTime(),
-		req.ExpiresBefore.AsTime(),
-		req.IssuerNameID,
-		core.OCSPStatusRevoked,
-	}
-
-	selector, err := db.NewMappedSelector[crlEntryModel](ssa.dbReadOnlyMap)
-	if err != nil {
-		return fmt.Errorf("initializing db map: %w", err)
-	}
-
-	rows, err := selector.QueryContext(stream.Context(), clauses, params...)
-	if err != nil {
-		return fmt.Errorf("reading db: %w", err)
-	}
-
-	return rows.ForEach(func(row *crlEntryModel) error {
-		// Double-check that the cert wasn't revoked between the time at which we're
-		// constructing this snapshot CRL and right now. If the cert was revoked
-		// at-or-after the "atTime", we'll just include it in the next generation
-		// of CRLs.
-		if row.RevokedDate.After(atTime) || row.RevokedDate.Equal(atTime) {
-			return nil
-		}
-
-		return stream.Send(&corepb.CRLEntry{
-			Serial:    row.Serial,
-			Reason:    int32(row.RevokedReason), //nolint: gosec // Revocation reasons are guaranteed to be small, no risk of overflow.
-			RevokedAt: timestamppb.New(row.RevokedDate),
-		})
-	})
-}
-
 // Health implements the grpc.checker interface.
 func (ssa *SQLStorageAuthorityRO) Health(ctx context.Context) error {
 	err := ssa.dbReadOnlyMap.SelectOne(ctx, new(int), "SELECT 1")