Skip to content

Commit 0a72637

Browse files
authored
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.
1 parent 75a89f7 commit 0a72637

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

crl/updater/updater.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func (cu *crlUpdater) updateShard(ctx context.Context, atTime time.Time, issuerN
314314

315315
// Query for unexpired certificates, with padding to ensure that revoked certificates show
316316
// up in at least one CRL, even if they expire between revocation and CRL generation.
317-
expiresAfter := cu.clk.Now().Add(cu.lookbackPeriod)
317+
expiresAfter := cu.clk.Now().Add(-cu.lookbackPeriod)
318318

319319
saStream, err := cu.sa.GetRevokedCertsByShard(ctx, &sapb.GetRevokedCertsByShardRequest{
320320
IssuerNameID: int64(issuerNameID),

crl/updater/updater_test.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"errors"
8+
"fmt"
89
"io"
910
"reflect"
1011
"testing"
@@ -67,6 +68,15 @@ func (f *fakeSAC) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRe
6768

6869
// Return some configured contents, but only for shard 2.
6970
func (f *fakeSAC) GetRevokedCertsByShard(ctx context.Context, req *sapb.GetRevokedCertsByShardRequest, _ ...grpc.CallOption) (grpc.ServerStreamingClient[corepb.CRLEntry], error) {
71+
// This time is based on the setting of `clk` in TestUpdateShard,
72+
// minus the setting of `lookbackPeriod` in that same function (24h).
73+
want := time.Date(2020, time.January, 17, 0, 0, 0, 0, time.UTC)
74+
got := req.ExpiresAfter.AsTime().UTC()
75+
if !got.Equal(want) {
76+
return nil, fmt.Errorf("fakeSAC.GetRevokedCertsByShard called with ExpiresAfter=%s, want %s",
77+
got, want)
78+
}
79+
7080
if req.ShardIdx == 2 {
7181
return &f.revokedCertsByShard, nil
7282
}
@@ -220,11 +230,15 @@ func TestUpdateShard(t *testing.T) {
220230
defer cancel()
221231

222232
clk := clock.NewFake()
223-
clk.Set(time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
233+
clk.Set(time.Date(2020, time.January, 18, 0, 0, 0, 0, time.UTC))
224234
cu, err := NewUpdater(
225235
[]*issuance.Certificate{e1, r3},
226-
2, 18*time.Hour, 24*time.Hour,
227-
6*time.Hour, time.Minute, 1, 1,
236+
2,
237+
18*time.Hour, // shardWidth
238+
24*time.Hour, // lookbackPeriod
239+
6*time.Hour, // updatePeriod
240+
time.Minute, // updateTimeout
241+
1, 1,
228242
"stale-if-error=60",
229243
5*time.Minute,
230244
nil,

0 commit comments

Comments
 (0)