diff --git a/ratelimits/bucket.go b/ratelimits/bucket.go index b382ca536a2..75b54fa05a3 100644 --- a/ratelimits/bucket.go +++ b/ratelimits/bucket.go @@ -235,39 +235,82 @@ func (builder *TransactionBuilder) OrdersPerAccountTransaction(regId int64) (Tra return newTransaction(limit, bucketKey, 1) } -// FailedAuthorizationsPerAccountCheckOnlyTransaction returns a check-only -// Transaction for the provided ACME registration Id for the -// FailedAuthorizationsPerAccount limit. -func (builder *TransactionBuilder) FailedAuthorizationsPerAccountCheckOnlyTransaction(regId int64) (Transaction, error) { - bucketKey, err := newRegIdBucketKey(FailedAuthorizationsPerAccount, regId) - if err != nil { - return Transaction{}, err +// FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions 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 +// checking capacity, before allowing more authorizations to be created. +func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(regId int64, orderDomains []string, maxNames int) ([]Transaction, error) { + if len(orderDomains) > maxNames { + return nil, fmt.Errorf("order contains more than %d DNS names", maxNames) } - limit, err := builder.getLimit(FailedAuthorizationsPerAccount, bucketKey) + + // FailedAuthorizationsPerDomainPerAccount limit uses the 'enum:regId' + // bucket key format for overrides. + perAccountBucketKey, err := newRegIdBucketKey(FailedAuthorizationsPerDomainPerAccount, regId) if err != nil { - if errors.Is(err, errLimitDisabled) { - return newAllowOnlyTransaction() + return nil, err + } + limit, err := builder.getLimit(FailedAuthorizationsPerDomainPerAccount, perAccountBucketKey) + if err != nil && !errors.Is(err, errLimitDisabled) { + return nil, 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 } - return Transaction{}, err + + // Add a check-only transaction for each per domain per account bucket. + txn, err := newCheckOnlyTransaction(limit, perDomainPerAccountBucketKey, 1) + if err != nil { + return nil, err + } + txns = append(txns, txn) } - return newCheckOnlyTransaction(limit, bucketKey, 1) + return txns, nil } -// FailedAuthorizationsPerAccountTransaction returns a Transaction for the -// FailedAuthorizationsPerAccount limit for the provided ACME registration Id. -func (builder *TransactionBuilder) FailedAuthorizationsPerAccountTransaction(regId int64) (Transaction, error) { - bucketKey, err := newRegIdBucketKey(FailedAuthorizationsPerAccount, regId) - if err != nil { - return Transaction{}, err +// 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) } - limit, err := builder.getLimit(FailedAuthorizationsPerAccount, bucketKey) + + // FailedAuthorizationsPerDomainPerAccount limit uses the 'enum:regId' + // bucket key format for overrides. + perAccountBucketKey, err := newRegIdBucketKey(FailedAuthorizationsPerDomainPerAccount, regId) if err != nil { - if errors.Is(err, errLimitDisabled) { - return newAllowOnlyTransaction() + return nil, err + } + limit, err := builder.getLimit(FailedAuthorizationsPerDomainPerAccount, perAccountBucketKey) + if err != nil && !errors.Is(err, errLimitDisabled) { + return nil, 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 } - return Transaction{}, 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) } - return newTransaction(limit, bucketKey, 1) + return txns, nil } // CertificatesPerDomainTransactions returns a slice of Transactions for the diff --git a/ratelimits/names.go b/ratelimits/names.go index 4a541d1e657..c92970498b0 100644 --- a/ratelimits/names.go +++ b/ratelimits/names.go @@ -40,11 +40,14 @@ const ( // NewOrdersPerAccount uses bucket key 'enum:regId'. NewOrdersPerAccount - // FailedAuthorizationsPerAccount uses bucket key 'enum:regId', where regId - // is the ACME registration Id of the account. Cost MUST be consumed from - // this bucket only when the authorization is considered "failed". It SHOULD - // be checked before new authorizations are created. - FailedAuthorizationsPerAccount + // FailedAuthorizationsPerDomainPerAccount uses two different bucket keys + // depending on the context: + // - When referenced in an overrides file: uses bucket key 'enum:regId', + // where regId is the ACME registration Id of the account. + // - When referenced in a transaction: uses bucket key 'enum:regId:domain', + // where regId is the ACME registration Id of the account and domain is a + // domain name in the certificate. + FailedAuthorizationsPerDomainPerAccount // CertificatesPerDomain uses bucket key 'enum:domain', where domain is a // domain name in the certificate. @@ -96,14 +99,14 @@ func (n Name) EnumString() string { // nameToString is a map of Name values to string names. var nameToString = map[Name]string{ - Unknown: "Unknown", - NewRegistrationsPerIPAddress: "NewRegistrationsPerIPAddress", - NewRegistrationsPerIPv6Range: "NewRegistrationsPerIPv6Range", - NewOrdersPerAccount: "NewOrdersPerAccount", - FailedAuthorizationsPerAccount: "FailedAuthorizationsPerAccount", - CertificatesPerDomain: "CertificatesPerDomain", - CertificatesPerDomainPerAccount: "CertificatesPerDomainPerAccount", - CertificatesPerFQDNSet: "CertificatesPerFQDNSet", + Unknown: "Unknown", + NewRegistrationsPerIPAddress: "NewRegistrationsPerIPAddress", + NewRegistrationsPerIPv6Range: "NewRegistrationsPerIPv6Range", + NewOrdersPerAccount: "NewOrdersPerAccount", + FailedAuthorizationsPerDomainPerAccount: "FailedAuthorizationsPerDomainPerAccount", + CertificatesPerDomain: "CertificatesPerDomain", + CertificatesPerDomainPerAccount: "CertificatesPerDomainPerAccount", + CertificatesPerFQDNSet: "CertificatesPerFQDNSet", } // validIPAddress validates that the provided string is a valid IP address. @@ -202,10 +205,19 @@ func validateIdForName(name Name, id string) error { // 'enum:ipv6rangeCIDR' return validIPv6RangeCIDR(id) - case NewOrdersPerAccount, FailedAuthorizationsPerAccount: + case NewOrdersPerAccount: // 'enum:regId' return validateRegId(id) + case FailedAuthorizationsPerDomainPerAccount: + // 'enum:regId:domain' for transaction + err := validateRegIdDomain(id) + if err == nil { + return nil + } + // 'enum:regId' for overrides + return validateRegId(id) + case CertificatesPerDomainPerAccount: // 'enum:regId:domain' for transaction err := validateRegIdDomain(id) diff --git a/test/config-next/wfe2-ratelimit-defaults.yml b/test/config-next/wfe2-ratelimit-defaults.yml index f10b5966d30..0192c4bb340 100644 --- a/test/config-next/wfe2-ratelimit-defaults.yml +++ b/test/config-next/wfe2-ratelimit-defaults.yml @@ -10,7 +10,7 @@ CertificatesPerDomain: count: 2 burst: 2 period: 2160h -FailedAuthorizationsPerAccount: +FailedAuthorizationsPerDomainPerAccount: count: 3 burst: 3 period: 5m diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 26d198c1e23..55a0c4e84dc 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -2064,19 +2064,19 @@ func (wfe *WebFrontEndImpl) newNewOrderLimitTransactions(regId int64, names []st } transactions = append(transactions, txn) - txn, err = wfe.txnBuilder.FailedAuthorizationsPerAccountCheckOnlyTransaction(regId) + failedAuthzTxns, err := wfe.txnBuilder.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(regId, names, wfe.maxNames) if err != nil { - logTxnErr(err, ratelimits.FailedAuthorizationsPerAccount) + logTxnErr(err, ratelimits.FailedAuthorizationsPerDomainPerAccount) return nil } - transactions = append(transactions, txn) + transactions = append(transactions, failedAuthzTxns...) - txns, err := wfe.txnBuilder.CertificatesPerDomainTransactions(regId, names, wfe.maxNames) + certsPerDomainTxns, err := wfe.txnBuilder.CertificatesPerDomainTransactions(regId, names, wfe.maxNames) if err != nil { logTxnErr(err, ratelimits.CertificatesPerDomain) return nil } - transactions = append(transactions, txns...) + transactions = append(transactions, certsPerDomainTxns...) txn, err = wfe.txnBuilder.CertificatesPerFQDNSetTransaction(names) if err != nil {