Use sub-problems for the certificates per name rate limit (#4416)

Fixes #4360.
This commit is contained in:
Roland Bracewell Shoemaker 2019-09-09 09:20:05 -07:00 committed by GitHub
parent f02e9da38f
commit 9df9c21ddc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 8 deletions

View File

@ -1427,13 +1427,20 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
return nil
}
}
domains := strings.Join(namesOutOfLimit, ", ")
ra.log.Infof("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, strings.Join(namesOutOfLimit, ", "))
ra.certsForDomainStats.Inc("Exceeded", 1)
ra.log.Infof("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, domains)
return berrors.RateLimitError(
"too many certificates already issued for: %s",
domains,
)
if len(namesOutOfLimit) > 1 {
var subErrors []berrors.SubBoulderError
for _, name := range namesOutOfLimit {
subErrors = append(subErrors, berrors.SubBoulderError{
Identifier: identifier.DNSIdentifier(name),
BoulderError: berrors.RateLimitError("too many certificates already issued").(*berrors.BoulderError),
})
}
return berrors.RateLimitError("too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
}
return berrors.RateLimitError("too many certificates already issued for: %s", namesOutOfLimit[0])
}
ra.certsForDomainStats.Inc("Pass", 1)

View File

@ -1400,13 +1400,30 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
err := ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "example.com"}, rlp, 99)
test.AssertNotError(t, err, "rate limited example.com incorrectly")
// One base domain, above threshold
// Two base domains, one above threshold, one below
mockSA.nameCounts["example.com"] = nameCount("example.com", 10)
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "example.com"}, rlp, 99)
mockSA.nameCounts["good-example.com"] = nameCount("good-example.com", 1)
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "example.com", "good-example.com"}, rlp, 99)
test.AssertError(t, err, "incorrectly failed to rate limit example.com")
if !berrors.Is(err, berrors.RateLimit) {
t.Errorf("Incorrect error type %#v", err)
}
// 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, len(err.(*berrors.BoulderError).SubErrors), 0)
// Three base domains, two above threshold, one below
mockSA.nameCounts["example.com"] = nameCount("example.com", 10)
mockSA.nameCounts["other-example.com"] = nameCount("other-example.com", 10)
mockSA.nameCounts["good-example.com"] = nameCount("good-example.com", 1)
err = ra.checkCertificatesPerNameLimit(ctx, []string{"example.com", "other-example.com", "good-example.com"}, rlp, 99)
test.AssertError(t, err, "incorrectly failed to rate limit example.com, other-example.com")
if !berrors.Is(err, berrors.RateLimit) {
t.Errorf("Incorrect error type %#v", err)
}
// 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, len(err.(*berrors.BoulderError).SubErrors), 2)
// SA misbehaved and didn't send back a count for every input name
err = ra.checkCertificatesPerNameLimit(ctx, []string{"zombo.com", "www.example.com", "example.com"}, rlp, 99)