RA: apply certificate rate limits at NewOrder time. (#4074)
If an order for a given set of names will fail finalization because of certificate rate limits (certs per domain, certs per fqdn set) there isn't any point in allowing an order for those names to be created. We can stop a lot of requests earlier by enforcing the cert rate limits at new order time as well as finalization time. A new RA `EarlyOrderRateLimit` feature flag controls whether this is done or not. Resolves #3975
This commit is contained in:
parent
b0574cd0de
commit
16e464a37d
|
|
@ -4,9 +4,9 @@ package features
|
|||
|
||||
import "strconv"
|
||||
|
||||
const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverAllowRenewalFirstRLTLSSNIRevalidationCAAValidationMethodsCAAAccountURIProbeCTLogsSimplifiedVAHTTPHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRASetIssuedNamesRenewalBit"
|
||||
const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverAllowRenewalFirstRLTLSSNIRevalidationCAAValidationMethodsCAAAccountURIProbeCTLogsSimplifiedVAHTTPHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRASetIssuedNamesRenewalBitEarlyOrderRateLimit"
|
||||
|
||||
var _FeatureFlag_index = [...]uint8{0, 6, 26, 43, 62, 80, 100, 113, 124, 140, 157, 179, 189, 213}
|
||||
var _FeatureFlag_index = [...]uint8{0, 6, 26, 43, 62, 80, 100, 113, 124, 140, 157, 179, 189, 213, 232}
|
||||
|
||||
func (i FeatureFlag) String() string {
|
||||
if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {
|
||||
|
|
|
|||
|
|
@ -38,6 +38,9 @@ const (
|
|||
// SetIssuedNamesRenewalBit enables the SA setting the renewal bit for
|
||||
// issuedNames entries during AddCertificate.
|
||||
SetIssuedNamesRenewalBit
|
||||
// EarlyOrderRateLimit enables the RA applying certificate per name/per FQDN
|
||||
// set rate limits in NewOrder in addition to FinalizeOrder.
|
||||
EarlyOrderRateLimit
|
||||
)
|
||||
|
||||
// List of features and their default value, protected by fMu
|
||||
|
|
@ -55,6 +58,7 @@ var features = map[FeatureFlag]bool{
|
|||
NewAuthorizationSchema: false,
|
||||
RevokeAtRA: false,
|
||||
SetIssuedNamesRenewalBit: false,
|
||||
EarlyOrderRateLimit: false,
|
||||
}
|
||||
|
||||
var fMu = new(sync.RWMutex)
|
||||
|
|
|
|||
9
ra/ra.go
9
ra/ra.go
|
|
@ -1779,6 +1779,15 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
|
|||
return nil, err
|
||||
}
|
||||
|
||||
if features.Enabled(features.EarlyOrderRateLimit) {
|
||||
// Check if there is rate limit space for issuing a certificate for the new
|
||||
// order's names. If there isn't then it doesn't make sense to allow creating
|
||||
// an order - it will just fail when finalization checks the same limits.
|
||||
if err := ra.checkLimits(ctx, order.Names, *order.RegistrationID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
// An order's lifetime is effectively bound by the shortest remaining lifetime
|
||||
// of its associated authorizations. For that reason it would be Uncool if
|
||||
// `sa.GetAuthorizations` returned an authorization that was very close to
|
||||
|
|
|
|||
|
|
@ -1199,6 +1199,69 @@ func TestNewOrderRateLimiting(t *testing.T) {
|
|||
test.AssertNotError(t, err, "NewOrder for orderTwo failed after advancing clock")
|
||||
}
|
||||
|
||||
// TestEarlyOrderRateLimiting tests that the EarlyOrderRateLimiting flag results
|
||||
// in NewOrder applying the certificates per name/per FQDN rate limits against
|
||||
// the order names.
|
||||
func TestEarlyOrderRateLimiting(t *testing.T) {
|
||||
_, _, ra, _, cleanUp := initAuthorities(t)
|
||||
defer cleanUp()
|
||||
ra.orderLifetime = 5 * 24 * time.Hour
|
||||
|
||||
rateLimitDuration := 5 * time.Minute
|
||||
|
||||
domain := "early-ratelimit-example.com"
|
||||
|
||||
// Set a mock RL policy with a CertificatesPerName threshold for the domain
|
||||
// name so low if it were enforced it would prevent a new order for any names.
|
||||
ra.rlPolicies = &dummyRateLimitConfig{
|
||||
CertificatesPerNamePolicy: ratelimit.RateLimitPolicy{
|
||||
Threshold: 10,
|
||||
Window: cmd.ConfigDuration{Duration: rateLimitDuration},
|
||||
// Setting the Threshold to 0 skips applying the rate limit. Setting an
|
||||
// override to 0 does the trick.
|
||||
Overrides: map[string]int{
|
||||
domain: 0,
|
||||
},
|
||||
},
|
||||
NewOrdersPerAccountPolicy: ratelimit.RateLimitPolicy{
|
||||
Threshold: 10,
|
||||
Window: cmd.ConfigDuration{Duration: rateLimitDuration},
|
||||
},
|
||||
}
|
||||
|
||||
// Start with the feature flag enabled.
|
||||
err := features.Set(map[string]bool{"EarlyOrderRateLimit": true})
|
||||
test.AssertNotError(t, err, "Failed to set EarlyOrderRateLimit feature flag")
|
||||
defer features.Reset()
|
||||
|
||||
// Request an order for the test domain
|
||||
newOrder := &rapb.NewOrderRequest{
|
||||
RegistrationID: &Registration.ID,
|
||||
Names: []string{domain},
|
||||
}
|
||||
|
||||
// With the feature flag enabled the NewOrder request should fail because of
|
||||
// the CertificatesPerNamePolicy.
|
||||
_, err = ra.NewOrder(ctx, newOrder)
|
||||
test.AssertError(t, err, "NewOrder did not apply cert rate limits with feature flag enabled")
|
||||
|
||||
// The err should be the expected rate limit error
|
||||
expectedErrPrefix := "too many certificates already issued for: " +
|
||||
"early-ratelimit-example.com"
|
||||
test.Assert(t,
|
||||
strings.HasPrefix(err.Error(), expectedErrPrefix),
|
||||
fmt.Sprintf("expected error to have prefix %q got %q", expectedErrPrefix, err))
|
||||
|
||||
// Reset the feature flags explicitly to disable EarlyOrderRateLimit
|
||||
features.Reset()
|
||||
|
||||
// The same NewOrder request should now succeed because EarlyOrderRateLimit
|
||||
// isn't enabled and the CertificatesPerNamePolicy won't be enforced until
|
||||
// finalization time.
|
||||
_, err = ra.NewOrder(ctx, newOrder)
|
||||
test.AssertNotError(t, err, "NewOrder applied cert rate limits with feature flag disabled")
|
||||
}
|
||||
|
||||
func TestAuthzFailedRateLimiting(t *testing.T) {
|
||||
_, _, ra, _, cleanUp := initAuthorities(t)
|
||||
defer cleanUp()
|
||||
|
|
|
|||
|
|
@ -46,7 +46,8 @@
|
|||
]
|
||||
},
|
||||
"features": {
|
||||
"RevokeAtRA": true
|
||||
"RevokeAtRA": true,
|
||||
"EarlyOrderRateLimit": true
|
||||
},
|
||||
"CTLogGroups2": [
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue