sa: use a faster query for certificates per name rate limit (#4179)

Right now we run a `SELECT COUNT` query on issuedNames to calculate this rate limit.
Unfortunately, counting large numbers of rows is very slow. This change introduces a
dedicated table for keeping track of that rate limit.

Instead of being keyed by FQDN, this new `certificatesPerName` table is keyed by
the same field as rate limits are calculated on: the base domain. Each row has a base
domain, a time (chunked by hour), and a count. This means calculating the rate limit
status for each domain reads at most 7 * 24 rows.

This should particularly speed up cases when a single domain has lots of subdomains.

Fixes #4152

features/featureflag_string.go

@@ -25,12 +25,13 @@ func _() {
 	_ = x[EnforceMultiVA-14]
 	_ = x[MultiVAFullResults-15]
 	_ = x[RemoveWFE2AccountID-16]
-	_ = x[CheckRenewalFirst-17]
+	_ = x[FasterRateLimit-17]
+	_ = x[CheckRenewalFirst-18]
 }
 
-const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRAEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDCheckRenewalFirst"
+const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRAEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDFasterRateLimitCheckRenewalFirst"
 
-var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 140, 153, 164, 181, 203, 213, 232, 246, 264, 283, 300}
+var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 140, 153, 164, 181, 203, 213, 232, 246, 264, 283, 298, 315}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -45,6 +45,9 @@ const (
 	// RemoveWFE2AccountID will remove the account ID from account objects returned
 	// from the new-account endpoint if enabled.
 	RemoveWFE2AccountID
+	// FasterRateLimit enables use of a separate table for counting the
+	// Certificates Per Name rate limit.
+	FasterRateLimit
 	// CheckRenewalFirst will check whether an issuance is a renewal before
 	// checking the "certificates per name" rate limit.
 	CheckRenewalFirst
@@ -69,6 +72,7 @@ var features = map[FeatureFlag]bool{
 	EnforceMultiVA:           false,
 	MultiVAFullResults:       false,
 	RemoveWFE2AccountID:      false,
+	FasterRateLimit:          false,
 	CheckRenewalFirst:        false,
 }
 

sa/_db-next/migrations/20190416000000_AddRateLimitTable.sql

@@ -0,0 +1,15 @@
+-- +goose Up
+-- SQL in section 'Up' is executed when this migration is applied
+
+CREATE TABLE `certificatesPerName` (
+    `id` BIGINT(20) PRIMARY KEY AUTO_INCREMENT,
+    `eTLDPlusOne` VARCHAR(255) NOT NULL,
+    `time` DATETIME NOT NULL,
+    `count` INTEGER NOT NULL,
+    UNIQUE KEY `eTLDPlusOne_time_idx` (`eTLDPlusOne`, `time`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+-- +goose Down
+-- SQL section 'Down' is executed when this migration is rolled back
+
+DROP TABLE `certificatesPerName`;

sa/rate_limits.go

@@ -0,0 +1,102 @@
+package sa
+
+import (
+	"database/sql"
+	"strings"
+	"time"
+
+	"github.com/letsencrypt/boulder/features"
+	"github.com/weppos/publicsuffix-go/publicsuffix"
+	"golang.org/x/net/context"
+)
+
+// baseDomain returns the eTLD+1 of a domain name for the purpose of rate
+// limiting. For a domain name that is itself an eTLD, it returns its input.
+func baseDomain(name string) string {
+	eTLDPlusOne, err := publicsuffix.Domain(name)
+	if err != nil {
+		// publicsuffix.Domain will return an error if the input name is itself a
+		// public suffix. In that case we use the input name as the key for rate
+		// limiting. Since all of its subdomains will have separate keys for rate
+		// limiting (e.g. "foo.bar.publicsuffix.com" will have
+		// "bar.publicsuffix.com", this means that domains exactly equal to a
+		// public suffix get their own rate limit bucket. This is important
+		// because otherwise they might be perpetually unable to issue, assuming
+		// the rate of issuance from their subdomains was high enough.
+		return name
+	}
+	return eTLDPlusOne
+}
+
+// addCertificatesPerName adds 1 to the rate limit count for the provided domains,
+// in a specific time bucket. It must be executed in a transaction, and the
+// input timeToTheHour must be a time rounded to an hour.
+func (ssa *SQLStorageAuthority) addCertificatesPerName(
+	ctx context.Context,
+	db dbSelectExecer,
+	names []string,
+	timeToTheHour time.Time,
+) error {
+	if !features.Enabled(features.FasterRateLimit) {
+		return nil
+	}
+	// De-duplicate the base domains.
+	baseDomainsMap := make(map[string]bool)
+	var qmarks []string
+	var values []interface{}
+	for _, name := range names {
+		base := baseDomain(name)
+		if !baseDomainsMap[base] {
+			baseDomainsMap[base] = true
+			values = append(values, base, timeToTheHour, 1)
+			qmarks = append(qmarks, "(?, ?, ?)")
+		}
+	}
+
+	_, err := db.Exec(`INSERT INTO certificatesPerName (eTLDPlusOne, time, count) VALUES `+
+		strings.Join(qmarks, ", ")+` ON DUPLICATE KEY UPDATE count=count+1;`,
+		values...)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// countCertificatesFaster returns, for a single domain, the count of
+// certificates issued in the given time range for that domain's eTLD+1 (aka
+// base domain). It uses the certificatesPerName table to make this lookup fast.
+// This function can replace both countCertificatesByName and
+// countCertificatesByExactName because domains that are exactly equal to an
+// public suffix have their issuances counted under a separate bucket from their
+// subdomains.
+func (ssa *SQLStorageAuthority) countCertificatesFaster(
+	db dbSelector,
+	domain string,
+	earliest,
+	latest time.Time,
+) (int, error) {
+	base := baseDomain(domain)
+	var counts []int
+	_, err := db.Select(
+		&counts,
+		`SELECT count FROM certificatesPerName
+		 WHERE eTLDPlusOne = :baseDomain AND
+		 time > :earliest AND
+		 time <= :latest`,
+		map[string]interface{}{
+			"baseDomain": base,
+			"earliest":   earliest,
+			"latest":     latest,
+		})
+	if err == sql.ErrNoRows {
+		return 0, nil
+	} else if err != nil {
+		return 0, err
+	}
+	var total int
+	for _, count := range counts {
+		total += count
+	}
+	return total, nil
+}

sa/rate_limits_test.go

@@ -0,0 +1,94 @@
+package sa
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/letsencrypt/boulder/features"
+
+	"golang.org/x/net/context"
+)
+
+func TestFasterRateLimit(t *testing.T) {
+	sa, _, cleanUp := initSA(t)
+	defer cleanUp()
+
+	if !strings.HasSuffix(os.Getenv("BOULDER_CONFIG_DIR"), "config-next") {
+		return
+	}
+	features.Set(map[string]bool{"FasterRateLimit": true})
+
+	aprilFirst, err := time.Parse(time.RFC3339, "2019-04-01T00:00:00Z")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	type inputCase struct {
+		time  time.Time
+		names []string
+	}
+	inputs := []inputCase{
+		{aprilFirst, []string{"example.com"}},
+		{aprilFirst, []string{"example.com", "www.example.com"}},
+		{aprilFirst, []string{"example.com", "other.example.com"}},
+		{aprilFirst, []string{"dyndns.org"}},
+		{aprilFirst, []string{"mydomain.dyndns.org"}},
+		{aprilFirst, []string{"mydomain.dyndns.org"}},
+		{aprilFirst, []string{"otherdomain.dyndns.org"}},
+	}
+
+	// For each hour in a week, add an enry for a certificate that has
+	// progressively more names.
+	var manyNames []string
+	for i := 0; i < 7*24; i++ {
+		manyNames = append(manyNames, fmt.Sprintf("%d.manynames.example.net", i))
+		inputs = append(inputs, inputCase{aprilFirst.Add(time.Duration(i) * time.Hour), manyNames})
+	}
+
+	for _, input := range inputs {
+		tx, err := sa.dbMap.Begin()
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = sa.addCertificatesPerName(context.Background(), tx, input.names, input.time)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = tx.Commit()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	const aWeek = time.Duration(7*24) * time.Hour
+
+	testCases := []struct {
+		caseName   string
+		domainName string
+		expected   int
+	}{
+		{"name doesn't exist", "non.example.org", 0},
+		{"base name gets dinged for all certs including it", "example.com", 3},
+		{"subdomain gets dinged for neighbors", "www.example.com", 3},
+		{"other subdomain", "other.example.com", 3},
+		{"many subdomains", "1.manynames.example.net", 168},
+		{"public suffix gets its own bucket", "dyndns.org", 1},
+		{"subdomain of public suffix gets its own bucket", "mydomain.dyndns.org", 2},
+		{"subdomain of public suffix gets its own bucket 2", "otherdomain.dyndns.org", 1},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.caseName, func(t *testing.T) {
+			count, err := sa.countCertificatesFaster(sa.dbMap, tc.domainName, aprilFirst.Add(-1*time.Second), aprilFirst.Add(aWeek))
+			if err != nil {
+				t.Fatal(err)
+			}
+			if count != tc.expected {
+				t.Errorf("Expected count of %d for %q, got %d", tc.expected, tc.domainName, count)
+			}
+		})
+	}
+}

sa/sa.go

@@ -20,6 +20,7 @@ import (
 	"github.com/letsencrypt/boulder/core"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	berrors "github.com/letsencrypt/boulder/errors"
+	"github.com/letsencrypt/boulder/features"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
@@ -43,8 +44,9 @@ type SQLStorageAuthority struct {
 
 	// We use function types here so we can mock out this internal function in
 	// unittests.
-	countCertificatesByName certCountFunc
-	getChallenges           getChallengesFunc
+	countCertificatesByName      certCountFunc
+	countCertificatesByExactName certCountFunc
+	getChallenges                getChallengesFunc
 }
 
 func digest256(data []byte) []byte {
@@ -105,6 +107,11 @@ func NewSQLStorageAuthority(
 	}
 
 	ssa.countCertificatesByName = ssa.countCertificatesByNameImpl
+	ssa.countCertificatesByExactName = ssa.countCertificatesByExactNameImpl
+	if features.Enabled(features.FasterRateLimit) {
+		ssa.countCertificatesByName = ssa.countCertificatesFaster
+		ssa.countCertificatesByExactName = ssa.countCertificatesFaster
+	}
 	ssa.getChallenges = ssa.getChallengesImpl
 
 	return ssa, nil
@@ -476,7 +483,7 @@ func (ssa *SQLStorageAuthority) countCertificatesByNameImpl(
 // countCertificatesByExactNames returns, for a single domain, the count of
 // certificates issued in the given time range for that domain. In contrast to
 // countCertificatesByNames subdomains are NOT considered.
-func (ssa *SQLStorageAuthority) countCertificatesByExactName(
+func (ssa *SQLStorageAuthority) countCertificatesByExactNameImpl(
 	db dbSelector,
 	domain string,
 	earliest,
@@ -985,6 +992,16 @@ func (ssa *SQLStorageAuthority) AddCertificate(
 		return "", Rollback(tx, err)
 	}
 
+	// Add to the rate limit table, but only for new certificates. Renewals
+	// don't count against the certificatesPerName limit.
+	if !isRenewal {
+		timeToTheHour := parsedCertificate.NotBefore.Round(time.Hour)
+		err = ssa.addCertificatesPerName(ctx, txWithCtx, parsedCertificate.DNSNames, timeToTheHour)
+		if err != nil {
+			return "", Rollback(tx, err)
+		}
+	}
+
 	err = addFQDNSet(
 		txWithCtx,
 		parsedCertificate.DNSNames,

test/config-next/sa.json

@@ -25,6 +25,7 @@
       ]
     },
     "features": {
+      "FasterRateLimit": true
     }
   },
 

test/sa_db_users.sql

@@ -34,6 +34,7 @@ GRANT SELECT(id,Lockcol) ON pendingAuthorizations TO 'sa'@'localhost';
 GRANT SELECT,INSERT ON certificates TO 'sa'@'localhost';
 GRANT SELECT,INSERT,UPDATE ON certificateStatus TO 'sa'@'localhost';
 GRANT SELECT,INSERT ON issuedNames TO 'sa'@'localhost';
+GRANT SELECT,INSERT,UPDATE ON certificatesPerName TO 'sa'@'localhost';
 GRANT SELECT,INSERT ON sctReceipts TO 'sa'@'localhost';
 GRANT SELECT,INSERT,UPDATE ON registrations TO 'sa'@'localhost';
 GRANT SELECT,INSERT,UPDATE ON challenges TO 'sa'@'localhost';