diff --git a/ra/ra.go b/ra/ra.go index ee264258a..afbce312d 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -532,16 +532,19 @@ func (ra *RegistrationAuthorityImpl) validateContacts(contacts []string) error { func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context.Context, regID int64) error { limit := ra.rlPolicies.PendingAuthorizationsPerAccount() if limit.Enabled() { + // This rate limit's threshold can only be overridden on a per-regID basis, + // not based on any other key. + threshold := limit.GetThreshold("", regID) + if threshold == -1 { + return nil + } countPB, err := ra.SA.CountPendingAuthorizations2(ctx, &sapb.RegistrationID{ Id: regID, }) if err != nil { return err } - // Most rate limits have a key for overrides, but there is no meaningful key - // here. - noKey := "" - if countPB.Count >= limit.GetThreshold(noKey, regID) { + if countPB.Count >= threshold { ra.rateLimitCounter.WithLabelValues("pending_authorizations_by_registration_id", "exceeded").Inc() ra.log.Infof("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID) return berrors.RateLimitError(0, "too many currently pending authorizations: %d", countPB.Count) diff --git a/ra/ra_test.go b/ra/ra_test.go index d6bc3bcb0..595d5084a 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -2127,6 +2127,38 @@ func TestNewOrderReuseInvalidAuthz(t *testing.T) { test.AssertNotEquals(t, secondOrder.V2Authorizations[0], order.V2Authorizations[0]) } +// mockSACountPendingFails has a CountPendingAuthorizations2 implementation +// that always returns error +type mockSACountPendingFails struct { + mocks.StorageAuthority +} + +func (mock *mockSACountPendingFails) CountPendingAuthorizations2(ctx context.Context, req *sapb.RegistrationID, _ ...grpc.CallOption) (*sapb.Count, error) { + return nil, errors.New("counting is slow and boring") +} + +// Ensure that we don't bother to call the SA to count pending authorizations +// when an "unlimited" limit is set. +func TestPendingAuthorizationsUnlimited(t *testing.T) { + _, _, ra, _, cleanUp := initAuthorities(t) + defer cleanUp() + + ra.rlPolicies = &dummyRateLimitConfig{ + PendingAuthorizationsPerAccountPolicy: ratelimit.RateLimitPolicy{ + Threshold: 1, + Window: cmd.ConfigDuration{Duration: 24 * time.Hour}, + RegistrationOverrides: map[int64]int64{ + 13: -1, + }, + }, + } + + ra.SA = &mockSACountPendingFails{} + + err := ra.checkPendingAuthorizationLimit(context.Background(), 13) + test.AssertNotError(t, err, "checking pending authorization limit") +} + // Test that the failed authorizations limit is checked before authz reuse. func TestNewOrderCheckFailedAuthorizationsFirst(t *testing.T) { _, _, ra, _, cleanUp := initAuthorities(t) diff --git a/ratelimit/rate-limits.go b/ratelimit/rate-limits.go index 822f820cc..c1f084730 100644 --- a/ratelimit/rate-limits.go +++ b/ratelimit/rate-limits.go @@ -185,7 +185,8 @@ type RateLimitPolicy struct { // For instance, a rate limit on the number of certificates per name uses name as // a key, while a rate limit on the number of registrations per IP subnet would // use subnet as a key. Note that a zero entry in the overrides map does not - // mean "no limit," it means a limit of zero. + // mean "no limit," it means a limit of zero. An entry of -1 means + // "no limit", only for the pending authorizations rate limit. Overrides map[string]int64 `yaml:"overrides"` // A per-registration override setting. This can be used, e.g. if there are // hosting providers that we would like to grant a higher rate of issuance