Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ratelimits: Update errors to deep link to individual limits documentation #7767

Merged
merged 1 commit into from
Oct 25, 2024
Merged

ratelimits: Update errors to deep link to individual limits documentation #7767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,26 +179,50 @@ func RateLimitError(retryAfter time.Duration, msg string, args ...interface{}) e
}
}

func DuplicateCertificateError(retryAfter time.Duration, msg string, args ...interface{}) error {
func RegistrationsPerIPAddressError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/duplicate-certificate-limit/", args...),
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ip-address", args...),
RetryAfter: retryAfter,
}
}

func FailedValidationError(retryAfter time.Duration, msg string, args ...interface{}) error {
func RegistrationsPerIPv6RangeError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/failed-validation-limit/", args...),
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ipv6-range", args...),
RetryAfter: retryAfter,
}
}

func RegistrationsPerIPError(retryAfter time.Duration, msg string, args ...interface{}) error {
func NewOrdersPerAccountError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/", args...),
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account", args...),
RetryAfter: retryAfter,
}
}

func CertificatesPerDomainError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-registered-domain", args...),
RetryAfter: retryAfter,
}
}

func CertificatesPerFQDNSetError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-exact-set-of-hostnames", args...),
RetryAfter: retryAfter,
}
}

func FailedAuthorizationsPerDomainPerAccountError(retryAfter time.Duration, msg string, args ...interface{}) error {
return &BoulderError{
Type: RateLimit,
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/#authorization-failures-per-hostname-per-account", args...),
RetryAfter: retryAfter,
}
}
Expand Down
14 changes: 7 additions & 7 deletions ra/ra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Contex

