Improve renewal rate limiting (#2832)

As described in Boulder issue #2800 the implementation of the SA's
`countCertificates` function meant that the renewal exemption for the
Certificates Per Domain rate limit was difficult to work with. To
maximize allotted certificates clients were required to perform all new
issuances first, followed by the "free" renewals. This arrangement was
difficult to coordinate.

In this PR `countCertificates` is updated such that renewals are
excluded from the count reliably. To do so the SA takes the serials it
finds for a given domain from the issuedNames table and cross references
them with the FQDN sets it can find for the associated serials. With the
FQDN sets a second query is done to find all the non-renewal FQDN sets
for the serials, giving a count of the total non-renewal issuances to
use for rate limiting.

Resolves #2800
This commit is contained in:
Daniel McCarney 2017-06-27 15:39:59 -04:00 committed by GitHub
parent 4119bc7cde
commit 71f8ae0e87
6 changed files with 358 additions and 13 deletions

View File

@ -4,9 +4,9 @@ package features
import "fmt"
const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationAllowKeyRolloverResubmitMissingSCTsOnlyGoogleSafeBrowsingV4UseAIAIssuerURLAllowTLS02ChallengesGenerateOCSPEarlyReusePendingAuthzCountCertificatesExactRandomDirectoryEntryIPv6FirstDirectoryMeta"
const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationAllowKeyRolloverResubmitMissingSCTsOnlyGoogleSafeBrowsingV4UseAIAIssuerURLAllowTLS02ChallengesGenerateOCSPEarlyReusePendingAuthzCountCertificatesExactRandomDirectoryEntryIPv6FirstDirectoryMetaAllowRenewalFirstRL"
var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 57, 80, 100, 115, 135, 152, 169, 191, 211, 220, 233}
var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 57, 80, 100, 115, 135, 152, 169, 191, 211, 220, 233, 252}
func (i FeatureFlag) String() string {
if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

View File

@ -27,6 +27,7 @@ const (
RandomDirectoryEntry
IPv6First
DirectoryMeta
AllowRenewalFirstRL
)
// List of features and their default value, protected by fMu
@ -45,6 +46,7 @@ var features = map[FeatureFlag]bool{
RandomDirectoryEntry: false,
IPv6First: false,
DirectoryMeta: false,
AllowRenewalFirstRL: false,
}
var fMu = new(sync.RWMutex)

161
sa/sa.go
View File

@ -442,9 +442,11 @@ func (ssa *SQLStorageAuthority) countCertificatesByExactName(domain string, earl
}
// countCertificates returns, for a single domain, the count of
// certificates issued in the given time range for that domain using the
// non-renewal certificate issuances in the given time range for that domain using the
// provided query, assumed to be either `countCertificatesExactSelect` or
// `countCertificatesSelect`.
// `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
@ -452,9 +454,7 @@ func (ssa *SQLStorageAuthority) countCertificatesByExactName(domain string, earl
func (ssa *SQLStorageAuthority) countCertificates(domain string, earliest, latest time.Time, query string) (int, error) {
var count int64
const max = 10000
var serials []struct {
Serial string
}
var serials []string
_, err := ssa.dbMap.Select(
&serials,
query,
@ -467,16 +467,45 @@ func (ssa *SQLStorageAuthority) countCertificates(domain string, earliest, lates
if err == sql.ErrNoRows {
return 0, nil
} else if err != nil {
return -1, err
return 0, err
} else if count > max {
return max, TooManyCertificatesError(fmt.Sprintf("More than %d issuedName entries for %s.", max, domain))
}
serialMap := make(map[string]struct{}, len(serials))
for _, s := range serials {
serialMap[s.Serial] = struct{}{}
}
return len(serialMap), nil
// If the `AllowRenewalFirstRL` feature flag is enabled then do the work
// required to discount renewals
if features.Enabled(features.AllowRenewalFirstRL) {
// If there are no serials found, short circuit since there isn't subsequent
// work to do
if len(serials) == 0 {
return 0, nil
}
// Find all FQDN Set Hashes with the serials from the issuedNames table that
// were visible within our search window
fqdnSets, err := ssa.getFQDNSetsBySerials(serials)
if err != nil {
return 0, err
}
// Using those FQDN Set Hashes, we can then find all of the non-renewal
// issuances with a second query against the fqdnSets table using the set
// hashes we know about
nonRenewalIssuances, err := ssa.getNewIssuancesByFQDNSet(fqdnSets, earliest)
if err != nil {
return 0, err
}
return nonRenewalIssuances, nil
} else {
// Otherwise, use the preexisting behaviour and deduplicate by serials
// returning a count of unique serials qignoring any potential renewals
serialMap := make(map[string]struct{}, len(serials))
for _, s := range serials {
serialMap[s] = struct{}{}
}
return len(serialMap), nil
}
}
// GetCertificate takes a serial number and returns the corresponding
@ -1062,6 +1091,116 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(ctx context.Context, window time.D
return count, err
}
// setHash is a []byte representing the hash of an FQDN Set
type setHash []byte
// getFQDNSetsBySerials finds the setHashes corresponding to a set of
// certificate serials. These serials can be used to check whether any
// certificates have been issued for the same set of names previously.
func (ssa *SQLStorageAuthority) getFQDNSetsBySerials(serials []string) ([]setHash, error) {
var fqdnSets []setHash
// It is unexpected that this function would be called with no serials
if len(serials) == 0 {
err := fmt.Errorf("getFQDNSetsBySerials called with no serials")
ssa.log.AuditErr(err.Error())
return nil, err
}
qmarks := make([]string, len(serials))
params := make([]interface{}, len(serials))
for i, serial := range serials {
params[i] = serial
qmarks[i] = "?"
}
query := "SELECT setHash FROM fqdnSets " +
"WHERE serial IN (" + strings.Join(qmarks, ",") + ")"
_, err := ssa.dbMap.Select(
&fqdnSets,
query,
params...)
if err != nil {
return nil, err
}
// The serials existed when we found them in issuedNames, they should continue
// to exist here. Otherwise an internal consistency violation occured and
// needs to be audit logged
if err == sql.ErrNoRows {
err := fmt.Errorf("getFQDNSetsBySerials returned no rows - internal consistency violation")
ssa.log.AuditErr(err.Error())
return nil, err
}
return fqdnSets, nil
}
// getNewIssuancesByFQDNSet returns a count of new issuances (renewals are not
// included) for a given slice of fqdnSets that occurred after the earliest
// parameter.
func (ssa *SQLStorageAuthority) getNewIssuancesByFQDNSet(fqdnSets []setHash, earliest time.Time) (int, error) {
var results []struct {
Serial string
SetHash setHash
Issued time.Time
}
qmarks := make([]string, len(fqdnSets))
params := make([]interface{}, len(fqdnSets))
for i, setHash := range fqdnSets {
// We have to cast the setHash back to []byte here since the sql package
// isn't able to convert `sa.setHash` for the parameter value itself
params[i] = []byte(setHash)
qmarks[i] = "?"
}
query := "SELECT serial, setHash, issued FROM fqdnSets " +
"WHERE setHash IN (" + strings.Join(qmarks, ",") + ") " +
"ORDER BY setHash, issued"
// First, find the serial, sethash and issued date from the fqdnSets table for
// the given fqdn set hashes
_, err := ssa.dbMap.Select(
&results,
query,
params...)
if err != nil && err != sql.ErrNoRows {
return -1, err
}
// If there are no results we have encountered a major error and
// should loudly complain
if err == sql.ErrNoRows || len(results) == 0 {
ssa.log.AuditErr(fmt.Sprintf("Found no results from fqdnSets for setHashes known to exist: %#v", fqdnSets))
return 0, err
}
processedSetHashes := make(map[string]bool)
issuanceCount := 0
// Loop through each set hash result, counting issuances per unique set hash
// that are within the window specified by the earliest parameter
for _, result := range results {
key := string(result.SetHash)
// Skip set hashes that we have already processed - we only care about the
// first issuance
if processedSetHashes[key] {
continue
}
// If the issued date is before our earliest cutoff then skip it
if result.Issued.Before(earliest) {
continue
}
// Otherwise note the issuance and mark the set hash as processed
issuanceCount++
processedSetHashes[key] = true
}
// Return the count of how many non-renewal issuances there were
return issuanceCount, nil
}
// FQDNSetExists returns a bool indicating if one or more FQDN sets |names|
// exists in the database
func (ssa *SQLStorageAuthority) FQDNSetExists(ctx context.Context, names []string) (bool, error) {

View File

@ -18,6 +18,7 @@ import (
"golang.org/x/net/context"
"github.com/jmhodges/clock"
gorp "gopkg.in/go-gorp/gorp.v2"
jose "gopkg.in/square/go-jose.v1"
"github.com/letsencrypt/boulder/core"
@ -1107,3 +1108,174 @@ func TestReverseName(t *testing.T) {
test.AssertEquals(t, output, tc.inputReversed)
}
}
type fqdnTestcase struct {
Serial string
Names []string
ExpectedHash setHash
Issued time.Time
Expires time.Time
}
func setupFQDNSets(t *testing.T, db *gorp.DbMap, fc clock.FakeClock) map[string]fqdnTestcase {
namesA := []string{"a.example.com", "B.example.com"}
namesB := []string{"example.org"}
namesC := []string{"letsencrypt.org"}
expectedHashA := setHash{0x92, 0xc7, 0xf2, 0x47, 0xbd, 0x1e, 0xea, 0x8d, 0x52, 0x7f, 0xb0, 0x59, 0x19, 0xe9, 0xbe, 0x81, 0x78, 0x88, 0xe6, 0xf7, 0x55, 0xf0, 0x1c, 0xc9, 0x63, 0x15, 0x5f, 0x8e, 0x52, 0xae, 0x95, 0xc1}
expectedHashB := setHash{0xbf, 0xab, 0xc3, 0x74, 0x32, 0x95, 0x8b, 0x6, 0x33, 0x60, 0xd3, 0xad, 0x64, 0x61, 0xc9, 0xc4, 0x73, 0x5a, 0xe7, 0xf8, 0xed, 0xd4, 0x65, 0x92, 0xa5, 0xe0, 0xf0, 0x14, 0x52, 0xb2, 0xe4, 0xb5}
expectedHashC := setHash{0xf2, 0xbb, 0x7b, 0xab, 0x8, 0x2c, 0x18, 0xee, 0x8, 0x97, 0x17, 0xbe, 0x67, 0xd7, 0x12, 0x14, 0xaa, 0x4, 0xac, 0xe2, 0x29, 0x2a, 0x67, 0x2c, 0x37, 0x2c, 0xf3, 0x33, 0xe1, 0xb0, 0xd8, 0xe7}
now := fc.Now()
testcases := map[string]fqdnTestcase{
// One test case with serial "a" issued now and expiring in two hours for
// namesA
"a": fqdnTestcase{
Serial: "a",
Names: namesA,
ExpectedHash: expectedHashA,
Issued: now,
Expires: now.Add(time.Hour * 2).UTC(),
},
// One test case with serial "b", issued one hour from now and expiring in
// two hours, also for namesA
"b": fqdnTestcase{
Serial: "b",
Names: namesA,
ExpectedHash: expectedHashA,
Issued: now.Add(time.Hour),
Expires: now.Add(time.Hour * 2).UTC(),
},
// One test case with serial "c", issued one hour from now and expiring in
// two hours, for namesB
"c": fqdnTestcase{
Serial: "c",
Names: namesB,
ExpectedHash: expectedHashB,
Issued: now.Add(time.Hour),
Expires: now.Add(time.Hour * 2).UTC(),
},
// One test case with serial "d", issued five hours in the past and expiring
// in two hours from now, with namesC
"d": fqdnTestcase{
Serial: "d",
Names: namesC,
ExpectedHash: expectedHashC,
Issued: now.Add(-5 * time.Hour),
Expires: now.Add(time.Hour * 2).UTC(),
},
}
for _, tc := range testcases {
tx, err := db.Begin()
test.AssertNotError(t, err, "Failed to open transaction")
err = addFQDNSet(tx, tc.Names, tc.Serial, tc.Issued, tc.Expires)
test.AssertNotError(t, err, fmt.Sprintf("Failed to add fqdnSet for %#v", tc))
test.AssertNotError(t, tx.Commit(), "Failed to commit transaction")
}
return testcases
}
func TestGetFQDNSetsBySerials(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()
// Add the test fqdn sets
testcases := setupFQDNSets(t, sa.dbMap, fc)
// Asking for the fqdnSets for no serials should produce an error since this
// is not expected in normal conditions
fqdnSets, err := sa.getFQDNSetsBySerials([]string{})
test.AssertError(t, err, "No error calling getFQDNSetsBySerials for empty serials")
test.AssertEquals(t, len(fqdnSets), 0)
// Asking for the fqdnSets for serials that don't exist should return nothing
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"this", "doesn't", "exist"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for non-existent serials")
test.AssertEquals(t, len(fqdnSets), 0)
// Asking for the fqdnSets for serial "a" should return the expectedHashA hash
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"a"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"a\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["a"].ExpectedHash))
// Asking for the fqdnSets for serial "b" should return the expectedHashA hash
// because cert "b" has namesA subjects
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"b"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"b\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["b"].ExpectedHash))
// Asking for the fqdnSets for serial "d" should return the expectedHashC hash
// because cert "d" has namesC subjects
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"d"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"d\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["d"].ExpectedHash))
// Asking for the fqdnSets for serial "c" should return the expectedHashB hash
// because cert "c" has namesB subjects
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"c"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"c\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["c"].ExpectedHash))
// Asking for the fqdnSets for serial "a", "b", "c" and "made up" should return
// the three expected hashes - two expectedHashA (for "a" and "b"), one
// expectedHashB (for "c")
fqdnSets, err = sa.getFQDNSetsBySerials([]string{"a", "b", "c", "made up"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"a\", \"b\", \"c\", \"made up\"")
test.AssertEquals(t, len(fqdnSets), 3)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["a"].ExpectedHash))
test.AssertEquals(t, string(fqdnSets[1]), string(testcases["b"].ExpectedHash))
test.AssertEquals(t, string(fqdnSets[2]), string(testcases["c"].ExpectedHash))
}
func TestGetNewIssuancesByFQDNSet(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()
// Add the test fqdn sets
testcases := setupFQDNSets(t, sa.dbMap, fc)
// Use one hour ago as the earliest cut off
earliest := fc.Now().Add(-time.Hour)
// Calling getNewIssuancesByFQDNSet with an empty FQDNSet should error
count, err := sa.getNewIssuancesByFQDNSet(nil, earliest)
test.AssertError(t, err, "No error calling getNewIssuancesByFQDNSet for empty fqdn set")
test.AssertEquals(t, count, -1)
// Calling getNewIssuancesByFQDNSet with FQDNSet hashes that don't exist
// should return 0
count, err = sa.getNewIssuancesByFQDNSet([]setHash{setHash{0xC0, 0xFF, 0xEE}, setHash{0x13, 0x37}}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for non-existent set hashes")
test.AssertEquals(t, count, 0)
// Calling getNewIssuancesByFQDNSet with the "a" expected hash should return
// 1, since both testcase "b" was a renewal of testcase "a"
count, err = sa.getNewIssuancesByFQDNSet([]setHash{testcases["a"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase a")
test.AssertEquals(t, count, 1)
// Calling getNewIssuancesByFQDNSet with the "c" expected hash should return
// 1, since there is only one issuance for this sethash
count, err = sa.getNewIssuancesByFQDNSet([]setHash{testcases["c"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c")
test.AssertEquals(t, count, 1)
// Calling getNewIssuancesByFQDNSet with the "c" and "d" expected hashes should return
// only 1, since there is only one issuance for the provided set hashes that
// is within the earliest window. The issuance for "d" was too far in the past
// to be counted
count, err = sa.getNewIssuancesByFQDNSet([]setHash{testcases["c"].ExpectedHash, testcases["d"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c and d")
test.AssertEquals(t, count, 1)
// But by moving the earliest point behind the "d" issuance, we should now get a count of 2
count, err = sa.getNewIssuancesByFQDNSet([]setHash{testcases["c"].ExpectedHash, testcases["d"].ExpectedHash}, earliest.Add(-6*time.Hour))
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c and d with adjusted earliest")
test.AssertEquals(t, count, 2)
}

View File

@ -25,6 +25,7 @@
},
"features": {
"AllowAccountDeactivation": true,
"AllowRenewalFirstRL": true,
"ReusePendingAuthz": true
}
},

View File

@ -311,6 +311,36 @@ def test_expired_authz_purger():
expect(now, 0, "authz")
expect(after_grace_period, 1, "authz")
def test_renewal_exemption():
"""
Under a single domain, issue one certificate, then two renewals of that
certificate, then one more different certificate (with a different
subdomain). Since the certificatesPerName rate limit in testing is 2 per 90
days, and the renewals should be discounted under the renewal exemption,
each of these issuances should succeed. Then do one last issuance that we
expect to be rate limited, just to check that the rate limit is actually 2,
and we are testing what we think we are testing. See
https://letsencrypt.org/docs/rate-limits/ for more details.
"""
# TODO(@cpu): Once the `AllowRenewalFirstRL` feature flag is enabled by
# default, delete this early return.
if not default_config_dir.startswith("test/config-next"):
return
base_domain = random_domain()
# First issuance
auth_and_issue(["www." + base_domain])
# First Renewal
auth_and_issue(["www." + base_domain])
# Second Renewal
auth_and_issue(["www." + base_domain])
# Issuance of a different cert
auth_and_issue(["blog." + base_domain])
# Final, failed issuance, for another different cert
chisel.expect_problem("urn:acme:error:rateLimited",
lambda: auth_and_issue(["mail." + base_domain]))
def test_certificates_per_name():
chisel.expect_problem("urn:acme:error:rateLimited",
lambda: auth_and_issue(["lim.it"]))
@ -410,6 +440,7 @@ def run_chisel():
test_ocsp()
test_single_ocsp()
test_dns_challenge()
test_renewal_exemption()
if __name__ == "__main__":
try: