WFE: Check NewOrder rate limits

cmd/boulder-wfe2/main.go

@@ -158,6 +158,13 @@ type Config struct {
 			// default rate limits.
 			Overrides string
 		}
+
+		// MaxNames is the maximum number of subjectAltNames in a single cert.
+		// The value supplied SHOULD be greater than 0 and no more than 100,
+		// defaults to 100. These limits are per section 7.1 of our combined
+		// CP/CPS, under "DV-SSL Subscriber Certificate". The value must match
+		// the CA and RA configurations.
+		MaxNames int `validate:"min=0,max=100"`
 	}
 
 	Syslog        cmd.SyslogConfig
@@ -299,6 +306,11 @@ func main() {
 	if *debugAddr != "" {
 		c.WFE.DebugAddr = *debugAddr
 	}
+	maxNames := c.WFE.MaxNames
+	if maxNames == 0 {
+		// Default to 100 names per cert.
+		maxNames = 100
+	}
 
 	certChains := map[issuance.IssuerNameID][][]byte{}
 	issuerCerts := map[issuance.IssuerNameID]*issuance.Certificate{}
@@ -396,6 +408,7 @@ func main() {
 		accountGetter,
 		limiter,
 		txnBuilder,
+		maxNames,
 	)
 	cmd.FailOnError(err, "Unable to create WFE")
 

ratelimits/bucket.go

@@ -282,7 +282,11 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerAccountTransaction(reg
 //
 // When a CertificatesPerDomainPerAccount override is not configured, a check-
 // and-spend Transaction is returned for each per domain bucket.
-func (builder *TransactionBuilder) CertificatesPerDomainTransactions(regId int64, orderDomains []string) ([]Transaction, error) {
+func (builder *TransactionBuilder) CertificatesPerDomainTransactions(regId int64, orderDomains []string, maxNames int) ([]Transaction, error) {
+	if len(orderDomains) > maxNames {
+		return nil, fmt.Errorf("order contains more than %d DNS names", maxNames)
+	}
+
 	perAccountLimitBucketKey, err := newRegIdBucketKey(CertificatesPerDomainPerAccount, regId)
 	if err != nil {
 		return nil, err

test/config-next/wfe2-ratelimit-defaults.yml

@@ -1,2 +1,24 @@
-NewRegistrationsPerIPAddress: { burst: 10000, count: 10000, period: 168h }
-NewRegistrationsPerIPv6Range: { burst: 99999, count: 99999, period: 168h }
+NewRegistrationsPerIPAddress:
+  count: 10000
+  burst: 10000
+  period: 168h
+NewRegistrationsPerIPv6Range:
+  count: 99999
+  burst: 99999
+  period: 168h
+CertificatesPerDomain:
+  count: 2
+  burst: 2
+  period: 2160h
+FailedAuthorizationsPerAccount:
+  count: 3
+  burst: 3
+  period: 5m
+NewOrdersPerAccount:
+  count: 1500
+  burst: 1500
+  period: 3h
+CertificatesPerFQDNSet:
+  count: 6
+  burst: 6
+  period: 168h

test/config-next/wfe2-ratelimit-overrides.yml

@@ -2,4 +2,39 @@
     burst: 1000000
     count: 1000000
     period: 168h
-    ids: [127.0.0.1]
+    ids:
+      - 127.0.0.1
+- CertificatesPerDomain:
+    burst: 1
+    count: 1
+    period: 2160h
+    ids:
+      - ratelimit.me
+- CertificatesPerDomain:
+    burst: 10000
+    count: 10000
+    period: 2160h
+    ids:
+      - le.wtf
+      - le1.wtf
+      - le2.wtf
+      - le3.wtf
+      - nginx.wtf
+      - good-caa-reserved.com
+      - bad-caa-reserved.com
+      - ecdsa.le.wtf
+      - must-staple.le.wtf
+- CertificatesPerFQDNSet:
+    burst: 10000
+    count: 10000
+    period: 168h
+    ids:
+      - le.wtf
+      - le1.wtf
+      - le2.wtf
+      - le3.wtf
+      - le.wtf,le1.wtf
+      - good-caa-reserved.com
+      - nginx.wtf
+      - ecdsa.le.wtf
+      - must-staple.le.wtf

test/integration/ratelimit_test.go

@@ -3,8 +3,17 @@
 package integration
 
 import (
+	"context"
+	"os"
+	"strings"
 	"testing"
 
+	"github.com/jmhodges/clock"
+	"github.com/letsencrypt/boulder/cmd"
+	blog "github.com/letsencrypt/boulder/log"
+	"github.com/letsencrypt/boulder/metrics"
+	"github.com/letsencrypt/boulder/ratelimits"
+	bredis "github.com/letsencrypt/boulder/redis"
 	"github.com/letsencrypt/boulder/test"
 )
 
@@ -20,4 +29,45 @@ func TestDuplicateFQDNRateLimit(t *testing.T) {
 
 	_, err = authAndIssue(nil, nil, []string{domain}, true)
 	test.AssertError(t, err, "Somehow managed to issue third certificate")
+
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		// Setup rate limiting.
+		rc := bredis.Config{
+			Username: "unittest-rw",
+			TLS: cmd.TLSConfig{
+				CACertFile: "test/redis-tls/minica.pem",
+				CertFile:   "test/redis-tls/boulder/cert.pem",
+				KeyFile:    "test/redis-tls/boulder/key.pem",
+			},
+			Lookups: []cmd.ServiceDomain{
+				{
+					Service: "redisratelimits",
+					Domain:  "service.consul",
+				},
+			},
+			LookupDNSAuthority: "consul.service.consul",
+		}
+		rc.PasswordConfig = cmd.PasswordConfig{
+			PasswordFile: "test/secrets/ratelimits_redis_password",
+		}
+
+		fc := clock.NewFake()
+		stats := metrics.NoopRegisterer
+		log := blog.NewMock()
+		ring, err := bredis.NewRingFromConfig(rc, stats, log)
+		test.AssertNotError(t, err, "making redis ring client")
+		source := ratelimits.NewRedisSource(ring.Ring, fc, stats)
+		test.AssertNotNil(t, source, "source should not be nil")
+		limiter, err := ratelimits.NewLimiter(fc, source, stats)
+		test.AssertNotError(t, err, "making limiter")
+		txnBuilder, err := ratelimits.NewTransactionBuilder("test/config-next/wfe2-ratelimit-defaults.yml", "")
+		test.AssertNotError(t, err, "making transaction composer")
+
+		// Check that the CertificatesPerFQDNSet limit is reached.
+		txn, err := txnBuilder.CertificatesPerFQDNSetTransaction([]string{domain})
+		test.AssertNotError(t, err, "making transaction")
+		result, err := limiter.Check(context.Background(), txn)
+		test.AssertNotError(t, err, "checking transaction")
+		test.Assert(t, !result.Allowed, "should not be allowed")
+	}
 }

test/v2_integration.py

@@ -1565,6 +1565,11 @@ def test_renewal_exemption():
     chisel2.expect_problem("urn:ietf:params:acme:error:rateLimited",
         lambda: chisel2.auth_and_issue(["mail." + base_domain]))
 
+# TODO(#5545)
+#   - Phase 2: Once the new rate limits are authoritative in config-next, ensure
+#     that this test only runs in config.
+#   - Phase 3: Once the new rate limits are authoritative in config, remove this
+#     test entirely.
 def test_certificates_per_name():
     chisel2.expect_problem("urn:ietf:params:acme:error:rateLimited",
         lambda: chisel2.auth_and_issue([random_domain() + ".lim.it"]))

wfe2/wfe.go

@@ -170,6 +170,7 @@ type WebFrontEndImpl struct {
 	pendingAuthorizationLifetime time.Duration
 	limiter                      *ratelimits.Limiter
 	txnBuilder                   *ratelimits.TransactionBuilder
+	maxNames                     int
 }
 
 // NewWebFrontEndImpl constructs a web service for Boulder
@@ -193,6 +194,7 @@ func NewWebFrontEndImpl(
 	accountGetter AccountGetter,
 	limiter *ratelimits.Limiter,
 	txnBuilder *ratelimits.TransactionBuilder,
+	maxNames int,
 ) (WebFrontEndImpl, error) {
 	if len(issuerCertificates) == 0 {
 		return WebFrontEndImpl{}, errors.New("must provide at least one issuer certificate")
@@ -231,6 +233,7 @@ func NewWebFrontEndImpl(
 		accountGetter:                accountGetter,
 		limiter:                      limiter,
 		txnBuilder:                   txnBuilder,
+		maxNames:                     maxNames,
 	}
 
 	return wfe, nil
@@ -2056,6 +2059,75 @@ func (wfe *WebFrontEndImpl) orderToOrderJSON(request *http.Request, order *corep
 	return respObj
 }
 
+// checkNewOrderLimits checks whether sufficient limit quota exists for the
+// creation of a new order. If so, that quota is spent. If an error is
+// encountered during the check, it is logged but not returned.
+//
+// TODO(#5545): For now we're simply exercising the new rate limiter codepath.
+// This should eventually return a berrors.RateLimit error containing the retry
+// after duration among other information available in the ratelimits.Decision.
+func (wfe *WebFrontEndImpl) checkNewOrderLimits(ctx context.Context, regId int64, names []string) []ratelimits.Transaction {
+	if wfe.limiter == nil && wfe.txnBuilder == nil {
+		// Limiter is disabled.
+		return nil
+	}
+
+	logTxnErr := func(err error, limit ratelimits.Name) {
+		// TODO(#5545): Once key-value rate limits are authoritative this log
+		// line should be removed in favor of returning the error.
+		wfe.log.Errf("constructing rate limit transaction for %s rate limit: %s", limit, err)
+	}
+
+	var transactions []ratelimits.Transaction
+	txn, err := wfe.txnBuilder.OrdersPerAccountTransaction(regId)
+	if err != nil {
+		logTxnErr(err, ratelimits.NewOrdersPerAccount)
+		return nil
+	}
+	transactions = append(transactions, txn)
+
+	txn, err = wfe.txnBuilder.FailedAuthorizationsPerAccountCheckOnlyTransaction(regId)
+	if err != nil {
+		logTxnErr(err, ratelimits.FailedAuthorizationsPerAccount)
+		return nil
+	}
+	transactions = append(transactions, txn)
+
+	txns, err := wfe.txnBuilder.CertificatesPerDomainTransactions(regId, names, wfe.maxNames)
+	if err != nil {
+		logTxnErr(err, ratelimits.CertificatesPerDomain)
+		return nil
+	}
+	transactions = append(transactions, txns...)
+
+	txn, err = wfe.txnBuilder.CertificatesPerFQDNSetTransaction(names)
+	if err != nil {
+		logTxnErr(err, ratelimits.CertificatesPerFQDNSet)
+		return nil
+	}
+	transactions = append(transactions, txn)
+
+	_, err = wfe.limiter.BatchSpend(ctx, transactions)
+	if err != nil {
+		if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+			return nil
+		}
+		wfe.log.Errf("checking limits for newOrder: %s", err)
+		return nil
+	}
+	return transactions
+}
+
+func (wfe *WebFrontEndImpl) refundNewOrderLimits(ctx context.Context, transactions []ratelimits.Transaction) {
+	if wfe.limiter == nil || wfe.txnBuilder == nil {
+		return
+	}
+	_, err := wfe.limiter.BatchRefund(ctx, transactions)
+	if err != nil {
+		wfe.log.Errf("refunding newOrder limits: %s", err)
+	}
+}
+
 // NewOrder is used by clients to create a new order object and a set of
 // authorizations to fulfill for issuance.
 func (wfe *WebFrontEndImpl) NewOrder(
@@ -2130,13 +2202,38 @@ func (wfe *WebFrontEndImpl) NewOrder(
 
 	logEvent.DNSNames = names
 
+	// TODO(#5545): Spending and Refunding can be async until these rate limits
+	// are authoritative. This saves us from adding latency to each request.
+	doneCheckingLimits := make(chan struct{})
+	var ratelimitTxns []ratelimits.Transaction
+	var newOrderSuccessful bool
+	var errIsRateLimit bool
+
+	go func() {
+		// Close the channel on goroutine completion.
+		defer close(doneCheckingLimits)
+		ratelimitTxns = wfe.checkNewOrderLimits(ctx, acct.ID, names)
+	}()
+
+	defer func() {
+		// Wait for the rate limit check to complete before attempting to refund
+		// the limits. If the check failed, we don't want to refund anything.
+		<-doneCheckingLimits
+		if !newOrderSuccessful && !errIsRateLimit && ratelimitTxns != nil {
+			wfe.refundNewOrderLimits(ctx, ratelimitTxns)
+		}
+	}()
+
 	order, err := wfe.ra.NewOrder(ctx, &rapb.NewOrderRequest{
 		RegistrationID: acct.ID,
 		Names:          names,
 	})
 	// TODO(#7153): Check each value via core.IsAnyNilOrZero
 	if err != nil || order == nil || order.Id == 0 || order.RegistrationID == 0 || len(order.Names) == 0 || core.IsAnyNilOrZero(order.Created, order.Expires) {
 		wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Error creating new order"), err)
+		if errors.Is(err, berrors.RateLimit) {
+			errIsRateLimit = true
+		}
 		return
 	}
 	logEvent.Created = fmt.Sprintf("%d", order.Id)
@@ -2151,6 +2248,7 @@ func (wfe *WebFrontEndImpl) NewOrder(
 		wfe.sendError(response, logEvent, probs.ServerInternal("Error marshaling order"), err)
 		return
 	}
+	newOrderSuccessful = true
 }
 
 // GetOrder is used to retrieve a existing order object