Skip to content

Commit

Permalink
RA: Return retry-after when Certificates per Registered Domain is exc…
Browse files Browse the repository at this point in the history
…eeded (#6470)

Have the database query return timestamps for when certificates
were issued for certain names. Use that information to compute
when the next time that name will be eligible for issuance again.
Include that timestamp in the error message and a Retry-After
HTTP header.

Fixes #6465
  • Loading branch information
beautifulentropy authored Nov 1, 2022
1 parent 2af11e4 commit 6838a2c
Show file tree
Hide file tree
Showing 8 changed files with 620 additions and 565 deletions.
21 changes: 13 additions & 8 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ func domainsForRateLimiting(names []string) []string {
// for each of the names. If the count for any of the names exceeds the limit
// for the given registration then the names out of policy are returned to be
// used for a rate limit error.
func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, names []string, limit ratelimit.RateLimitPolicy, regID int64) ([]string, error) {
func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, names []string, limit ratelimit.RateLimitPolicy, regID int64) ([]string, time.Time, error) {
now := ra.clk.Now()
req := &sapb.CountCertificatesByNamesRequest{
Names: names,
Expand All @@ -1289,11 +1289,11 @@ func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, name

response, err := ra.SA.CountCertificatesByNames(ctx, req)
if err != nil {
return nil, err
return nil, time.Time{}, err
}

if len(response.Counts) == 0 {
return nil, errIncompleteGRPCResponse
return nil, time.Time{}, errIncompleteGRPCResponse
}

var badNames []string
Expand All @@ -1305,7 +1305,7 @@ func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, name
badNames = append(badNames, name)
}
}
return badNames, nil
return badNames, response.Earliest.AsTime(), nil
}

func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.Context, names []string, limit ratelimit.RateLimitPolicy, regID int64) error {
Expand All @@ -1322,7 +1322,7 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
}

tldNames := domainsForRateLimiting(names)
namesOutOfLimit, err := ra.enforceNameCounts(ctx, tldNames, limit, regID)
namesOutOfLimit, earliest, err := ra.enforceNameCounts(ctx, tldNames, limit, regID)
if err != nil {
return fmt.Errorf("checking certificates per name limit for %q: %s",
names, err)
Expand All @@ -1341,19 +1341,24 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
return nil
}

// Determine the amount of time until the earliest event would fall out
// of the window.
retryAfter := earliest.Add(limit.Window.Duration).Sub(ra.clk.Now())
retryString := earliest.Add(limit.Window.Duration).Format(time.RFC3339)

ra.log.Infof("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, strings.Join(namesOutOfLimit, ", "))
ra.rateLimitCounter.WithLabelValues("certificates_for_domain", "exceeded").Inc()
if len(namesOutOfLimit) > 1 {
var subErrors []berrors.SubBoulderError
for _, name := range namesOutOfLimit {
subErrors = append(subErrors, berrors.SubBoulderError{
Identifier: identifier.DNSIdentifier(name),
BoulderError: berrors.RateLimitError(0, "too many certificates already issued").(*berrors.BoulderError),
BoulderError: berrors.RateLimitError(retryAfter, "too many certificates already issued. Retry after %s", retryString).(*berrors.BoulderError),
})
}
return berrors.RateLimitError(0, "too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
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.RateLimitError(0, "too many certificates already issued for: %s", namesOutOfLimit[0])
return berrors.RateLimitError(retryAfter, "too many certificates already issued for %q. Retry after %s", namesOutOfLimit[0], retryString)
}
ra.rateLimitCounter.WithLabelValues("certificates_for_domain", "pass").Inc()

Expand Down
15 changes: 8 additions & 7 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,12 +1010,13 @@ func TestEarlyOrderRateLimiting(t *testing.T) {
_, err := ra.NewOrder(ctx, newOrder)
test.AssertError(t, err, "NewOrder did not apply cert rate limits with feature flag enabled")

var bErr *berrors.BoulderError
test.Assert(t, errors.As(err, &bErr), "NewOrder did not return a boulder error")
test.AssertEquals(t, bErr.RetryAfter, rateLimitDuration)

// The err should be the expected rate limit error
expectedErrPrefix := "too many certificates already issued for: " +
"early-ratelimit-example.com"
test.Assert(t,
strings.HasPrefix(err.Error(), expectedErrPrefix),
fmt.Sprintf("expected error to have prefix %q got %q", expectedErrPrefix, err))
expected := "too many certificates already issued for \"early-ratelimit-example.com\". Retry after 2015-03-04T05:05:00Z: see https://letsencrypt.org/docs/rate-limits/"
test.AssertEquals(t, bErr.Error(), expected)
}

func TestAuthzFailedRateLimitingNewOrder(t *testing.T) {
Expand Down Expand Up @@ -1169,7 +1170,7 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
test.AssertError(t, err, "incorrectly failed to rate limit example.com")
test.AssertErrorIs(t, err, berrors.RateLimit)
// 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: 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/")
var bErr *berrors.BoulderError
test.AssertErrorWraps(t, err, &bErr)
test.AssertEquals(t, len(bErr.SubErrors), 0)
Expand All @@ -1182,7 +1183,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): 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/")
test.AssertErrorWraps(t, err, &bErr)
test.AssertEquals(t, len(bErr.SubErrors), 2)

Expand Down
Loading

0 comments on commit 6838a2c

Please sign in to comment.