Skip to content

Commit

Permalink
ratelimits: Fix transaction building for Failed Authorizations Limit
Browse files Browse the repository at this point in the history
  • Loading branch information
beautifulentropy committed Feb 29, 2024
1 parent a97e074 commit 3a6a21e
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 43 deletions.
89 changes: 66 additions & 23 deletions ratelimits/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 26 additions & 14 deletions ratelimits/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/config-next/wfe2-ratelimit-defaults.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ CertificatesPerDomain:
count: 2
burst: 2
period: 2160h
FailedAuthorizationsPerAccount:
FailedAuthorizationsPerDomainPerAccount:
count: 3
burst: 3
period: 5m
Expand Down
10 changes: 5 additions & 5 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 3a6a21e

Please sign in to comment.