diff --git a/ra/ra.go b/ra/ra.go index 8e92a095faa..9fd1ee99d10 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1438,8 +1438,13 @@ func (ra *RegistrationAuthorityImpl) countFailedValidations(ctx context.Context, // resetAccountPausingLimit resets bucket to maximum capacity for given account. // There is no reason to surface errors from this function to the Subscriber. func (ra *RegistrationAuthorityImpl) resetAccountPausingLimit(ctx context.Context, regId int64, ident identifier.ACMEIdentifier) { - bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, regId, ident.Value) - err := ra.limiter.Reset(ctx, bucketKey) + txns, err := ra.txnBuilder.NewPausingResetTransactions(regId, ident) + if err != nil { + ra.log.Warningf("building reset transaction for regID=[%d] identifier=[%s]: %s", regId, ident.Value, err) + return + } + + err = ra.limiter.BatchReset(ctx, txns) if err != nil { ra.log.Warningf("resetting bucket for regID=[%d] identifier=[%s]: %s", regId, ident.Value, err) } diff --git a/ra/ra_test.go b/ra/ra_test.go index 164272f5e33..f06fd3e33cf 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -715,7 +715,8 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t * domain := randomDomain() ident := identifier.NewDNS(domain) authzPB := createPendingAuthorization(t, sa, registration.Id, ident, fc.Now().Add(12*time.Hour)) - bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident.Value) + bucketKey, err := ratelimits.BuildBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident, nil, netip.Addr{}) + test.AssertNotError(t, err, "building bucket key") // Set the stored TAT to indicate that this bucket has exhausted its quota. err = rl.BatchSet(context.Background(), map[string]time.Time{ @@ -758,15 +759,17 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t * // mockRLSourceWithSyncDelete is a mock ratelimits.Source that forwards all // method calls to an inner Source, but also performs a blocking write to a -// channel when Delete is called to allow the tests to synchronize. +// channel when BatchDelete is called to allow the tests to synchronize. type mockRLSourceWithSyncDelete struct { ratelimits.Source out chan<- string } -func (rl mockRLSourceWithSyncDelete) Delete(ctx context.Context, bucketKey string) error { - err := rl.Source.Delete(ctx, bucketKey) - rl.out <- bucketKey +func (rl mockRLSourceWithSyncDelete) BatchDelete(ctx context.Context, bucketKeys []string) error { + err := rl.Source.BatchDelete(ctx, bucketKeys) + for _, bucketKey := range bucketKeys { + rl.out <- bucketKey + } return err } @@ -791,7 +794,8 @@ func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersR domain := randomDomain() ident := identifier.NewDNS(domain) authzPB := createPendingAuthorization(t, sa, registration.Id, ident, fc.Now().Add(12*time.Hour)) - bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident.Value) + bucketKey, err := ratelimits.BuildBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident, nil, netip.Addr{}) + test.AssertNotError(t, err, "building bucket key") // Set a stored TAT so that we can tell when it's been reset. err = rl.BatchSet(context.Background(), map[string]time.Time{ diff --git a/ratelimits/gcra_test.go b/ratelimits/gcra_test.go index 7b8cf2bc46b..8c48eb23a92 100644 --- a/ratelimits/gcra_test.go +++ b/ratelimits/gcra_test.go @@ -16,16 +16,16 @@ func TestDecide(t *testing.T) { limit.precompute() // Begin by using 1 of our 10 requests. - d := maybeSpend(clk, Transaction{"test", limit, 1, true, true}, clk.Now()) + d := maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, clk.Now()) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(9)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Second) // Transaction is set when we're allowed. - test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true}) + test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true, false}) // Immediately use another 9 of our remaining requests. - d = maybeSpend(clk, Transaction{"test", limit, 9, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 9, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) // We should have to wait 1 second before we can use another request but we @@ -37,21 +37,21 @@ func TestDecide(t *testing.T) { test.AssertEquals(t, d.newTAT, clk.Now().Add(time.Second*10)) // Let's try using just 1 more request without waiting. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, !d.allowed, "should not be allowed") test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.retryIn, time.Second) test.AssertEquals(t, d.resetIn, time.Second*10) // Transaction is set when we're denied. - test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true}) + test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true, false}) // Let's try being exactly as patient as we're told to be. clk.Add(d.retryIn) - d = maybeSpend(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.AssertEquals(t, d.remaining, int64(1)) // We are 1 second in the future, we should have 1 new request. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.retryIn, time.Second) @@ -61,7 +61,7 @@ func TestDecide(t *testing.T) { clk.Add(d.resetIn) // We should have 10 new requests. If we use 1 we should have 9 remaining. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(9)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -72,7 +72,7 @@ func TestDecide(t *testing.T) { // We should still have 9 remaining because we're still 1ms shy of the // refill time. - d = maybeSpend(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(9)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -83,7 +83,7 @@ func TestDecide(t *testing.T) { clk.Add(20 * time.Hour) // C'mon, big money, no whammies, no whammies, STOP! - d = maybeSpend(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -91,7 +91,7 @@ func TestDecide(t *testing.T) { // Turns out that the most we can accrue is 10 (limit.Burst). Let's empty // this bucket out so we can try something else. - d = maybeSpend(clk, Transaction{"test", limit, 10, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 10, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) // We should have to wait 1 second before we can use another request but we @@ -101,14 +101,14 @@ func TestDecide(t *testing.T) { test.AssertEquals(t, d.resetIn, time.Second*10) // If you spend 0 while you have 0 you should get 0. - d = maybeSpend(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Second*10) // We don't play by the rules, we spend 1 when we have 0. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, !d.allowed, "should not be allowed") test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.retryIn, time.Second) @@ -118,7 +118,7 @@ func TestDecide(t *testing.T) { clk.Add(d.retryIn) // Our patience pays off, we should have 1 new request. Let's use it. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.retryIn, time.Second) @@ -130,7 +130,7 @@ func TestDecide(t *testing.T) { // Attempt to spend 7 when we only have 5. We should be denied but the // decision should reflect a retry of 2 seconds, the time it would take to // refill from 5 to 7. - d = maybeSpend(clk, Transaction{"test", limit, 7, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 7, true, true, false}, d.newTAT) test.Assert(t, !d.allowed, "should not be allowed") test.AssertEquals(t, d.remaining, int64(5)) test.AssertEquals(t, d.retryIn, time.Second*2) @@ -143,28 +143,28 @@ func TestMaybeRefund(t *testing.T) { limit.precompute() // Begin by using 1 of our 10 requests. - d := maybeSpend(clk, Transaction{"test", limit, 1, true, true}, clk.Now()) + d := maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, clk.Now()) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(9)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Second) // Transaction is set when we're refunding. - test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true}) + test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true, false}) // Refund back to 10. - d = maybeRefund(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) // Refund 0, we should still have 10. - d = maybeRefund(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) // Spend 1 more of our 10 requests. - d = maybeSpend(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(9)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -174,16 +174,16 @@ func TestMaybeRefund(t *testing.T) { clk.Add(d.resetIn) // Attempt to refund from 10 to 11. - d = maybeRefund(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, !d.allowed, "should not be allowed") test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) // Transaction is set when our bucket is full. - test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true}) + test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true, false}) // Spend 10 all 10 of our requests. - d = maybeSpend(clk, Transaction{"test", limit, 10, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 10, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(0)) // We should have to wait 1 second before we can use another request but we @@ -193,7 +193,7 @@ func TestMaybeRefund(t *testing.T) { test.AssertEquals(t, d.resetIn, time.Second*10) // Attempt a refund of 10. - d = maybeRefund(clk, Transaction{"test", limit, 10, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 10, true, true, false}, d.newTAT) test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) @@ -202,17 +202,17 @@ func TestMaybeRefund(t *testing.T) { clk.Add(11 * time.Second) // Attempt to refund to 11, then ensure it's still 10. - d = maybeRefund(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, !d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) // Transaction is set when our TAT is in the past. - test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true}) + test.AssertEquals(t, d.transaction, Transaction{"test", limit, 1, true, true, false}) // Spend 5 of our 10 requests, then refund 1. - d = maybeSpend(clk, Transaction{"test", limit, 5, true, true}, d.newTAT) - d = maybeRefund(clk, Transaction{"test", limit, 1, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 5, true, true, false}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 1, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(6)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -221,7 +221,7 @@ func TestMaybeRefund(t *testing.T) { clk.Add(time.Millisecond * 2500) // Ensure we have 8.5 requests. - d = maybeSpend(clk, Transaction{"test", limit, 0, true, true}, d.newTAT) + d = maybeSpend(clk, Transaction{"test", limit, 0, true, true, false}, d.newTAT) test.Assert(t, d.allowed, "should be allowed") test.AssertEquals(t, d.remaining, int64(8)) test.AssertEquals(t, d.retryIn, time.Duration(0)) @@ -229,7 +229,7 @@ func TestMaybeRefund(t *testing.T) { test.AssertEquals(t, d.resetIn, time.Millisecond*1500) // Refund 2 requests, we should only have 10, not 10.5. - d = maybeRefund(clk, Transaction{"test", limit, 2, true, true}, d.newTAT) + d = maybeRefund(clk, Transaction{"test", limit, 2, true, true, false}, d.newTAT) test.AssertEquals(t, d.remaining, int64(10)) test.AssertEquals(t, d.retryIn, time.Duration(0)) test.AssertEquals(t, d.resetIn, time.Duration(0)) diff --git a/ratelimits/limiter.go b/ratelimits/limiter.go index c7e9a039460..ea5c2b64237 100644 --- a/ratelimits/limiter.go +++ b/ratelimits/limiter.go @@ -434,11 +434,29 @@ func (l *Limiter) BatchRefund(ctx context.Context, txns []Transaction) (*Decisio return batchDecision, nil } -// Reset resets the specified bucket to its maximum capacity. The new bucket -// state is persisted to the underlying datastore before returning. -func (l *Limiter) Reset(ctx context.Context, bucketKey string) error { +// BatchReset resets the specified buckets to their maximum capacity using the +// provided reset Transactions. The new bucket state is persisted to the +// underlying datastore before returning. +func (l *Limiter) BatchReset(ctx context.Context, txns []Transaction) error { + var bucketKeys []string + for _, txn := range txns { + if txn.allowOnly() { + // Ignore allow-only transactions. + continue + } + if !txn.resetOnly() { + return fmt.Errorf("found reset-only transaction, received check=%t spend=%t reset=%t", txn.check, txn.spend, txn.reset) + } + if slices.Contains(bucketKeys, txn.bucketKey) { + return fmt.Errorf("found duplicate bucket %q in batch", txn.bucketKey) + } + bucketKeys = append(bucketKeys, txn.bucketKey) + } + if len(bucketKeys) == 0 { + return nil + } // Remove cancellation from the request context so that transactions are not // interrupted by a client disconnect. ctx = context.WithoutCancel(ctx) - return l.source.Delete(ctx, bucketKey) + return l.source.BatchDelete(ctx, bucketKeys) } diff --git a/ratelimits/limiter_test.go b/ratelimits/limiter_test.go index 167cb2920ad..825c6056fcc 100644 --- a/ratelimits/limiter_test.go +++ b/ratelimits/limiter_test.go @@ -59,6 +59,14 @@ func setup(t *testing.T) (context.Context, map[string]*Limiter, *TransactionBuil }, newTestTransactionBuilder(t), clk, randIP.String() } +func mustReset(t *testing.T, l *Limiter, ctx context.Context, limit *Limit, bucketKey string) { + t.Helper() + txn, err := newResetTransaction(limit, bucketKey) + test.AssertNotError(t, err, "txn should be valid") + err = l.BatchReset(ctx, []Transaction{txn}) + test.AssertNotError(t, err, "should not error") +} + func TestLimiter_CheckWithLimitOverrides(t *testing.T) { t.Parallel() testCtx, limiters, txnBuilder, clk, testIP := setup(t) @@ -153,10 +161,8 @@ func TestLimiter_CheckWithLimitOverrides(t *testing.T) { test.AssertEquals(t, d.resetIn, time.Millisecond*50) // Reset between tests. - err = l.Reset(testCtx, overriddenBucketKey) - test.AssertNotError(t, err, "should not error") - err = l.Reset(testCtx, normalBucketKey) - test.AssertNotError(t, err, "should not error") + mustReset(t, l, testCtx, overriddenLimit, overriddenBucketKey) + mustReset(t, l, testCtx, normalLimit, normalBucketKey) // Spend the same bucket but in a batch with a Transaction that is // check-only. This should succeed, but the decision should reflect @@ -238,8 +244,7 @@ func TestLimiter_CheckWithLimitOverrides(t *testing.T) { test.AssertEquals(t, d.resetIn, time.Millisecond*50) // Reset between tests. - err = l.Reset(testCtx, overriddenBucketKey) - test.AssertNotError(t, err, "should not error") + mustReset(t, l, testCtx, overriddenLimit, overriddenBucketKey) }) } } @@ -278,8 +283,7 @@ func TestLimiter_InitializationViaCheckAndSpend(t *testing.T) { test.AssertEquals(t, d.retryIn, time.Duration(0)) // Reset our bucket. - err = l.Reset(testCtx, bucketKey) - test.AssertNotError(t, err, "should not error") + mustReset(t, l, testCtx, limit, bucketKey) // Similar to above, but we'll use Spend() to actually initialize // the bucket. Spend should return the same result as Check. @@ -400,8 +404,7 @@ func TestLimiter_RefundAndReset(t *testing.T) { test.AssertEquals(t, d.remaining, int64(0)) test.AssertEquals(t, d.resetIn, time.Second) - err = l.Reset(testCtx, bucketKey) - test.AssertNotError(t, err, "should not error") + mustReset(t, l, testCtx, limit, bucketKey) // Attempt to spend 20 more requests, this should succeed. d, err = l.Spend(testCtx, txn20) diff --git a/ratelimits/names.go b/ratelimits/names.go index cc32e49b6bf..c31dcf94898 100644 --- a/ratelimits/names.go +++ b/ratelimits/names.go @@ -409,7 +409,7 @@ func BuildBucketKey(name Name, regId int64, singleIdent identifier.ACMEIdentifie if err != nil { return "", err } - return NewRegIdIdentValueBucketKey(name, regId, coveringIdent), nil + return newRegIdIdentValueBucketKey(name, regId, coveringIdent), nil } if regId == 0 { return "", makeMissingErr("regId") @@ -429,7 +429,7 @@ func BuildBucketKey(name Name, regId int64, singleIdent identifier.ACMEIdentifie return "", makeMissingErr("regId") } // Default: use 'enum:regId:identValue' bucket key format. - return NewRegIdIdentValueBucketKey(name, regId, singleIdent.Value), nil + return newRegIdIdentValueBucketKey(name, regId, singleIdent.Value), nil } if regId == 0 { return "", makeMissingErr("regId") diff --git a/ratelimits/source.go b/ratelimits/source.go index 1095f162945..7e070948218 100644 --- a/ratelimits/source.go +++ b/ratelimits/source.go @@ -49,13 +49,13 @@ type Source interface { // the underlying storage client implementation). BatchGet(ctx context.Context, bucketKeys []string) (map[string]time.Time, error) - // Delete removes the TAT associated with the specified bucketKey (formatted - // as 'name:id'). Implementations MUST ensure non-blocking operations by - // either: + // BatchDelete removes the TATs associated with the specified bucketKeys + // (formatted as 'name:id'). Implementations MUST ensure non-blocking + // operations by either: // a) applying a deadline or timeout to the context WITHIN the method, or // b) guaranteeing the operation will not block indefinitely (e.g. via // the underlying storage client implementation). - Delete(ctx context.Context, bucketKey string) error + BatchDelete(ctx context.Context, bucketKeys []string) error } type increment struct { @@ -131,9 +131,11 @@ func (in *inmem) BatchGet(_ context.Context, bucketKeys []string) (map[string]ti return tats, nil } -func (in *inmem) Delete(_ context.Context, bucketKey string) error { +func (in *inmem) BatchDelete(_ context.Context, bucketKeys []string) error { in.Lock() defer in.Unlock() - delete(in.m, bucketKey) + for _, bucketKey := range bucketKeys { + delete(in.m, bucketKey) + } return nil } diff --git a/ratelimits/source_redis.go b/ratelimits/source_redis.go index b2614f41fd5..f15c69da980 100644 --- a/ratelimits/source_redis.go +++ b/ratelimits/source_redis.go @@ -227,12 +227,12 @@ func (r *RedisSource) BatchGet(ctx context.Context, bucketKeys []string) (map[st return tats, nil } -// Delete deletes the TAT at the specified bucketKey ('name:id'). A nil return -// value does not indicate that the bucketKey existed. -func (r *RedisSource) Delete(ctx context.Context, bucketKey string) error { +// BatchDelete deletes the TATs at the specified bucketKeys ('name:id'). A nil +// return value does not indicate that the bucketKeys existed. +func (r *RedisSource) BatchDelete(ctx context.Context, bucketKeys []string) error { start := r.clk.Now() - err := r.client.Del(ctx, bucketKey).Err() + err := r.client.Del(ctx, bucketKeys...).Err() if err != nil { r.observeLatency("delete", r.clk.Since(start), err) return err @@ -242,6 +242,12 @@ func (r *RedisSource) Delete(ctx context.Context, bucketKey string) error { return nil } +// Delete deletes the TAT at the specified bucketKey ('name:id'). A nil return +// value does not indicate that the bucketKey existed. +func (r *RedisSource) Delete(ctx context.Context, bucketKey string) error { + return r.BatchDelete(ctx, []string{bucketKey}) +} + // Ping checks that each shard of the *redis.Ring is reachable using the PING // command. func (r *RedisSource) Ping(ctx context.Context) error { diff --git a/ratelimits/transaction.go b/ratelimits/transaction.go index 6f60cdb64e2..feb9ab6f5d9 100644 --- a/ratelimits/transaction.go +++ b/ratelimits/transaction.go @@ -52,10 +52,9 @@ func newDomainOrCIDRBucketKey(name Name, domainOrCIDR string) string { return joinWithColon(name.EnumString(), domainOrCIDR) } -// NewRegIdIdentValueBucketKey returns a bucketKey for limits that use the -// 'enum:regId:identValue' bucket key format. This function is exported for use -// by the RA when resetting the account pausing limit. -func NewRegIdIdentValueBucketKey(name Name, regId int64, orderIdent string) string { +// newRegIdIdentValueBucketKey returns a bucketKey for limits that use the +// 'enum:regId:identValue' bucket key format. +func newRegIdIdentValueBucketKey(name Name, regId int64, orderIdent string) string { return joinWithColon(name.EnumString(), strconv.FormatInt(regId, 10), orderIdent) } @@ -80,6 +79,7 @@ func newFQDNSetBucketKey(name Name, orderIdents identifier.ACMEIdentifiers) stri // - allow-only: when neither check nor spend are true, the transaction will // be considered "allowed" regardless of the bucket's capacity. This is // useful for limits that are disabled. +// - reset: when reset is true, the bucket will be reset to full capacity. // // The zero value of Transaction is an allow-only transaction and is valid even if // it would fail validateTransaction (for instance because cost and burst are zero). @@ -89,21 +89,38 @@ type Transaction struct { cost int64 check bool spend bool + reset bool } func (txn Transaction) checkOnly() bool { - return txn.check && !txn.spend + return txn.check && !txn.spend && !txn.reset } func (txn Transaction) spendOnly() bool { - return txn.spend && !txn.check + return txn.spend && !txn.check && !txn.reset } func (txn Transaction) allowOnly() bool { - return !txn.check && !txn.spend + return !txn.check && !txn.spend && !txn.reset +} + +func (txn Transaction) resetOnly() bool { + return txn.reset && !txn.check && !txn.spend } func validateTransaction(txn Transaction) (Transaction, error) { + if txn.limit == nil { + return Transaction{}, fmt.Errorf("invalid limit, must not be nil") + } + if txn.reset { + if txn.check || txn.spend { + return Transaction{}, fmt.Errorf("invalid reset transaction, check and spend must be false") + } + if txn.limit.Burst == 0 { + return Transaction{}, fmt.Errorf("invalid limit, burst must be > 0") + } + return txn, nil + } if txn.cost < 0 { return Transaction{}, ErrInvalidCost } @@ -149,6 +166,14 @@ func newSpendOnlyTransaction(limit *Limit, bucketKey string, cost int64) (Transa }) } +func newResetTransaction(limit *Limit, bucketKey string) (Transaction, error) { + return validateTransaction(Transaction{ + bucketKey: bucketKey, + limit: limit, + reset: true, + }) +} + func newAllowOnlyTransaction() Transaction { // Zero values are sufficient. return Transaction{} @@ -380,7 +405,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountCheckO for _, ident := range orderIdents { // FailedAuthorizationsPerDomainPerAccount limit uses the // 'enum:regId:identValue' bucket key format for transactions. - perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, ident.Value) + perIdentValuePerAccountBucketKey := newRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, ident.Value) // Add a check-only transaction for each per identValue per account // bucket. @@ -411,7 +436,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountSpendO // FailedAuthorizationsPerDomainPerAccount limit uses the // 'enum:regId:identValue' bucket key format for transactions. - perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, orderIdent.Value) + perIdentValuePerAccountBucketKey := newRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, orderIdent.Value) txn, err := newSpendOnlyTransaction(limit, perIdentValuePerAccountBucketKey, 1) if err != nil { return Transaction{}, err @@ -438,7 +463,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsForPausingPerDomainPerAcc // FailedAuthorizationsForPausingPerDomainPerAccount limit uses the // 'enum:regId:identValue' bucket key format for transactions. - perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId, orderIdent.Value) + perIdentValuePerAccountBucketKey := newRegIdIdentValueBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId, orderIdent.Value) txn, err := newTransaction(limit, perIdentValuePerAccountBucketKey, 1) if err != nil { return Transaction{}, err @@ -490,7 +515,7 @@ func (builder *TransactionBuilder) certificatesPerDomainCheckOnlyTransactions(re if !perAccountLimit.isOverride { return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override") } - perAccountPerDomainOrCIDRBucketKey := NewRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident) + perAccountPerDomainOrCIDRBucketKey := newRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident) // Add a check-only transaction for each per account per identValue // bucket. txn, err := newCheckOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRBucketKey, 1) @@ -569,7 +594,7 @@ func (builder *TransactionBuilder) CertificatesPerDomainSpendOnlyTransactions(re if !perAccountLimit.isOverride { return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override") } - perAccountPerDomainOrCIDRBucketKey := NewRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident) + perAccountPerDomainOrCIDRBucketKey := newRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident) // Add a spend-only transaction for each per account per // domainOrCIDR bucket. txn, err := newSpendOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRBucketKey, 1) @@ -711,6 +736,25 @@ func (builder *TransactionBuilder) NewAccountLimitTransactions(ip netip.Addr) ([ return append(transactions, txn), nil } +func (builder *TransactionBuilder) NewPausingResetTransactions(regId int64, orderIdent identifier.ACMEIdentifier) ([]Transaction, error) { + perAccountBucketKey := newRegIdBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId) + limit, err := builder.getLimit(FailedAuthorizationsForPausingPerDomainPerAccount, perAccountBucketKey) + if err != nil { + if errors.Is(err, errLimitDisabled) { + return []Transaction{newAllowOnlyTransaction()}, nil + } + return nil, err + } + + perIdentValuePerAccountBucketKey := newRegIdIdentValueBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId, orderIdent.Value) + txn, err := newResetTransaction(limit, perIdentValuePerAccountBucketKey) + if err != nil { + return nil, err + } + + return []Transaction{txn}, nil +} + // LimitOverrideRequestsPerIPAddressTransaction returns a Transaction for the // LimitOverrideRequestsPerIPAddress limit for the provided IP address. This // limit is used to rate limit requests to the SFE override request endpoint.