diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 51390c996..0d78c3f7a 100644 --- a/cmd/cert-checker/main.go +++ b/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.After(c.issuedReport.end) { + if lastCert.Issued.AsTime().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 { - authzs, err := sa.SelectAuthzsMatchingIssuance(ctx, c.dbMap, cert.RegistrationID, cert.Issued, idents) +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.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)) diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index 4cfc10019..d8f9e8aaa 100644 --- a/cmd/cert-checker/main_test.go +++ b/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, - Issued: time.Now(), - Expires: expiry, + Der: certDer, + Issued: timestamppb.New(time.Now()), + 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, - Issued: parsed.NotBefore, - DER: wildcardCertDer, + Expires: timestamppb.New(parsed.NotAfter), + Issued: timestamppb.New(parsed.NotBefore), + 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), - Issued: time.Now(), - DER: block.Bytes, + Expires: timestamppb.New(time.Now().Add(time.Hour)), + Issued: timestamppb.New(time.Now()), + 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, - Issued: issued.Add(12 * time.Hour), - Expires: goodExpiry.AddDate(0, 0, 2), // Expiration doesn't match + Der: brokenCertDer, + Issued: timestamppb.New(issued.Add(12 * time.Hour)), + 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.Expires = parsed.NotAfter - cert.Issued = parsed.NotBefore + cert.Der = goodCertDer + cert.Expires = timestamppb.New(parsed.NotAfter) + 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, - Expires: subjectCert.NotAfter, + Issued: timestamppb.New(subjectCert.NotBefore), + 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, - Issued: time.Now(), - Expires: expiry, + Der: certDer, + Issued: timestamppb.New(time.Now()), + Expires: timestamppb.New(expiry), } _, problems := checker.checkCert(context.Background(), cert) if len(problems) == 0 { diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index 3a6b8c19b..8c80c8408 100644 --- a/cmd/expiration-mailer/main.go +++ b/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()) } diff --git a/grpc/pb-marshalling.go b/grpc/pb-marshalling.go index f379d09ed..a05c83716 100644 --- a/grpc/pb-marshalling.go +++ b/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, diff --git a/grpc/pb-marshalling_test.go b/grpc/pb-marshalling_test.go index b38e6249e..d07664191 100644 --- a/grpc/pb-marshalling_test.go +++ b/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) diff --git a/sa/model.go b/sa/model.go index 3e82991dc..8c6a74366 100644 --- a/sa/model.go +++ b/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) { - var model core.Certificate +func SelectCertificate(ctx context.Context, s db.OneSelector, serial string) (*corepb.Certificate, error) { + 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{ - RegistrationID: model.RegistrationID, - Serial: model.Serial, - DER: model.DER, - Issued: model.Issued, - Expires: model.Expires, - }, err -} - -type CertWithID struct { - ID int64 - core.Certificate + if err != nil { + return nil, err + } + return model.toPb(), nil } // 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) - return models, err -} - -// SelectPrecertificates selects all fields of multiple precertificate objects. -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 + "SELECT "+certFields+" FROM certificates "+q, args) + var pbs []*corepb.Certificate + var highestID int64 + for _, m := range models { + pbs = append(pbs, m.toPb()) + if m.ID > highestID { + highestID = m.ID + } + } + return pbs, highestID, 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 { diff --git a/sa/model_test.go b/sa/model_test.go index 9e7659548..60c79e1de 100644 --- a/sa/model_test.go +++ b/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 { diff --git a/sa/saro.go b/sa/saro.go index 4342e94d1..3780ff10a 100644 --- a/sa/saro.go +++ b/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 @@ -305,7 +305,7 @@ func (ssa *SQLStorageAuthorityRO) FQDNSetTimestampsForWindow(ctx context.Context _, err := ssa.dbReadOnlyMap.Select( ctx, &rows, - `SELECT issued FROM fqdnSets + `SELECT issued FROM fqdnSets WHERE setHash = ? AND issued > ? ORDER BY issued DESC @@ -1254,7 +1254,7 @@ func (ssa *SQLStorageAuthorityRO) GetPausedIdentifiers(ctx context.Context, req _, err := ssa.dbReadOnlyMap.Select(ctx, &matches, ` SELECT identifierType, identifierValue FROM paused - WHERE + WHERE registrationID = ? AND unpausedAt IS NULL LIMIT 15`,