crl/updater: fix lookback period (#8072)

We were adding the lookback period to `clk.Now()` but should have been
subtracting it. Includes a unittest, which I've verified fails against
the pre-fix code.
This commit is contained in:
Jacob Hoffman-Andrews 2025-03-18 10:39:29 -07:00 committed by GitHub
parent 75a89f7a4a
commit 0a726370b9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 18 additions and 4 deletions

View File

@ -314,7 +314,7 @@ func (cu *crlUpdater) updateShard(ctx context.Context, atTime time.Time, issuerN
// Query for unexpired certificates, with padding to ensure that revoked certificates show
// up in at least one CRL, even if they expire between revocation and CRL generation.
expiresAfter := cu.clk.Now().Add(cu.lookbackPeriod)
expiresAfter := cu.clk.Now().Add(-cu.lookbackPeriod)
saStream, err := cu.sa.GetRevokedCertsByShard(ctx, &sapb.GetRevokedCertsByShardRequest{
IssuerNameID: int64(issuerNameID),

View File

@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"reflect"
"testing"
@ -67,6 +68,15 @@ func (f *fakeSAC) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRe
// Return some configured contents, but only for shard 2.
func (f *fakeSAC) GetRevokedCertsByShard(ctx context.Context, req *sapb.GetRevokedCertsByShardRequest, _ ...grpc.CallOption) (grpc.ServerStreamingClient[corepb.CRLEntry], error) {
// This time is based on the setting of `clk` in TestUpdateShard,
// minus the setting of `lookbackPeriod` in that same function (24h).
want := time.Date(2020, time.January, 17, 0, 0, 0, 0, time.UTC)
got := req.ExpiresAfter.AsTime().UTC()
if !got.Equal(want) {
return nil, fmt.Errorf("fakeSAC.GetRevokedCertsByShard called with ExpiresAfter=%s, want %s",
got, want)
}
if req.ShardIdx == 2 {
return &f.revokedCertsByShard, nil
}
@ -220,11 +230,15 @@ func TestUpdateShard(t *testing.T) {
defer cancel()
clk := clock.NewFake()
clk.Set(time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
clk.Set(time.Date(2020, time.January, 18, 0, 0, 0, 0, time.UTC))
cu, err := NewUpdater(
[]*issuance.Certificate{e1, r3},
2, 18*time.Hour, 24*time.Hour,
6*time.Hour, time.Minute, 1, 1,
2,
18*time.Hour, // shardWidth
24*time.Hour, // lookbackPeriod
6*time.Hour, // updatePeriod
time.Minute, // updateTimeout
1, 1,
"stale-if-error=60",
5*time.Minute,
nil,