diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index c63d7a3a095..d844fb4a4e5 100644 --- a/cmd/boulder-ra/main.go +++ b/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, diff --git a/cmd/boulder-wfe2/main.go b/cmd/boulder-wfe2/main.go index 3da9928e965..ae6d7f94b78 100644 --- a/cmd/boulder-wfe2/main.go +++ b/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 } diff --git a/ra/ra.go b/ra/ra.go index b046e6c26ad..3d0a8926b57 100644 --- a/ra/ra.go +++ b/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 } diff --git a/ra/ra_test.go b/ra/ra_test.go index 101799256c9..2f272622669 100644 --- a/ra/ra_test.go +++ b/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) { diff --git a/ratelimits/bucket.go b/ratelimits/bucket.go index 764766c5ff0..18c425ee038 100644 --- a/ratelimits/bucket.go +++ b/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