letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove TooManyCertificatesError. (#3228) 

When counting certificates for rate limiting, we attempted to impose a limit on
the query results to avoid we did not receive so many results that they caused
slowness on the database or SA side. However, that check has never actually been
executed correctly. The check was fixed in #3126, but rolling out that fix
broke issuance for subscribers with rate limit overrides that have allowed them
to exceed the limit.

Because this limit has not been needed in practice over the years, remove it
rather than refining it. The size of the results are loosely governed by our
rate limits (and overrides), and if result sizes from this query become a
performance issue in the future, we can address it then. For now, opt for
simplification.

Fixes #3214.
This commit is contained in:
Jacob Hoffman-Andrews 2017-11-09 06:11:45 -08:00 committed by Daniel McCarney
parent 60ca8febb3
commit ef0bf7e9d0
1 changed files with 2 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
sa/sa.go
View File

@ -325,21 +325,10 @@ func (ssa *SQLStorageAuthority) CountRegistrationsByIPRange(ctx context.Context,
return int(count), nil
}
// TooManyCertificatesError indicates that the number of certificates returned by
// CountCertificates exceeded the hard-coded limit of 10,000 certificates.
type TooManyCertificatesError string
func (t TooManyCertificatesError) Error() string {
return string(t)
}
// CountCertificatesByNames counts, for each input domain, the number of
// certificates issued in the given time range for that domain and its
// subdomains. It returns a map from domains to counts, which is guaranteed to
// contain an entry for each input domain, so long as err is nil.
// The highest count this function can return is 10,000. If there are more
// certificates than that matching one of the provided domain names, it will return
// TooManyCertificatesError.
// Queries will be run in parallel. If any of them error, only one error will
// be returned.
func (ssa *SQLStorageAuthority) CountCertificatesByNames(ctx context.Context, domains []string, earliest, latest time.Time) ([]*sapb.CountByNames_MapElement, error) {
@ -431,14 +420,12 @@ const countCertificatesSelect = `
SELECT serial from issuedNames
WHERE (reversedName = :reversedDomain OR
reversedName LIKE CONCAT(:reversedDomain, ".%"))
AND notBefore > :earliest AND notBefore <= :latest
LIMIT :limit;`
AND notBefore > :earliest AND notBefore <= :latest;`
const countCertificatesExactSelect = `
SELECT serial from issuedNames
WHERE reversedName = :reversedDomain
AND notBefore > :earliest AND notBefore <= :latest
LIMIT :limit;`
AND notBefore > :earliest AND notBefore <= :latest;`
// countCertificatesByNames returns, for a single domain, the count of
// certificates issued in the given time range for that domain and its
@ -460,13 +447,7 @@ func (ssa *SQLStorageAuthority) countCertificatesByExactName(domain string, earl
// `countCertificatesSelect`. If the `AllowRenewalFirstRL` feature flag is set,
// renewals of certificates issued within the same window are considered "free"
// and are not counted.
//
// The highest count this function can return is 10,000. If there are more
// certificates than that matching one of the provided domain names, it will return
// TooManyCertificatesError.
func (ssa *SQLStorageAuthority) countCertificates(domain string, earliest, latest time.Time, query string) (int, error) {
var count int64
const max = 10000
var serials []string
_, err := ssa.dbMap.Select(
&serials,
@ -475,14 +456,11 @@ func (ssa *SQLStorageAuthority) countCertificates(domain string, earliest, lates
"reversedDomain": ReverseName(domain),
"earliest": earliest,
"latest": latest,
"limit": max + 1,
})
if err == sql.ErrNoRows {
return 0, nil
} else if err != nil {
return 0, err
} else if count > max {
return max, TooManyCertificatesError(fmt.Sprintf("More than %d issuedName entries for %s.", max, domain))
}
// If the `AllowRenewalFirstRL` feature flag is enabled then do the work