threshold, overrideKey := limit.GetThreshold(ip.String(), noRegistrationID)
if count.Count >= threshold {
return berrors.RegistrationsPerIPError(0, "too many registrations for this IP")
return berrors.RegistrationsPerIPAddressError(0, "too many registrations for this IP")
}
if overrideKey != "" {
// We do not support overrides for the NewRegistrationsPerIPRange limit.
Expand Down Expand Up @@ -598,7 +598,7 @@ func (ra *RegistrationAuthorityImpl) checkInvalidAuthorizationLimit(ctx context.
threshold, overrideKey := limit.GetThreshold(noKey, regID)
if count.Count >= threshold {
ra.log.Infof("Rate limit exceeded, InvalidAuthorizationsByRegID, regID: %d", regID)
return berrors.FailedValidationError(0, "too many failed authorizations recently")
return berrors.FailedAuthorizationsPerDomainPerAccountError(0, "too many failed authorizations recently")
}
if overrideKey != "" {
utilization := float64(count.Count) / float64(threshold)
Expand Down Expand Up @@ -626,7 +626,7 @@ func (ra *RegistrationAuthorityImpl) checkNewOrdersPerAccountLimit(ctx context.C
noKey := ""
threshold, overrideKey := limit.GetThreshold(noKey, acctID)
if count.Count >= threshold {
return berrors.RateLimitError(0, "too many new orders recently")
return berrors.NewOrdersPerAccountError(0, "too many new orders recently")
}
if overrideKey != "" {
utilization := float64(count.Count+1) / float64(threshold)
Expand Down Expand Up @@ -1473,12 +1473,12 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
for _, name := range namesOutOfLimit {
subErrors = append(subErrors, berrors.SubBoulderError{
Identifier: identifier.NewDNS(name),
BoulderError: berrors.RateLimitError(retryAfter, "too many certificates already issued. Retry after %s", retryString).(*berrors.BoulderError),
BoulderError: berrors.NewOrdersPerAccountError(retryAfter, "too many certificates already issued. Retry after %s", retryString).(*berrors.BoulderError),
})
}
return berrors.RateLimitError(retryAfter, "too many certificates already issued for multiple names (%q and %d others). Retry after %s", namesOutOfLimit[0], len(namesOutOfLimit), retryString).(*berrors.BoulderError).WithSubErrors(subErrors)
return berrors.NewOrdersPerAccountError(retryAfter, "too many certificates already issued for multiple names (%q and %d others). Retry after %s", namesOutOfLimit[0], len(namesOutOfLimit), retryString).(*berrors.BoulderError).WithSubErrors(subErrors)
}
return berrors.RateLimitError(retryAfter, "too many certificates already issued for %q. Retry after %s", namesOutOfLimit[0], retryString)
return berrors.NewOrdersPerAccountError(retryAfter, "too many certificates already issued for %q. Retry after %s", namesOutOfLimit[0], retryString)
}

return nil
Expand Down Expand Up @@ -1535,7 +1535,7 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
}
retryTime := prevIssuances.Timestamps[0].AsTime().Add(time.Duration(nsPerToken))
retryAfter := retryTime.Sub(now)
return berrors.DuplicateCertificateError(
return berrors.CertificatesPerFQDNSetError(
retryAfter,
"too many certificates (%d) already issued for this exact set of domains in the last %.0f hours: %s, retry after %s",
threshold, limit.Window.Duration.Hours(), strings.Join(names, ","), retryTime.Format(time.RFC3339),
Expand Down
14 changes: 7 additions & 7 deletions ra/ra_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ func TestEarlyOrderRateLimiting(t *testing.T) {
test.AssertEquals(t, bErr.RetryAfter, rateLimitDuration)

// The err should be the expected rate limit error
expected := "too many certificates already issued for \"early-ratelimit-example.com\". Retry after 2020-03-04T05:05:00Z: see https://letsencrypt.org/docs/rate-limits/"
expected := "too many certificates already issued for \"early-ratelimit-example.com\". Retry after 2020-03-04T05:05:00Z: see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account"
test.AssertEquals(t, bErr.Error(), expected)
}

Expand Down Expand Up @@ -1048,7 +1048,7 @@ func TestAuthzFailedRateLimitingNewOrder(t *testing.T) {
err := ra.checkInvalidAuthorizationLimits(ctx, Registration.Id,
[]string{"charlie.brown.com", "all.i.do.is.lose.com"}, limit)
test.AssertError(t, err, "checkInvalidAuthorizationLimits did not encounter expected rate limit error")
test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/failed-validation-limit/")
test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/rate-limits/#authorization-failures-per-hostname-per-account")
}

type mockSAWithNameCounts struct {
Expand Down Expand Up @@ -1117,7 +1117,7 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
// should contain 0 entries with labels matching it.
test.AssertMetricWithLabelsEquals(t, ra.rlOverrideUsageGauge, prometheus.Labels{"limit": ratelimit.CertificatesPerName, "override_key": "example.com"}, 0)
// Verify it has no sub errors as there is only one bad name
test.AssertEquals(t, err.Error(), "too many certificates already issued for \"example.com\". Retry after 1970-01-01T23:00:00Z: see https://letsencrypt.org/docs/rate-limits/")
test.AssertEquals(t, err.Error(), "too many certificates already issued for \"example.com\". Retry after 1970-01-01T23:00:00Z: see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account")
var bErr *berrors.BoulderError
test.AssertErrorWraps(t, err, &bErr)
test.AssertEquals(t, len(bErr.SubErrors), 0)
Expand All @@ -1130,7 +1130,7 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
test.AssertError(t, err, "incorrectly failed to rate limit example.com, other-example.com")
test.AssertErrorIs(t, err, berrors.RateLimit)
// Verify it has two sub errors as there are two bad names
test.AssertEquals(t, err.Error(), "too many certificates already issued for multiple names (\"example.com\" and 2 others). Retry after 1970-01-01T23:00:00Z: see https://letsencrypt.org/docs/rate-limits/")
test.AssertEquals(t, err.Error(), "too many certificates already issued for multiple names (\"example.com\" and 2 others). Retry after 1970-01-01T23:00:00Z: see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account")
test.AssertErrorWraps(t, err, &bErr)
test.AssertEquals(t, len(bErr.SubErrors), 2)

Expand Down Expand Up @@ -1241,15 +1241,15 @@ func TestCheckExactCertificateLimit(t *testing.T) {
Name: "FQDN set issuances above limit NS",
Domain: "over.example.com",
ExpectedErr: fmt.Errorf(
"too many certificates (3) already issued for this exact set of domains in the last 24 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/duplicate-certificate-limit/",
"too many certificates (3) already issued for this exact set of domains in the last 24 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-exact-set-of-hostnames",
expectRetryAfterNS,
),
},
{
Name: "FQDN set issuances above limit",
Domain: "over.example.com",
ExpectedErr: fmt.Errorf(
"too many certificates (3) already issued for this exact set of domains in the last 24 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/duplicate-certificate-limit/",
"too many certificates (3) already issued for this exact set of domains in the last 24 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-exact-set-of-hostnames",
expectRetryAfter,
),
},
Expand Down Expand Up @@ -2154,7 +2154,7 @@ func TestNewOrderCheckFailedAuthorizationsFirst(t *testing.T) {
})

test.AssertError(t, err, "expected error for domain with too many failures")
test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/failed-validation-limit/")
test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/rate-limits/#authorization-failures-per-hostname-per-account")
}

// mockSAWithAuthzs has a GetAuthorizations2 method that returns the protobuf
Expand Down
14 changes: 7 additions & 7 deletions ratelimits/limiter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (d *Decision) Result(now time.Time) error {

switch d.transaction.limit.name {
case NewRegistrationsPerIPAddress:
return berrors.RegistrationsPerIPError(
return berrors.RegistrationsPerIPAddressError(
retryAfter,
"too many new registrations (%d) from this IP address in the last %s, retry after %s",
d.transaction.limit.Burst,
Expand All @@ -121,15 +121,15 @@ func (d *Decision) Result(now time.Time) error {
)

case NewRegistrationsPerIPv6Range:
return berrors.RateLimitError(
return berrors.RegistrationsPerIPv6RangeError(
retryAfter,
"too many new registrations (%d) from this /48 block of IPv6 addresses in the last %s, retry after %s",
"too many new registrations (%d) from this /48 subnet of IPv6 addresses in the last %s, retry after %s",
d.transaction.limit.Burst,
d.transaction.limit.Period.Duration,
retryAfterTs,
)
case NewOrdersPerAccount:
return berrors.RateLimitError(
return berrors.NewOrdersPerAccountError(
retryAfter,
"too many new orders (%d) from this account in the last %s, retry after %s",
d.transaction.limit.Burst,
Expand All @@ -144,7 +144,7 @@ func (d *Decision) Result(now time.Time) error {
return berrors.InternalServerError("unrecognized bucket key while generating error")
}
domain := d.transaction.bucketKey[idx+1:]
return berrors.FailedValidationError(
return berrors.FailedAuthorizationsPerDomainPerAccountError(
retryAfter,
"too many failed authorizations (%d) for %q in the last %s, retry after %s",
d.transaction.limit.Burst,
Expand All @@ -160,7 +160,7 @@ func (d *Decision) Result(now time.Time) error {
return berrors.InternalServerError("unrecognized bucket key while generating error")
}
domain := d.transaction.bucketKey[idx+1:]
return berrors.RateLimitError(
return berrors.CertificatesPerDomainError(
retryAfter,
"too many certificates (%d) already issued for %q in the last %s, retry after %s",
d.transaction.limit.Burst,
Expand All @@ -170,7 +170,7 @@ func (d *Decision) Result(now time.Time) error {
)

case CertificatesPerFQDNSet:
return berrors.DuplicateCertificateError(
return berrors.CertificatesPerFQDNSetError(
retryAfter,
"too many certificates (%d) already issued for this exact set of domains in the last %s, retry after %s",
d.transaction.limit.Burst,
Expand Down
28 changes: 22 additions & 6 deletions ratelimits/limiter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ func TestRateLimitError(t *testing.T) {
},
},
},
expectedErr: "too many new registrations (10) from this IP address in the last 1h0m0s, retry after 1970-01-01 00:00:05 UTC",
expectedErr: "too many new registrations (10) from this IP address in the last 1h0m0s, retry after 1970-01-01 00:00:05 UTC: see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ip-address",
expectedErrType: berrors.RateLimit,
},
{
Expand All @@ -505,7 +505,23 @@ func TestRateLimitError(t *testing.T) {
},
},
},
expectedErr: "too many new registrations (5) from this /48 block of IPv6 addresses in the last 1h0m0s, retry after 1970-01-01 00:00:10 UTC",
expectedErr: "too many new registrations (5) from this /48 subnet of IPv6 addresses in the last 1h0m0s, retry after 1970-01-01 00:00:10 UTC: see https://letsencrypt.org/docs/rate-limits/#new-registrations-per-ipv6-range",
expectedErrType: berrors.RateLimit,
},
{
name: "NewOrdersPerAccount limit reached",
decision: &Decision{
allowed: false,
retryIn: 10 * time.Second,
transaction: Transaction{
limit: limit{
name: NewOrdersPerAccount,
Burst: 2,
Period: config.Duration{Duration: time.Hour},
},
},
},
expectedErr: "too many new orders (2) from this account in the last 1h0m0s, retry after 1970-01-01 00:00:10 UTC: see https://letsencrypt.org/docs/rate-limits/#new-orders-per-account",
expectedErrType: berrors.RateLimit,
},
{
Expand All @@ -522,7 +538,7 @@ func TestRateLimitError(t *testing.T) {
bucketKey: "4:12345:example.com",
},
},
expectedErr: "too many failed authorizations (7) for \"example.com\" in the last 1h0m0s, retry after 1970-01-01 00:00:15 UTC",
expectedErr: "too many failed authorizations (7) for \"example.com\" in the last 1h0m0s, retry after 1970-01-01 00:00:15 UTC: see https://letsencrypt.org/docs/rate-limits/#authorization-failures-per-hostname-per-account",
expectedErrType: berrors.RateLimit,
},
{
Expand All @@ -539,7 +555,7 @@ func TestRateLimitError(t *testing.T) {
bucketKey: "5:example.org",
},
},
expectedErr: "too many certificates (3) already issued for \"example.org\" in the last 1h0m0s, retry after 1970-01-01 00:00:20 UTC",
expectedErr: "too many certificates (3) already issued for \"example.org\" in the last 1h0m0s, retry after 1970-01-01 00:00:20 UTC: see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-registered-domain",
expectedErrType: berrors.RateLimit,
},
{
Expand All @@ -556,7 +572,7 @@ func TestRateLimitError(t *testing.T) {
bucketKey: "6:12345678:example.net",
},
},
expectedErr: "too many certificates (3) already issued for \"example.net\" in the last 1h0m0s, retry after 1970-01-01 00:00:20 UTC",
expectedErr: "too many certificates (3) already issued for \"example.net\" in the last 1h0m0s, retry after 1970-01-01 00:00:20 UTC: see https://letsencrypt.org/docs/rate-limits/#new-certificates-per-registered-domain",
expectedErrType: berrors.RateLimit,
},
{
Expand All @@ -583,7 +599,7 @@ func TestRateLimitError(t *testing.T) {
test.AssertNotError(t, err, "expected no error")
} else {
test.AssertError(t, err, "expected an error")
test.AssertContains(t, err.Error(), tc.expectedErr)
test.AssertEquals(t, err.Error(), tc.expectedErr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't love using AssertEquals for error strings, it makes us a bit too precise imo. I'd prefer that the test case have two fields expectedErr and expectedLink, and assert that the resulting message contains both of them. But that's a super minor thing.

test.AssertErrorIs(t, err, tc.expectedErrType)
}
})
Expand Down