RA: Count failed authorizations using key-value rate limits

cmd/boulder-ra/main.go

@@ -22,6 +22,8 @@ import (
 	pubpb "github.com/letsencrypt/boulder/publisher/proto"
 	"github.com/letsencrypt/boulder/ra"
 	rapb "github.com/letsencrypt/boulder/ra/proto"
+	"github.com/letsencrypt/boulder/ratelimits"
+	bredis "github.com/letsencrypt/boulder/redis"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	vapb "github.com/letsencrypt/boulder/va/proto"
 )
@@ -42,6 +44,33 @@ type Config struct {
 		PublisherService    *cmd.GRPCClientConfig
 		AkamaiPurgerService *cmd.GRPCClientConfig
 
+		Limiter struct {
+			// Redis contains the configuration necessary to connect to Redis
+			// for rate limiting. This field is required to enable rate
+			// limiting.
+			Redis *bredis.Config `validate:"required_with=Defaults"`
+
+			// Defaults is a path to a YAML file containing default rate limits.
+			// See: ratelimits/README.md for details. This field is required to
+			// enable rate limiting. If any individual rate limit is not set,
+			// that limit will be disabled. Limits passed in this file must be
+			// identical to those in the WFE.
+			//
+			// Note: At this time, only the Failed Authorizations rate limit is
+			// necessary in the RA.
+			Defaults string `validate:"required_with=Redis"`
+
+			// Overrides is a path to a YAML file containing overrides for the
+			// default rate limits. See: ratelimits/README.md for details. If
+			// this field is not set, all requesters will be subject to the
+			// default rate limits. Overrides passed in this file must be
+			// identical to those in the WFE.
+			//
+			// Note: At this time, only the Failed Authorizations overrides are
+			// necessary in the RA.
+			Overrides string
+		}
+
 		// MaxNames is the maximum number of subjectAltNames in a single cert.
 		// The value supplied MUST be greater than 0 and no more than 100. These
 		// limits are per section 7.1 of our combined CP/CPS, under "DV-SSL
@@ -232,12 +261,29 @@ func main() {
 		cmd.Fail("Error in RA config: MaxNames must not be 0")
 	}
 
+	var limiter *ratelimits.Limiter
+	var txnBuilder *ratelimits.TransactionBuilder
+	var limiterRedis *bredis.Ring
+	if c.RA.Limiter.Defaults != "" {
+		// Setup rate limiting.
+		limiterRedis, err = bredis.NewRingFromConfig(*c.RA.Limiter.Redis, scope, logger)
+		cmd.FailOnError(err, "Failed to create Redis ring")
+
+		source := ratelimits.NewRedisSource(limiterRedis.Ring, clk, scope)
+		limiter, err = ratelimits.NewLimiter(clk, source, scope)
+		cmd.FailOnError(err, "Failed to create rate limiter")
+		txnBuilder, err = ratelimits.NewTransactionBuilder(c.RA.Limiter.Defaults, c.RA.Limiter.Overrides)
+		cmd.FailOnError(err, "Failed to create rate limits transaction builder")
+	}
+
 	rai := ra.NewRegistrationAuthorityImpl(
 		clk,
 		logger,
 		scope,
 		c.RA.MaxContactsPerRegistration,
 		kp,
+		limiter,
+		txnBuilder,
 		c.RA.MaxNames,
 		authorizationLifetime,
 		pendingAuthorizationLifetime,

cmd/boulder-wfe2/main.go

@@ -149,13 +149,16 @@ type Config struct {
 			// Defaults is a path to a YAML file containing default rate limits.
 			// See: ratelimits/README.md for details. This field is required to
 			// enable rate limiting. If any individual rate limit is not set,
-			// that limit will be disabled.
+			// that limit will be disabled. Failed Authorizations limits passed
+			// in this file must be identical to those in the RA.
 			Defaults string `validate:"required_with=Redis"`
 
 			// Overrides is a path to a YAML file containing overrides for the
 			// default rate limits. See: ratelimits/README.md for details. If
 			// this field is not set, all requesters will be subject to the
-			// default rate limits.
+			// default rate limits. Overrides for the Failed Authorizations
+			// overrides passed in this file must be identical to those in the
+			// RA.
 			Overrides string
 		}
 

ra/ra.go

@@ -96,6 +96,8 @@ type RegistrationAuthorityImpl struct {
 	pendingAuthorizationLifetime time.Duration
 	rlPolicies                   ratelimit.Limits
 	maxContactsPerReg            int
+	limiter                      *ratelimits.Limiter
+	txnBuilder                   *ratelimits.TransactionBuilder
 	maxNames                     int
 	orderLifetime                time.Duration
 	finalizeTimeout              time.Duration
@@ -127,6 +129,8 @@ func NewRegistrationAuthorityImpl(
 	stats prometheus.Registerer,
 	maxContactsPerReg int,
 	keyPolicy goodkey.KeyPolicy,
+	limiter *ratelimits.Limiter,
+	txnBuilder *ratelimits.TransactionBuilder,
 	maxNames int,
 	authorizationLifetime time.Duration,
 	pendingAuthorizationLifetime time.Duration,
@@ -246,6 +250,8 @@ func NewRegistrationAuthorityImpl(
 		rlPolicies:                   ratelimit.New(),
 		maxContactsPerReg:            maxContactsPerReg,
 		keyPolicy:                    keyPolicy,
+		limiter:                      limiter,
+		txnBuilder:                   txnBuilder,
 		maxNames:                     maxNames,
 		publisher:                    pubc,
 		caa:                          caaClient,
@@ -1756,6 +1762,26 @@ func (ra *RegistrationAuthorityImpl) recordValidation(ctx context.Context, authI
 	return err
 }
 
+func (ra *RegistrationAuthorityImpl) countFailedValidation(ctx context.Context, regId int64, name string) {
+	if ra.limiter == nil || ra.txnBuilder == nil {
+		// Limiter is disabled.
+		return
+	}
+
+	txn, err := ra.txnBuilder.FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction(regId, name)
+	if err != nil {
+		ra.log.Errf("constructing rate limit transaction for the %s rate limit: %s", ratelimits.FailedAuthorizationsPerDomainPerAccount, err)
+	}
+
+	_, err = ra.limiter.Spend(ctx, txn)
+	if err != nil {
+		if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+			return
+		}
+		ra.log.Errf("checking the %s rate limit: %s", ratelimits.FailedAuthorizationsPerDomainPerAccount, err)
+	}
+}
+
 // PerformValidation initiates validation for a specific challenge associated
 // with the given base authorization. The authorization and challenge are
 // updated based on the results.
@@ -1884,6 +1910,13 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
 		if prob != nil {
 			challenge.Status = core.StatusInvalid
 			challenge.Error = prob
+
+			// TODO(#5545): Spending can be async until key-value rate limits
+			// are authoritative. This saves us from adding latency to each
+			// request. Goroutines spun out below will respect a context
+			// deadline set by the ratelimits package and cannot be prematurely
+			// canceled by the requester.
+			go ra.countFailedValidation(vaCtx, authz.RegistrationID, authz.Identifier.Value)
 		} else {
 			challenge.Status = core.StatusValid
 		}

ra/ra_test.go

@@ -17,6 +17,7 @@ import (
 	"math/big"
 	mrand "math/rand"
 	"net"
+	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -38,6 +39,7 @@ import (
 
 	akamaipb "github.com/letsencrypt/boulder/akamai/proto"
 	capb "github.com/letsencrypt/boulder/ca/proto"
+	"github.com/letsencrypt/boulder/cmd"
 	"github.com/letsencrypt/boulder/config"
 	"github.com/letsencrypt/boulder/core"
 	corepb "github.com/letsencrypt/boulder/core/proto"
@@ -56,6 +58,7 @@ import (
 	rapb "github.com/letsencrypt/boulder/ra/proto"
 	"github.com/letsencrypt/boulder/ratelimit"
 	"github.com/letsencrypt/boulder/ratelimits"
+	bredis "github.com/letsencrypt/boulder/redis"
 	"github.com/letsencrypt/boulder/sa"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	"github.com/letsencrypt/boulder/test"
@@ -365,9 +368,40 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, sapb.StorageAutho
 		},
 	}, nil, nil, 0, log, metrics.NoopRegisterer)
 
+	var limiter *ratelimits.Limiter
+	var txnBuilder *ratelimits.TransactionBuilder
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		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",
+		}
+		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")
+	}
+
 	ra := NewRegistrationAuthorityImpl(
 		fc, log, stats,
-		1, testKeyPolicy, 100,
+		1, testKeyPolicy, limiter, txnBuilder, 100,
 		300*24*time.Hour, 7*24*time.Hour,
 		nil, noopCAA{},
 		0, 5*time.Minute,
@@ -857,6 +891,19 @@ func TestPerformValidationSuccess(t *testing.T) {
 		Problems: nil,
 	}
 
+	var remainingFailedValidations int64
+	var rlTxns []ratelimits.Transaction
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		// Gather a baseline for the rate limit.
+		var err error
+		rlTxns, err = ra.txnBuilder.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(authzPB.RegistrationID, []string{Identifier}, 100)
+		test.AssertNotError(t, err, "FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions failed")
+
+		d, err := ra.limiter.BatchSpend(ctx, rlTxns)
+		test.AssertNotError(t, err, "BatchSpend failed")
+		remainingFailedValidations = d.Remaining
+	}
+
 	now := fc.Now()
 	challIdx := dnsChallIdx(t, authzPB.Challenges)
 	authzPB, err := ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
@@ -898,6 +945,13 @@ func TestPerformValidationSuccess(t *testing.T) {
 	// Check that validated timestamp was recorded, stored, and retrieved
 	expectedValidated := fc.Now()
 	test.Assert(t, *challenge.Validated == expectedValidated, "Validated timestamp incorrect or missing")
+
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		// The failed validations bucket should be identical to the baseline.
+		d, err := ra.limiter.BatchSpend(ctx, rlTxns)
+		test.AssertNotError(t, err, "BatchSpend failed")
+		test.AssertEquals(t, d.Remaining, remainingFailedValidations)
+	}
 }
 
 func TestPerformValidationVAError(t *testing.T) {
@@ -906,6 +960,19 @@ func TestPerformValidationVAError(t *testing.T) {
 
 	authzPB := createPendingAuthorization(t, sa, Identifier, fc.Now().Add(12*time.Hour))
 
+	var remainingFailedValidations int64
+	var rlTxns []ratelimits.Transaction
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		// Gather a baseline for the rate limit.
+		var err error
+		rlTxns, err = ra.txnBuilder.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(authzPB.RegistrationID, []string{Identifier}, 100)
+		test.AssertNotError(t, err, "FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions failed")
+
+		d, err := ra.limiter.BatchSpend(ctx, rlTxns)
+		test.AssertNotError(t, err, "BatchSpend failed")
+		remainingFailedValidations = d.Remaining
+	}
+
 	va.PerformValidationRequestResultError = fmt.Errorf("Something went wrong")
 
 	challIdx := dnsChallIdx(t, authzPB.Challenges)
@@ -945,6 +1012,13 @@ func TestPerformValidationVAError(t *testing.T) {
 	// Check that validated timestamp was recorded, stored, and retrieved
 	expectedValidated := fc.Now()
 	test.Assert(t, *challenge.Validated == expectedValidated, "Validated timestamp incorrect or missing")
+
+	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		// The failed validations bucket should have been decremented by 1.
+		d, err := ra.limiter.BatchSpend(ctx, rlTxns)
+		test.AssertNotError(t, err, "BatchSpend failed")
+		test.AssertEquals(t, d.Remaining, remainingFailedValidations-1)
+	}
 }
 
 func TestCertificateKeyNotEqualAccountKey(t *testing.T) {

ratelimits/bucket.go

@@ -276,43 +276,34 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountCheckO
 	return txns, nil
 }
 
-// FailedAuthorizationsPerDomainPerAccountSpendOnlyTransactions returns a slice
-// of Transactions for the provided order domain names. An error is returned if
-// any of the order domain names are invalid. This method should be used for
-// spending capacity, as a result of a failed authorization.
-func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountSpendOnlyTransactions(regId int64, orderDomains []string, maxNames int) ([]Transaction, error) {
-	if len(orderDomains) > maxNames {
-		return nil, fmt.Errorf("order contains more than %d DNS names", maxNames)
-	}
-
+// FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction returns a spend-
+// only Transaction for the provided order domain name. An error is returned if
+// the order domain name is invalid. This method should be used for spending
+// capacity, as a result of a failed authorization.
+func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction(regId int64, orderDomain string) (Transaction, error) {
 	// FailedAuthorizationsPerDomainPerAccount limit uses the 'enum:regId'
 	// bucket key format for overrides.
 	perAccountBucketKey, err := newRegIdBucketKey(FailedAuthorizationsPerDomainPerAccount, regId)
 	if err != nil {
-		return nil, err
+		return Transaction{}, err
 	}
 	limit, err := builder.getLimit(FailedAuthorizationsPerDomainPerAccount, perAccountBucketKey)
 	if err != nil && !errors.Is(err, errLimitDisabled) {
-		return nil, err
+		return Transaction{}, err
 	}
 
-	var txns []Transaction
-	for _, name := range DomainsForRateLimiting(orderDomains) {
-		// FailedAuthorizationsPerDomainPerAccount limit uses the
-		// 'enum:regId:domain' bucket key format for transactions.
-		perDomainPerAccountBucketKey, err := newRegIdDomainBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, name)
-		if err != nil {
-			return nil, err
-		}
-
-		// Add an allow-only transaction for each per domain per account bucket.
-		txn, err := newSpendOnlyTransaction(limit, perDomainPerAccountBucketKey, 1)
-		if err != nil {
-			return nil, err
-		}
-		txns = append(txns, txn)
+	// FailedAuthorizationsPerDomainPerAccount limit uses the
+	// 'enum:regId:domain' bucket key format for transactions.
+	perDomainPerAccountBucketKey, err := newRegIdDomainBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, orderDomain)
+	if err != nil {
+		return Transaction{}, err
 	}
-	return txns, nil
+	txn, err := newSpendOnlyTransaction(limit, perDomainPerAccountBucketKey, 1)
+	if err != nil {
+		return Transaction{}, err
+	}
+
+	return txn, nil
 }
 
 // CertificatesPerDomainTransactions returns a slice of Transactions for the