sa: use internal certificateModel (#8130)

This follows the system we've used for other types, where the SA has a
model type that is converted to a proto message for use outside the SA.

Part of #8112.

cmd/cert-checker/main.go

@@ -100,7 +100,7 @@ type certChecker struct {
 	kp                          goodkey.KeyPolicy
 	dbMap                       certDB
 	getPrecert                  precertGetter
-	certs                       chan core.Certificate
+	certs                       chan *corepb.Certificate
 	clock                       clock.Clock
 	rMu                         *sync.Mutex
 	issuedReport                report
@@ -124,14 +124,14 @@ func newChecker(saDbMap certDB,
 		if err != nil {
 			return nil, err
 		}
-		return precertPb.DER, nil
+		return precertPb.Der, nil
 	}
 	return certChecker{
 		pa:                          pa,
 		kp:                          kp,
 		dbMap:                       saDbMap,
 		getPrecert:                  precertGetter,
-		certs:                       make(chan core.Certificate, batchSize),
+		certs:                       make(chan *corepb.Certificate, batchSize),
 		rMu:                         new(sync.Mutex),
 		clock:                       clk,
 		issuedReport:                report{Entries: make(map[string]reportEntry)},
@@ -214,7 +214,7 @@ func (c *certChecker) getCerts(ctx context.Context) error {
 	batchStartID := initialID
 	var retries int
 	for {
-		certs, err := sa.SelectCertificates(
+		certs, highestID, err := sa.SelectCertificates(
 			ctx,
 			c.dbMap,
 			`WHERE id > :id AND
@@ -239,16 +239,16 @@ func (c *certChecker) getCerts(ctx context.Context) error {
 		}
 		retries = 0
 		for _, cert := range certs {
-			c.certs <- cert.Certificate
+			c.certs <- cert
 		}
 		if len(certs) == 0 {
 			break
 		}
 		lastCert := certs[len(certs)-1]
-		batchStartID = lastCert.ID
+		if lastCert.Issued.AsTime().After(c.issuedReport.end) {
-		if lastCert.Issued.After(c.issuedReport.end) {
 			break
 		}
+		batchStartID = highestID
 	}
 
 	// Close channel so range operations won't block once the channel empties out
@@ -302,8 +302,8 @@ var expectedExtensionContent = map[string][]byte{
 // likely valid at the time the certificate was issued. Authorizations with
 // status = "deactivated" are counted for this, so long as their validatedAt
 // is before the issuance and expiration is after.
-func (c *certChecker) checkValidations(ctx context.Context, cert core.Certificate, idents identifier.ACMEIdentifiers) error {
+func (c *certChecker) checkValidations(ctx context.Context, cert *corepb.Certificate, idents identifier.ACMEIdentifiers) error {
-	authzs, err := sa.SelectAuthzsMatchingIssuance(ctx, c.dbMap, cert.RegistrationID, cert.Issued, idents)
+	authzs, err := sa.SelectAuthzsMatchingIssuance(ctx, c.dbMap, cert.RegistrationID, cert.Issued.AsTime(), idents)
 	if err != nil {
 		return fmt.Errorf("error checking authzs for certificate %s: %w", cert.Serial, err)
 	}
@@ -334,16 +334,16 @@ func (c *certChecker) checkValidations(ctx context.Context, cert core.Certificat
 }
 
 // checkCert returns a list of DNS names in the certificate and a list of problems with the certificate.
-func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]string, []string) {
+func (c *certChecker) checkCert(ctx context.Context, cert *corepb.Certificate) ([]string, []string) {
 	var dnsNames []string
 	var problems []string
 
 	// Check that the digests match.
-	if cert.Digest != core.Fingerprint256(cert.DER) {
+	if cert.Digest != core.Fingerprint256(cert.Der) {
 		problems = append(problems, "Stored digest doesn't match certificate digest")
 	}
 	// Parse the certificate.
-	parsedCert, err := zX509.ParseCertificate(cert.DER)
+	parsedCert, err := zX509.ParseCertificate(cert.Der)
 	if err != nil {
 		problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err))
 	} else {
@@ -368,7 +368,7 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
 			problems = append(problems, "Stored serial doesn't match certificate serial")
 		}
 		// Check that we have the correct expiration time.
-		if !parsedCert.NotAfter.Equal(cert.Expires) {
+		if !parsedCert.NotAfter.Equal(cert.Expires.AsTime()) {
 			problems = append(problems, "Stored expiration doesn't match certificate NotAfter")
 		}
 		// Check if basic constraints are set.
@@ -388,7 +388,7 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
 			problems = append(problems, "Certificate has unacceptable validity period")
 		}
 		// Check that the stored issuance time isn't too far back/forward dated.
-		if parsedCert.NotBefore.Before(cert.Issued.Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.Add(6*time.Hour)) {
+		if parsedCert.NotBefore.Before(cert.Issued.AsTime().Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.AsTime().Add(6*time.Hour)) {
 			problems = append(problems, "Stored issuance date is outside of 6 hour window of certificate NotBefore")
 		}
 		if parsedCert.Subject.CommonName != "" {
@@ -451,7 +451,7 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
 		// checks which rely on external resources such as weak or blocked key
 		// lists, or the list of blocked keys in the database. This only performs
 		// static checks, such as against the RSA key size and the ECDSA curve.
-		p, err := x509.ParseCertificate(cert.DER)
+		p, err := x509.ParseCertificate(cert.Der)
 		if err != nil {
 			problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err))
 		}
@@ -467,7 +467,7 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
 			c.logger.Errf("fetching linting precertificate for %s: %s", cert.Serial, err)
 			atomic.AddInt64(&c.issuedReport.DbErrs, 1)
 		} else {
-			err = precert.Correspond(precertDER, cert.DER)
+			err = precert.Correspond(precertDER, cert.Der)
 			if err != nil {
 				problems = append(problems,
 					fmt.Sprintf("Certificate does not correspond to precert for %s: %s", cert.Serial, err))

cmd/cert-checker/main_test.go

@@ -27,6 +27,7 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/letsencrypt/boulder/core"
+	corepb "github.com/letsencrypt/boulder/core/proto"
 	"github.com/letsencrypt/boulder/ctpolicy/loglist"
 	"github.com/letsencrypt/boulder/goodkey"
 	"github.com/letsencrypt/boulder/goodkey/sagoodkey"
@@ -79,12 +80,12 @@ func BenchmarkCheckCert(b *testing.B) {
 		SerialNumber: serial,
 	}
 	certDer, _ := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey)
-	cert := core.Certificate{
+	cert := &corepb.Certificate{
 		Serial:  core.SerialToString(serial),
 		Digest:  core.Fingerprint256(certDer),
-		DER:     certDer,
+		Der:     certDer,
-		Issued:  time.Now(),
+		Issued:  timestamppb.New(time.Now()),
-		Expires: expiry,
+		Expires: timestamppb.New(expiry),
 	}
 	b.ResetTimer()
 	for range b.N {
@@ -125,12 +126,12 @@ func TestCheckWildcardCert(t *testing.T) {
 	test.AssertNotError(t, err, "Couldn't create certificate")
 	parsed, err := x509.ParseCertificate(wildcardCertDer)
 	test.AssertNotError(t, err, "Couldn't parse created certificate")
-	cert := core.Certificate{
+	cert := &corepb.Certificate{
 		Serial:  core.SerialToString(serial),
 		Digest:  core.Fingerprint256(wildcardCertDer),
-		Expires: parsed.NotAfter,
+		Expires: timestamppb.New(parsed.NotAfter),
-		Issued:  parsed.NotBefore,
+		Issued:  timestamppb.New(parsed.NotBefore),
-		DER:     wildcardCertDer,
+		Der:     wildcardCertDer,
 	}
 	_, problems := checker.checkCert(context.Background(), cert)
 	for _, p := range problems {
@@ -157,12 +158,12 @@ func TestCheckCertReturnsDNSNames(t *testing.T) {
 		t.Fatal("failed to parse cert PEM")
 	}
 
-	cert := core.Certificate{
+	cert := &corepb.Certificate{
 		Serial:  "00000000000",
 		Digest:  core.Fingerprint256(block.Bytes),
-		Expires: time.Now().Add(time.Hour),
+		Expires: timestamppb.New(time.Now().Add(time.Hour)),
-		Issued:  time.Now(),
+		Issued:  timestamppb.New(time.Now()),
-		DER:     block.Bytes,
+		Der:     block.Bytes,
 	}
 
 	names, problems := checker.checkCert(context.Background(), cert)
@@ -262,11 +263,11 @@ func TestCheckCert(t *testing.T) {
 			//   Serial doesn't match
 			//   Expiry doesn't match
 			//   Issued doesn't match
-			cert := core.Certificate{
+			cert := &corepb.Certificate{
 				Serial:  "8485f2687eba29ad455ae4e31c8679206fec",
-				DER:     brokenCertDer,
+				Der:     brokenCertDer,
-				Issued:  issued.Add(12 * time.Hour),
+				Issued:  timestamppb.New(issued.Add(12 * time.Hour)),
-				Expires: goodExpiry.AddDate(0, 0, 2), // Expiration doesn't match
+				Expires: timestamppb.New(goodExpiry.AddDate(0, 0, 2)), // Expiration doesn't match
 			}
 
 			_, problems := checker.checkCert(context.Background(), cert)
@@ -318,9 +319,9 @@ func TestCheckCert(t *testing.T) {
 			test.AssertNotError(t, err, "Couldn't parse created certificate")
 			cert.Serial = core.SerialToString(serial)
 			cert.Digest = core.Fingerprint256(goodCertDer)
-			cert.DER = goodCertDer
+			cert.Der = goodCertDer
-			cert.Expires = parsed.NotAfter
+			cert.Expires = timestamppb.New(parsed.NotAfter)
-			cert.Issued = parsed.NotBefore
+			cert.Issued = timestamppb.New(parsed.NotBefore)
 			_, problems = checker.checkCert(context.Background(), cert)
 			test.AssertEquals(t, len(problems), 0)
 		})
@@ -396,9 +397,6 @@ func (db mismatchedCountDB) SelectNullInt(_ context.Context, _ string, _ ...inte
 // `getCerts` then calls `Select` to retrieve the Certificate rows. We pull
 // a dastardly switch-a-roo here and return an empty set
 func (db mismatchedCountDB) Select(_ context.Context, output interface{}, _ string, _ ...interface{}) ([]interface{}, error) {
-	// But actually return nothing
-	outputPtr, _ := output.(*[]sa.CertWithID)
-	*outputPtr = []sa.CertWithID{}
 	return nil, nil
 }
 
@@ -624,12 +622,12 @@ func TestIgnoredLint(t *testing.T) {
 	subjectCert, err := x509.ParseCertificate(subjectCertDer)
 	test.AssertNotError(t, err, "failed to parse EE cert")
 
-	cert := core.Certificate{
+	cert := &corepb.Certificate{
 		Serial:  core.SerialToString(serial),
-		DER:     subjectCertDer,
+		Der:     subjectCertDer,
 		Digest:  core.Fingerprint256(subjectCertDer),
-		Issued:  subjectCert.NotBefore,
+		Issued:  timestamppb.New(subjectCert.NotBefore),
-		Expires: subjectCert.NotAfter,
+		Expires: timestamppb.New(subjectCert.NotAfter),
 	}
 
 	// Without any ignored lints we expect several errors and warnings about SCTs,
@@ -679,12 +677,12 @@ func TestPrecertCorrespond(t *testing.T) {
 		SerialNumber: serial,
 	}
 	certDer, _ := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey)
-	cert := core.Certificate{
+	cert := &corepb.Certificate{
 		Serial:  core.SerialToString(serial),
 		Digest:  core.Fingerprint256(certDer),
-		DER:     certDer,
+		Der:     certDer,
-		Issued:  time.Now(),
+		Issued:  timestamppb.New(time.Now()),
-		Expires: expiry,
+		Expires: timestamppb.New(expiry),
 	}
 	_, problems := checker.checkCert(context.Background(), cert)
 	if len(problems) == 0 {

cmd/expiration-mailer/main.go

@@ -608,7 +608,6 @@ func (m *mailer) getCerts(ctx context.Context, left, right time.Time, expiresIn
 		if ctx.Err() != nil {
 			return nil, ctx.Err()
 		}
-		var cert core.Certificate
 		cert, err := sa.SelectCertificate(ctx, m.dbMap, serial)
 		if err != nil {
 			// We can get a NoRowsErr when processing a serial number corresponding
@@ -623,13 +622,13 @@ func (m *mailer) getCerts(ctx context.Context, left, right time.Time, expiresIn
 			continue
 		}
 		certs = append(certs, certDERWithRegID{
-			DER:   cert.DER,
+			DER:   cert.Der,
 			RegID: cert.RegistrationID,
 		})
 		if i == 0 {
 			// Report the send delay metric. Note: this is the worst-case send delay
 			// of any certificate in this batch because it's based on the first (oldest).
-			sendDelay := expiresIn - cert.Expires.Sub(m.clk.Now())
+			sendDelay := expiresIn - cert.Expires.AsTime().Sub(m.clk.Now())
 			m.stats.sendDelay.With(prometheus.Labels{"nag_group": expiresIn.String()}).Set(
 				sendDelay.Truncate(time.Second).Seconds())
 		}

grpc/pb-marshalling.go

@@ -343,28 +343,6 @@ func newOrderValid(order *corepb.Order) bool {
 	return !(order.RegistrationID == 0 || order.Expires == nil || len(order.Identifiers) == 0)
 }
 
-func CertToPB(cert core.Certificate) *corepb.Certificate {
-	return &corepb.Certificate{
-		RegistrationID: cert.RegistrationID,
-		Serial:         cert.Serial,
-		Digest:         cert.Digest,
-		Der:            cert.DER,
-		Issued:         timestamppb.New(cert.Issued),
-		Expires:        timestamppb.New(cert.Expires),
-	}
-}
-
-func PBToCert(pb *corepb.Certificate) core.Certificate {
-	return core.Certificate{
-		RegistrationID: pb.RegistrationID,
-		Serial:         pb.Serial,
-		Digest:         pb.Digest,
-		DER:            pb.Der,
-		Issued:         pb.Issued.AsTime(),
-		Expires:        pb.Expires.AsTime(),
-	}
-}
-
 func CertStatusToPB(certStatus core.CertificateStatus) *corepb.CertificateStatus {
 	return &corepb.CertificateStatus{
 		Serial:                certStatus.Serial,

grpc/pb-marshalling_test.go

@@ -267,23 +267,6 @@ func TestAuthz(t *testing.T) {
 	test.AssertDeepEquals(t, inAuthzNilExpires, outAuthz2)
 }
 
-func TestCert(t *testing.T) {
-	now := time.Now().Round(0).UTC()
-	cert := core.Certificate{
-		RegistrationID: 1,
-		Serial:         "serial",
-		Digest:         "digest",
-		DER:            []byte{255},
-		Issued:         now,
-		Expires:        now.Add(time.Hour),
-	}
-
-	certPB := CertToPB(cert)
-	outCert := PBToCert(certPB)
-
-	test.AssertDeepEquals(t, cert, outCert)
-}
-
 func TestOrderValid(t *testing.T) {
 	created := time.Now()
 	expires := created.Add(1 * time.Hour)

sa/model.go

@@ -139,65 +139,59 @@ func selectRegistration(ctx context.Context, s db.OneSelector, whereCol string,
 	return &model, err
 }
 
-const certFields = "registrationID, serial, digest, der, issued, expires"
+const certFields = "id, registrationID, serial, digest, der, issued, expires"
 
 // SelectCertificate selects all fields of one certificate object identified by
 // a serial. If more than one row contains the same serial only the first is
 // returned.
-func SelectCertificate(ctx context.Context, s db.OneSelector, serial string) (core.Certificate, error) {
+func SelectCertificate(ctx context.Context, s db.OneSelector, serial string) (*corepb.Certificate, error) {
-	var model core.Certificate
+	var model certificateModel
 	err := s.SelectOne(
 		ctx,
 		&model,
 		"SELECT "+certFields+" FROM certificates WHERE serial = ? LIMIT 1",
 		serial,
 	)
-	return model, err
+	return model.toPb(), err
 }
 
 const precertFields = "registrationID, serial, der, issued, expires"
 
 // SelectPrecertificate selects all fields of one precertificate object
 // identified by serial.
-func SelectPrecertificate(ctx context.Context, s db.OneSelector, serial string) (core.Certificate, error) {
+func SelectPrecertificate(ctx context.Context, s db.OneSelector, serial string) (*corepb.Certificate, error) {
 	var model lintingCertModel
 	err := s.SelectOne(
 		ctx,
 		&model,
 		"SELECT "+precertFields+" FROM precertificates WHERE serial = ? LIMIT 1",
 		serial)
-	return core.Certificate{
+	if err != nil {
-		RegistrationID: model.RegistrationID,
+		return nil, err
-		Serial:         model.Serial,
-		DER:            model.DER,
-		Issued:         model.Issued,
-		Expires:        model.Expires,
-	}, err
 	}
-
+	return model.toPb(), nil
-type CertWithID struct {
-	ID int64
-	core.Certificate
 }
 
 // SelectCertificates selects all fields of multiple certificate objects
-func SelectCertificates(ctx context.Context, s db.Selector, q string, args map[string]interface{}) ([]CertWithID, error) {
+//
-	var models []CertWithID
+// Returns a slice of *corepb.Certificate along with the highest ID field seen
+// (which can be used as input to a subsequent query when iterating in primary
+// key order).
+func SelectCertificates(ctx context.Context, s db.Selector, q string, args map[string]interface{}) ([]*corepb.Certificate, int64, error) {
+	var models []certificateModel
 	_, err := s.Select(
 		ctx,
 		&models,
-		"SELECT id, "+certFields+" FROM certificates "+q, args)
+		"SELECT "+certFields+" FROM certificates "+q, args)
-	return models, err
+	var pbs []*corepb.Certificate
+	var highestID int64
+	for _, m := range models {
+		pbs = append(pbs, m.toPb())
+		if m.ID > highestID {
+			highestID = m.ID
 		}
-
+	}
-// SelectPrecertificates selects all fields of multiple precertificate objects.
+	return pbs, highestID, err
-func SelectPrecertificates(ctx context.Context, s db.Selector, q string, args map[string]interface{}) ([]CertWithID, error) {
-	var models []CertWithID
-	_, err := s.Select(
-		ctx,
-		&models,
-		"SELECT id, "+precertFields+" FROM precertificates "+q, args)
-	return models, err
 }
 
 type CertStatusMetadata struct {
@@ -366,6 +360,38 @@ type lintingCertModel struct {
 	Expires        time.Time
 }
 
+func (model lintingCertModel) toPb() *corepb.Certificate {
+	return &corepb.Certificate{
+		RegistrationID: model.RegistrationID,
+		Serial:         model.Serial,
+		Digest:         "",
+		Der:            model.DER,
+		Issued:         timestamppb.New(model.Issued),
+		Expires:        timestamppb.New(model.Expires),
+	}
+}
+
+type certificateModel struct {
+	ID             int64     `db:"id"`
+	RegistrationID int64     `db:"registrationID"`
+	Serial         string    `db:"serial"`
+	Digest         string    `db:"digest"`
+	DER            []byte    `db:"der"`
+	Issued         time.Time `db:"issued"`
+	Expires        time.Time `db:"expires"`
+}
+
+func (model certificateModel) toPb() *corepb.Certificate {
+	return &corepb.Certificate{
+		RegistrationID: model.RegistrationID,
+		Serial:         model.Serial,
+		Digest:         model.Digest,
+		Der:            model.DER,
+		Issued:         timestamppb.New(model.Issued),
+		Expires:        timestamppb.New(model.Expires),
+	}
+}
+
 // orderModel represents one row in the orders table. The CertificateProfileName
 // column is a pointer because the column is NULL-able.
 type orderModel struct {

sa/model_test.go

@@ -305,7 +305,7 @@ func TestCertificatesTableContainsDuplicateSerials(t *testing.T) {
 	test.AssertNotError(t, err, "received an error for a valid query")
 
 	// Ensure that `certA` and `certB` are the same.
-	test.AssertByteEquals(t, certA.DER, certB.DER)
+	test.AssertByteEquals(t, certA.Der, certB.Der)
 }
 
 func insertCertificate(ctx context.Context, dbMap *db.WrappedMap, fc clock.FakeClock, hostname, cn string, serial, regID int64) error {

sa/saro.go

@@ -213,7 +213,7 @@ func (ssa *SQLStorageAuthorityRO) GetCertificate(ctx context.Context, req *sapb.
 	if err != nil {
 		return nil, err
 	}
-	return bgrpc.CertToPB(cert), nil
+	return cert, nil
 }
 
 // GetLintPrecertificate takes a serial number and returns the corresponding
@@ -235,7 +235,7 @@ func (ssa *SQLStorageAuthorityRO) GetLintPrecertificate(ctx context.Context, req
 	if err != nil {
 		return nil, err
 	}
-	return bgrpc.CertToPB(cert), nil
+	return cert, nil
 }
 
 // GetCertificateStatus takes a hexadecimal string representing the full 128-bit serial