diff --git a/cmd/bad-key-revoker/main.go b/cmd/bad-key-revoker/main.go index b234987f5cb..e7015e0c848 100644 --- a/cmd/bad-key-revoker/main.go +++ b/cmd/bad-key-revoker/main.go @@ -141,7 +141,7 @@ func (bkr *badKeyRevoker) findUnrevoked(ctx context.Context, unchecked unchecked "SELECT id, certSerial FROM keyHashToSerial WHERE keyHash = ? AND id > ? AND certNotAfter > ? ORDER BY id LIMIT ?", unchecked.KeyHash, initialID, - bkr.clk.Now().Truncate(time.Second), + bkr.clk.Now(), bkr.serialBatchSize, ) if err != nil { diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 7e311ae9e4a..b6ec4e7ac35 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -326,7 +326,7 @@ must check that timestamps are non-zero before accepting them. # Rounding time in DB -All times that we write to the database are truncated to one second's worth of +All times that we send to the database are truncated to one second's worth of precision. This reduces the size of indexes that include timestamps, and makes querying them more efficient. The Storage Authority (SA) is responsible for this truncation, and performs it for SELECT queries as well as INSERT and UPDATE. diff --git a/sa/sa.go b/sa/sa.go index 1aa1d606601..0546ea3a49a 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -109,7 +109,7 @@ func (ssa *SQLStorageAuthority) NewRegistration(ctx context.Context, req *corepb return nil, err } - reg.CreatedAt = ssa.clk.Now().Truncate(time.Second) + reg.CreatedAt = ssa.clk.Now() err = ssa.dbMap.Insert(ctx, reg) if err != nil { @@ -142,10 +142,6 @@ func (ssa *SQLStorageAuthority) UpdateRegistration(ctx context.Context, req *cor return nil, err } - // The CreatedAt field shouldn't change from the original, so we copy it straight through. - // This also ensures that it's already truncated to second (which happened on creation). - update.CreatedAt = curr.CreatedAt - // Copy the existing registration model's LockCol to the new updated // registration model's LockCol update.LockCol = curr.LockCol @@ -174,8 +170,8 @@ func (ssa *SQLStorageAuthority) AddSerial(ctx context.Context, req *sapb.AddSeri err := ssa.dbMap.Insert(ctx, &recordedSerialModel{ Serial: req.Serial, RegistrationID: req.RegID, - Created: req.Created.AsTime().Truncate(time.Second), - Expires: req.Expires.AsTime().Truncate(time.Second), + Created: req.Created.AsTime(), + Expires: req.Expires.AsTime(), }) if err != nil { return nil, err @@ -228,7 +224,7 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb Serial: serialHex, RegistrationID: req.RegID, DER: req.Der, - Issued: req.Issued.AsTime().Truncate(time.Second), + Issued: req.Issued.AsTime(), Expires: parsed.NotAfter, } @@ -257,15 +253,13 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb cs := &core.CertificateStatus{ Serial: serialHex, Status: status, - OCSPLastUpdated: ssa.clk.Now().Truncate(time.Second), + OCSPLastUpdated: ssa.clk.Now(), RevokedDate: time.Time{}, RevokedReason: 0, LastExpirationNagSent: time.Time{}, - // No need to truncate because it's already truncated to encode - // per https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.5.1 - NotAfter: parsed.NotAfter, - IsExpired: false, - IssuerNameID: req.IssuerNameID, + NotAfter: parsed.NotAfter, + IsExpired: false, + IssuerNameID: req.IssuerNameID, } err = ssa.dbMap.Insert(ctx, cs) if err != nil { @@ -324,7 +318,7 @@ func (ssa *SQLStorageAuthority) AddCertificate(ctx context.Context, req *sapb.Ad Serial: serial, Digest: digest, DER: req.Der, - Issued: req.Issued.AsTime().Truncate(time.Second), + Issued: req.Issued.AsTime(), Expires: parsedCertificate.NotAfter, } @@ -483,15 +477,13 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb if err != nil { return nil, err } - // These parameters correspond to the fields listed in `authzFields`, as used in the - // `db.NewMultiInserter` call above, and occur in the same order. err = inserter.Add([]interface{}{ am.ID, am.IdentifierType, am.IdentifierValue, am.RegistrationID, statusToUint[core.StatusPending], - am.Expires.Truncate(time.Second), + am.Expires, am.Challenges, nil, nil, @@ -512,12 +504,11 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb // Second, insert the new order. var orderID int64 var err error - created := ssa.clk.Now().Truncate(time.Second) - expires := req.NewOrder.Expires.AsTime().Truncate(time.Second) + created := ssa.clk.Now() if features.Get().MultipleCertificateProfiles { omv2 := orderModelv2{ RegistrationID: req.NewOrder.RegistrationID, - Expires: expires, + Expires: req.NewOrder.Expires.AsTime(), Created: created, CertificateProfileName: req.NewOrder.CertificateProfileName, } @@ -526,7 +517,7 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb } else { omv1 := orderModelv1{ RegistrationID: req.NewOrder.RegistrationID, - Expires: expires, + Expires: req.NewOrder.Expires.AsTime(), Created: created, } err = tx.Insert(ctx, &omv1) @@ -559,25 +550,19 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb } // Fourth, insert the FQDNSet entry for the order. - err = addOrderFQDNSet(ctx, - tx, - req.NewOrder.Names, - orderID, - req.NewOrder.RegistrationID, - expires, - ) + err = addOrderFQDNSet(ctx, tx, req.NewOrder.Names, orderID, req.NewOrder.RegistrationID, req.NewOrder.Expires.AsTime()) if err != nil { return nil, err } - // Finally, build the overall Order PB to return. + // Finally, build the overall Order PB. res := &corepb.Order{ // ID and Created were auto-populated on the order model when it was inserted. Id: orderID, Created: timestamppb.New(created), // These are carried over from the original request unchanged. RegistrationID: req.NewOrder.RegistrationID, - Expires: timestamppb.New(expires), + Expires: req.NewOrder.Expires, Names: req.NewOrder.Names, // Have to combine the already-associated and newly-reacted authzs. V2Authorizations: append(req.NewOrder.V2Authorizations, newAuthzIDs...), @@ -592,12 +577,7 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb if req.NewOrder.ReplacesSerial != "" { // Update the replacementOrders table to indicate that this order // replaces the provided certificate serial. - err := addReplacementOrder(ctx, - tx, - req.NewOrder.ReplacesSerial, - orderID, - req.NewOrder.Expires.AsTime().Truncate(time.Second), - ) + err := addReplacementOrder(ctx, tx, req.NewOrder.ReplacesSerial, orderID, req.NewOrder.Expires.AsTime()) if err != nil { return nil, err } @@ -810,7 +790,7 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization2(ctx context.Context, req // database attemptedAt field Null instead of 1970-01-01 00:00:00. var attemptedTime *time.Time if !core.IsAnyNilOrZero(req.AttemptedAt) { - val := req.AttemptedAt.AsTime().Truncate(time.Second) + val := req.AttemptedAt.AsTime() attemptedTime = &val } params := map[string]interface{}{ @@ -820,7 +800,7 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization2(ctx context.Context, req "validationRecord": vrJSON, "id": req.Id, "pending": statusUint(core.StatusPending), - "expires": req.Expires.AsTime().Truncate(time.Second), + "expires": req.Expires.AsTime(), // if req.ValidationError is nil veJSON should also be nil // which should result in a NULL field "validationError": veJSON, @@ -888,7 +868,7 @@ func (ssa *SQLStorageAuthority) RevokeCertificate(ctx context.Context, req *sapb } _, overallError := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) { - revokedDate := req.Date.AsTime().Truncate(time.Second) + revokedDate := req.Date.AsTime() res, err := tx.ExecContext(ctx, `UPDATE certificateStatus SET @@ -945,8 +925,8 @@ func (ssa *SQLStorageAuthority) UpdateRevokedCertificate(ctx context.Context, re } _, overallError := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) { - thisUpdate := req.Date.AsTime().Truncate(time.Second) - revokedDate := req.Backdate.AsTime().Truncate(time.Second) + thisUpdate := req.Date.AsTime() + revokedDate := req.Backdate.AsTime() res, err := tx.ExecContext(ctx, `UPDATE certificateStatus SET @@ -1028,7 +1008,7 @@ func (ssa *SQLStorageAuthority) AddBlockedKey(ctx context.Context, req *sapb.Add cols, qs := blockedKeysColumns, "?, ?, ?, ?" vals := []interface{}{ req.KeyHash, - req.Added.AsTime().Truncate(time.Second), + req.Added.AsTime(), sourceInt, req.Comment, } @@ -1271,12 +1251,11 @@ func (ssa *SQLStorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Up // Only set the nextUpdate if it's actually present in the request message. var nextUpdate *time.Time if req.NextUpdate != nil { - nut := req.NextUpdate.AsTime().Truncate(time.Second) + nut := req.NextUpdate.AsTime() nextUpdate = &nut } _, err := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) { - thisUpdate := req.ThisUpdate.AsTime().Truncate(time.Second) res, err := tx.ExecContext(ctx, `UPDATE crlShards SET thisUpdate = ?, nextUpdate = ?, leasedUntil = ? @@ -1284,12 +1263,12 @@ func (ssa *SQLStorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Up AND idx = ? AND (thisUpdate is NULL OR thisUpdate <= ?) LIMIT 1`, - thisUpdate, + req.ThisUpdate.AsTime(), nextUpdate, - thisUpdate, + req.ThisUpdate.AsTime(), req.IssuerNameID, req.ShardIdx, - thisUpdate, + req.ThisUpdate.AsTime(), ) if err != nil { return nil, err diff --git a/sa/sa_test.go b/sa/sa_test.go index 74f244c98a8..f4dcdafeafd 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -4013,7 +4013,7 @@ func TestUpdateCRLShard(t *testing.T) { `SELECT thisUpdate FROM crlShards WHERE issuerID = 1 AND idx = 0 LIMIT 1`, ) test.AssertNotError(t, err, "getting updated thisUpdate timestamp") - test.AssertEquals(t, *crlModel.ThisUpdate, thisUpdate) + test.Assert(t, crlModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp") // Updating an unleased shard should work. _, err = sa.UpdateCRLShard( diff --git a/sa/saro.go b/sa/saro.go index debc6b212f4..d736855aa96 100644 --- a/sa/saro.go +++ b/sa/saro.go @@ -230,8 +230,8 @@ func (ssa *SQLStorageAuthorityRO) CountRegistrationsByIP(ctx context.Context, re createdAt <= :latest`, map[string]interface{}{ "ip": req.Ip, - "earliest": req.Range.Earliest.AsTime().Truncate(time.Second), - "latest": req.Range.Latest.AsTime().Truncate(time.Second), + "earliest": req.Range.Earliest.AsTime(), + "latest": req.Range.Latest.AsTime(), }) if err != nil { return nil, err @@ -261,8 +261,8 @@ func (ssa *SQLStorageAuthorityRO) CountRegistrationsByIPRange(ctx context.Contex :earliest < createdAt AND createdAt <= :latest`, map[string]interface{}{ - "earliest": req.Range.Earliest.AsTime().Truncate(time.Second), - "latest": req.Range.Latest.AsTime().Truncate(time.Second), + "earliest": req.Range.Earliest.AsTime(), + "latest": req.Range.Latest.AsTime(), "beginIP": beginIP, "endIP": endIP, }) @@ -507,7 +507,7 @@ func (ssa *SQLStorageAuthorityRO) CountFQDNSets(ctx context.Context, req *sapb.C WHERE setHash = ? AND issued > ?`, core.HashNames(req.Domains), - ssa.clk.Now().Add(-req.Window.AsDuration()).Truncate(time.Second), + ssa.clk.Now().Add(-req.Window.AsDuration()), ) return &sapb.Count{Count: count}, err } @@ -531,7 +531,7 @@ func (ssa *SQLStorageAuthorityRO) FQDNSetTimestampsForWindow(ctx context.Context AND issued > ? ORDER BY issued DESC`, core.HashNames(req.Domains), - ssa.clk.Now().Add(-req.Window.AsDuration()).Truncate(time.Second), + ssa.clk.Now().Add(-req.Window.AsDuration()), ) if err != nil { return nil, err @@ -708,8 +708,7 @@ func (ssa *SQLStorageAuthorityRO) GetOrderForNames(ctx context.Context, req *sap AND expires > ? ORDER BY expires ASC LIMIT 1`, - fqdnHash, - ssa.clk.Now().Truncate(time.Second)) + fqdnHash, ssa.clk.Now()) if db.IsNoRows(err) { return nil, berrors.NotFoundError("no order matching request found") @@ -792,7 +791,7 @@ func (ssa *SQLStorageAuthorityRO) GetAuthorizations2(ctx context.Context, req *s req.RegistrationID, statusUint(core.StatusValid), statusUint(core.StatusPending), - req.Now.AsTime().Truncate(time.Second), + req.Now.AsTime(), identifierTypeToUint[string(identifier.DNS)], } @@ -860,7 +859,7 @@ func (ssa *SQLStorageAuthorityRO) GetPendingAuthorization2(ctx context.Context, map[string]interface{}{ "regID": req.RegistrationID, "status": statusUint(core.StatusPending), - "validUntil": req.ValidUntil.AsTime().Truncate(time.Second), + "validUntil": req.ValidUntil.AsTime(), "dnsType": identifierTypeToUint[string(identifier.DNS)], "ident": req.IdentifierValue, }, @@ -889,7 +888,7 @@ func (ssa *SQLStorageAuthorityRO) CountPendingAuthorizations2(ctx context.Contex status = :status`, map[string]interface{}{ "regID": req.Id, - "expires": ssa.clk.Now().Truncate(time.Second), + "expires": ssa.clk.Now(), "status": statusUint(core.StatusPending), }, ) @@ -930,7 +929,7 @@ func (ssa *SQLStorageAuthorityRO) GetValidOrderAuthorizations2(ctx context.Conte ), map[string]interface{}{ "regID": req.AcctID, - "expires": ssa.clk.Now().Truncate(time.Second), + "expires": ssa.clk.Now(), "status": statusUint(core.StatusValid), "orderID": req.Id, }, @@ -976,8 +975,8 @@ func (ssa *SQLStorageAuthorityRO) CountInvalidAuthorizations2(ctx context.Contex "regID": req.RegistrationID, "dnsType": identifierTypeToUint[string(identifier.DNS)], "ident": req.Hostname, - "expiresEarliest": req.Range.Earliest.AsTime().Truncate(time.Second), - "expiresLatest": req.Range.Latest.AsTime().Truncate(time.Second), + "expiresEarliest": req.Range.Earliest.AsTime(), + "expiresLatest": req.Range.Latest.AsTime(), "status": statusUint(core.StatusInvalid), }, ) @@ -1010,7 +1009,7 @@ func (ssa *SQLStorageAuthorityRO) GetValidAuthorizations2(ctx context.Context, r params := []interface{}{ req.RegistrationID, statusUint(core.StatusValid), - req.Now.AsTime().Truncate(time.Second), + req.Now.AsTime(), identifierTypeToUint[string(identifier.DNS)], } for _, domain := range req.Domains { @@ -1226,8 +1225,8 @@ func (ssa *SQLStorageAuthorityRO) getRevokedCertsFromCertificateStatusTable(req AND issuerID = ? AND status = ?` params := []interface{}{ - req.ExpiresAfter.AsTime().Truncate(time.Second), - req.ExpiresBefore.AsTime().Truncate(time.Second), + req.ExpiresAfter.AsTime(), + req.ExpiresBefore.AsTime(), req.IssuerNameID, core.OCSPStatusRevoked, } @@ -1358,7 +1357,7 @@ func (ssa *SQLStorageAuthorityRO) GetSerialsByKey(req *sapb.SPKIHash, stream grp AND certNotAfter > ?` params := []interface{}{ req.KeyHash, - ssa.clk.Now().Truncate(time.Second), + ssa.clk.Now(), } selector, err := db.NewMappedSelector[keyHashModel](ssa.dbReadOnlyMap) @@ -1385,7 +1384,7 @@ func (ssa *SQLStorageAuthorityRO) GetSerialsByAccount(req *sapb.RegistrationID, AND expires > ?` params := []interface{}{ req.Id, - ssa.clk.Now().Truncate(time.Second), + ssa.clk.Now(), } selector, err := db.NewMappedSelector[recordedSerialModel](ssa.dbReadOnlyMap)