ratelimits: Exempt renewals from NewOrdersPerAccount and Certificates…

…PerDomain (#7513)

- Rename `NewOrderRequest` field `LimitsExempt` to `IsARIRenewal`
- Introduce a new `NewOrderRequest` field, `IsRenewal`
- Introduce a new (temporary) feature flag, `CheckRenewalExemptionAtWFE`

WFE:
- Perform renewal detection in the WFE when `CheckRenewalExemptionAtWFE`
is set
- Skip (key-value) `NewOrdersPerAccount` and `CertificatesPerDomain`
limit checks when renewal detection indicates the the order is a
renewal.

RA:
- Leave renewal detection in the RA intact
- Skip renewal detection and (legacy) `NewOrdersPerAccount` and
`CertificatesPerDomain` limit checks when `CheckRenewalExemptionAtWFE`
is set and the `NewOrderRequest` indicates that the order is a renewal.

Fixes #7508
Part of #5545

features/features.go

@@ -88,6 +88,16 @@ type Config struct {
 	//     `orders.certificateProfileName` column. Values in this column are
 	//     allowed to be empty.
 	MultipleCertificateProfiles bool
+
+	// CheckRenewalExemptionAtWFE when enabled, triggers the following behavior:
+	//  - WFE.NewOrder: checks if the order is a renewal and if so skips checks
+	//    for NewOrdersPerAccount and NewOrdersPerDomain limits.
+	//  - RA.NewOrderAndAuthzs: skips checks for legacy NewOrdersPerAccount and
+	//    NewOrdersPerDomain limits if the WFE indicates that the order is a
+	//    renewal.
+	//
+	// TODO(#7511): Remove this feature flag.
+	CheckRenewalExemptionAtWFE bool
 }
 
 var fMu = new(sync.RWMutex)

ra/proto/ra.proto

@@ -69,12 +69,15 @@ message AdministrativelyRevokeCertificateRequest {
 }
 
 message NewOrderRequest {
-  // Next unused field number: 6
+  // Next unused field number: 7
   int64 registrationID = 1;
   repeated string names = 2;
   string replacesSerial = 3;
-  bool limitsExempt = 4;
+  // TODO(#7512): Remove this field.
+  bool isARIRenewal = 4;
   string certificateProfileName = 5;
+  // TODO(#7512): Remove this field.
+  bool isRenewal = 6;
 }
 
 message FinalizeOrderRequest {

ra/ra.go

@@ -686,6 +686,9 @@ func (ra *RegistrationAuthorityImpl) checkInvalidAuthorizationLimit(ctx context.
 func (ra *RegistrationAuthorityImpl) checkNewOrdersPerAccountLimit(ctx context.Context, acctID int64, names []string, limit ratelimit.RateLimitPolicy) error {
 	// Check if there is already an existing certificate for the exact name set we
 	// are issuing for. If so bypass the newOrders limit.
+	//
+	// TODO(#7511): This check and early return should be removed, it's
+	// being performed at the WFE.
 	exists, err := ra.SA.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{Domains: names})
 	if err != nil {
 		return fmt.Errorf("checking renewal exemption for %q: %s", names, err)
@@ -1483,6 +1486,9 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
 	// check if there is already an existing certificate for
 	// the exact name set we are issuing for. If so bypass the
 	// the certificatesPerName limit.
+	//
+	// TODO(#7511): This check and early return should be removed, it's
+	// being performed, once, at the WFE.
 	exists, err := ra.SA.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{Domains: names})
 	if err != nil {
 		return fmt.Errorf("checking renewal exemption for %q: %s", names, err)
@@ -1580,9 +1586,11 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
 	}
 }
 
-func (ra *RegistrationAuthorityImpl) checkNewOrderLimits(ctx context.Context, names []string, regID int64) error {
+func (ra *RegistrationAuthorityImpl) checkNewOrderLimits(ctx context.Context, names []string, regID int64, isRenewal bool) error {
 	newOrdersPerAccountLimits := ra.rlPolicies.NewOrdersPerAccount()
-	if newOrdersPerAccountLimits.Enabled() {
+	// TODO(#7511): Remove the feature flag check.
+	skipCheck := features.Get().CheckRenewalExemptionAtWFE && isRenewal
+	if newOrdersPerAccountLimits.Enabled() && !skipCheck {
 		started := ra.clk.Now()
 		err := ra.checkNewOrdersPerAccountLimit(ctx, regID, names, newOrdersPerAccountLimits)
 		elapsed := ra.clk.Since(started)
@@ -1596,7 +1604,7 @@ func (ra *RegistrationAuthorityImpl) checkNewOrderLimits(ctx context.Context, na
 	}
 
 	certNameLimits := ra.rlPolicies.CertificatesPerName()
-	if certNameLimits.Enabled() {
+	if certNameLimits.Enabled() && !skipCheck {
 		started := ra.clk.Now()
 		err := ra.checkCertificatesPerNameLimit(ctx, names, certNameLimits, regID)
 		elapsed := ra.clk.Since(started)
@@ -2517,10 +2525,10 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 	}
 
 	// Renewal orders, indicated by ARI, are exempt from NewOrder rate limits.
-	if !req.LimitsExempt {
+	if !req.IsARIRenewal {
 
 		// Check if there is rate limit space for issuing a certificate.
-		err = ra.checkNewOrderLimits(ctx, newOrder.Names, newOrder.RegistrationID)
+		err = ra.checkNewOrderLimits(ctx, newOrder.Names, newOrder.RegistrationID, req.IsRenewal)
 		if err != nil {
 			return nil, err
 		}
@@ -2597,7 +2605,7 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 	}
 
 	// Renewal orders, indicated by ARI, are exempt from NewOrder rate limits.
-	if len(missingAuthzNames) > 0 && !req.LimitsExempt {
+	if len(missingAuthzNames) > 0 && !req.IsARIRenewal {
 		pendingAuthzLimits := ra.rlPolicies.PendingAuthorizationsPerAccount()
 		if pendingAuthzLimits.Enabled() {
 			// The order isn't fully authorized we need to check that the client

ra/ra_test.go

@@ -4455,7 +4455,7 @@ func TestNewOrderRateLimitingExempt(t *testing.T) {
 	test.AssertError(t, err, "orderTwo should have failed")
 
 	// Exempt orderTwo from rate limiting.
-	exampleOrderTwo.LimitsExempt = true
+	exampleOrderTwo.IsARIRenewal = true
 	_, err = ra.NewOrder(ctx, exampleOrderTwo)
 	test.AssertNotError(t, err, "orderTwo should have succeeded")
 }
@@ -4496,7 +4496,7 @@ func TestNewOrderFailedAuthzRateLimitingExempt(t *testing.T) {
 	test.AssertError(t, err, "expected error for domain with too many failures")
 
 	// Exempt the order from rate limiting.
-	exampleOrder.LimitsExempt = true
+	exampleOrder.IsARIRenewal = true
 	_, err = ra.NewOrder(ctx, exampleOrder)
 	test.AssertNotError(t, err, "limit exempt order should have succeeded")
 }

ratelimits/bucket.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/letsencrypt/boulder/core"
+	"github.com/letsencrypt/boulder/features"
 )
 
 // ErrInvalidCost indicates that the cost specified was < 0.
@@ -419,31 +420,37 @@ func (builder *TransactionBuilder) certificatesPerFQDNSetTransaction(orderNames
 //
 // Precondition: names must be a list of DNS names that all pass
 // policy.WellFormedDomainNames.
-func (builder *TransactionBuilder) NewOrderLimitTransactions(regId int64, names []string, maxNames int) ([]Transaction, error) {
+func (builder *TransactionBuilder) NewOrderLimitTransactions(regId int64, names []string, maxNames int, isRenewal bool) ([]Transaction, error) {
 	makeTxnError := func(err error, limit Name) error {
 		return fmt.Errorf("error constructing rate limit transaction for %s rate limit: %w", limit, err)
 	}
 
 	var transactions []Transaction
-	txn, err := builder.ordersPerAccountTransaction(regId)
-	if err != nil {
-		return nil, makeTxnError(err, NewOrdersPerAccount)
+	// TODO(#7511) Remove this feature flag check.
+	if features.Get().CheckRenewalExemptionAtWFE && !isRenewal {
+		txn, err := builder.ordersPerAccountTransaction(regId)
+		if err != nil {
+			return nil, makeTxnError(err, NewOrdersPerAccount)
+		}
+		transactions = append(transactions, txn)
 	}
-	transactions = append(transactions, txn)
 
-	failedAuthzTxns, err := builder.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(regId, names, maxNames)
+	txns, err := builder.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(regId, names, maxNames)
 	if err != nil {
 		return nil, makeTxnError(err, FailedAuthorizationsPerDomainPerAccount)
 	}
-	transactions = append(transactions, failedAuthzTxns...)
+	transactions = append(transactions, txns...)
 
-	certsPerDomainTxns, err := builder.certificatesPerDomainTransactions(regId, names, maxNames)
-	if err != nil {
-		return nil, makeTxnError(err, CertificatesPerDomain)
+	// TODO(#7511) Remove this feature flag check.
+	if features.Get().CheckRenewalExemptionAtWFE && !isRenewal {
+		txns, err := builder.certificatesPerDomainTransactions(regId, names, maxNames)
+		if err != nil {
+			return nil, makeTxnError(err, CertificatesPerDomain)
+		}
+		transactions = append(transactions, txns...)
 	}
-	transactions = append(transactions, certsPerDomainTxns...)
 
-	txn, err = builder.certificatesPerFQDNSetTransaction(names)
+	txn, err := builder.certificatesPerFQDNSetTransaction(names)
 	if err != nil {
 		return nil, makeTxnError(err, CertificatesPerFQDNSet)
 	}

test/config-next/ra.json

@@ -105,7 +105,8 @@
 			}
 		},
 		"features": {
-			"AsyncFinalize": true
+			"AsyncFinalize": true,
+			"CheckRenewalExemptionAtWFE": true
 		},
 		"ctLogs": {
 			"stagger": "500ms",

test/config-next/wfe2.json

@@ -128,7 +128,8 @@
 		},
 		"features": {
 			"ServeRenewalInfo": true,
-			"TrackReplacementCertificatesARI": true
+			"TrackReplacementCertificatesARI": true,
+			"CheckRenewalExemptionAtWFE": true
 		},
 		"certificateProfileNames": [
 			"defaultBoulderCertificateProfile"

test/integration/ratelimit_test.go

@@ -65,7 +65,7 @@ func TestDuplicateFQDNRateLimit(t *testing.T) {
 		test.AssertNotError(t, err, "making transaction composer")
 
 		// Check that the CertificatesPerFQDNSet limit is reached.
-		txns, err := txnBuilder.NewOrderLimitTransactions(1, []string{domain}, 100)
+		txns, err := txnBuilder.NewOrderLimitTransactions(1, []string{domain}, 100, false)
 		test.AssertNotError(t, err, "making transaction")
 		result, err := limiter.BatchSpend(context.Background(), txns)
 		test.AssertNotError(t, err, "checking transaction")

wfe2/wfe.go

@@ -2016,13 +2016,13 @@ func (wfe *WebFrontEndImpl) orderToOrderJSON(request *http.Request, order *corep
 // 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) func() {
+func (wfe *WebFrontEndImpl) checkNewOrderLimits(ctx context.Context, regId int64, names []string, isRenewal bool) func() {
 	if wfe.limiter == nil && wfe.txnBuilder == nil {
 		// Key-value rate limiting is disabled.
 		return nil
 	}
 
-	txns, err := wfe.txnBuilder.NewOrderLimitTransactions(regId, names, wfe.maxNames)
+	txns, err := wfe.txnBuilder.NewOrderLimitTransactions(regId, names, wfe.maxNames, isRenewal)
 	if err != nil {
 		wfe.log.Errf("building new order limit transactions: %v", err)
 		return nil
@@ -2271,31 +2271,45 @@ func (wfe *WebFrontEndImpl) NewOrder(
 	logEvent.DNSNames = names
 
 	var replaces string
-	var limitsExempt bool
+	var isARIRenewal bool
 	if features.Get().TrackReplacementCertificatesARI {
-		replaces, limitsExempt, err = wfe.validateReplacementOrder(ctx, acct, names, newOrderRequest.Replaces)
+		replaces, isARIRenewal, err = wfe.validateReplacementOrder(ctx, acct, names, newOrderRequest.Replaces)
 		if err != nil {
 			wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While validating order as a replacement an error occurred"), err)
 			return
 		}
 	}
 
+	var isRenewal bool
+	// TODO(#7511) Remove this feature flag check.
+	if features.Get().CheckRenewalExemptionAtWFE && !isARIRenewal {
+		// The Subscriber does not have an ARI exemption. However, we can check
+		// if the order is a renewal, and thus exempt from the NewOrdersPerAccount
+		// and CertificatesPerDomain limits.
+		exists, err := wfe.sa.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{Domains: names})
+		if err != nil {
+			wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While checking renewal exemption status"), err)
+			return
+		}
+		isRenewal = exists.Exists
+	}
+
 	err = wfe.validateCertificateProfileName(newOrderRequest.Profile)
 	if err != nil {
 		// TODO(#7392) Provide link to profile documentation.
 		wfe.sendError(response, logEvent, probs.Malformed("Invalid certificate profile, %q: %s", newOrderRequest.Profile, err), err)
 		return
 	}
 
-	refundLimits := wfe.checkNewOrderLimits(ctx, acct.ID, names)
+	refundLimits := wfe.checkNewOrderLimits(ctx, acct.ID, names, isRenewal)
 
 	var newOrderSuccessful bool
 	var errIsRateLimit bool
 	defer func() {
 		if features.Get().TrackReplacementCertificatesARI {
 			wfe.stats.ariReplacementOrders.With(prometheus.Labels{
 				"isReplacement": fmt.Sprintf("%t", replaces != ""),
-				"limitsExempt":  fmt.Sprintf("%t", limitsExempt),
+				"limitsExempt":  fmt.Sprintf("%t", isARIRenewal),
 			}).Inc()
 		}
 
@@ -2308,8 +2322,9 @@ func (wfe *WebFrontEndImpl) NewOrder(
 		RegistrationID:         acct.ID,
 		Names:                  names,
 		ReplacesSerial:         replaces,
-		LimitsExempt:           limitsExempt,
 		CertificateProfileName: newOrderRequest.Profile,
+		IsARIRenewal:           isARIRenewal,
+		IsRenewal:              isRenewal,
 	})
 	// 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) {