Skip to content

Commit 16e464a

Browse files
cpuRoland Bracewell Shoemaker
cpu
authored and
Roland Bracewell Shoemaker
committed
RA: apply certificate rate limits at NewOrder time. (letsencrypt#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 letsencrypt#3975
1 parent b0574cd commit 16e464a

5 files changed

Lines changed: 80 additions & 3 deletions

File tree

‎features/featureflag_string.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎features/features.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ const (
3838
// SetIssuedNamesRenewalBit enables the SA setting the renewal bit for
3939
// issuedNames entries during AddCertificate.
4040
SetIssuedNamesRenewalBit
41+
// EarlyOrderRateLimit enables the RA applying certificate per name/per FQDN
42+
// set rate limits in NewOrder in addition to FinalizeOrder.
43+
EarlyOrderRateLimit
4144
)
4245

4346
// List of features and their default value, protected by fMu
@@ -55,6 +58,7 @@ var features = map[FeatureFlag]bool{
5558
NewAuthorizationSchema: false,
5659
RevokeAtRA: false,
5760
SetIssuedNamesRenewalBit: false,
61+
EarlyOrderRateLimit: false,
5862
}
5963

6064
var fMu = new(sync.RWMutex)

‎ra/ra.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,6 +1779,15 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
17791779
return nil, err
17801780
}
17811781

1782+
if features.Enabled(features.EarlyOrderRateLimit) {
1783+
// Check if there is rate limit space for issuing a certificate for the new
1784+
// order's names. If there isn't then it doesn't make sense to allow creating
1785+
// an order - it will just fail when finalization checks the same limits.
1786+
if err := ra.checkLimits(ctx, order.Names, *order.RegistrationID); err != nil {
1787+
return nil, err
1788+
}
1789+
}
1790+
17821791
// An order's lifetime is effectively bound by the shortest remaining lifetime
17831792
// of its associated authorizations. For that reason it would be Uncool if
17841793
// `sa.GetAuthorizations` returned an authorization that was very close to

‎ra/ra_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,6 +1199,69 @@ func TestNewOrderRateLimiting(t *testing.T) {
11991199
test.AssertNotError(t, err, "NewOrder for orderTwo failed after advancing clock")
12001200
}
12011201

1202+
// TestEarlyOrderRateLimiting tests that the EarlyOrderRateLimiting flag results
1203+
// in NewOrder applying the certificates per name/per FQDN rate limits against
1204+
// the order names.
1205+
func TestEarlyOrderRateLimiting(t *testing.T) {
1206+
_, _, ra, _, cleanUp := initAuthorities(t)
1207+
defer cleanUp()
1208+
ra.orderLifetime = 5 * 24 * time.Hour
1209+
1210+
rateLimitDuration := 5 * time.Minute
1211+
1212+
domain := "early-ratelimit-example.com"
1213+
1214+
// Set a mock RL policy with a CertificatesPerName threshold for the domain
1215+
// name so low if it were enforced it would prevent a new order for any names.
1216+
ra.rlPolicies = &dummyRateLimitConfig{
1217+
CertificatesPerNamePolicy: ratelimit.RateLimitPolicy{
1218+
Threshold: 10,
1219+
Window: cmd.ConfigDuration{Duration: rateLimitDuration},
1220+
// Setting the Threshold to 0 skips applying the rate limit. Setting an
1221+
// override to 0 does the trick.
1222+
Overrides: map[string]int{
1223+
domain: 0,
1224+
},
1225+
},
1226+
NewOrdersPerAccountPolicy: ratelimit.RateLimitPolicy{
1227+
Threshold: 10,
1228+
Window: cmd.ConfigDuration{Duration: rateLimitDuration},
1229+
},
1230+
}
1231+
1232+
// Start with the feature flag enabled.
1233+
err := features.Set(map[string]bool{"EarlyOrderRateLimit": true})
1234+
test.AssertNotError(t, err, "Failed to set EarlyOrderRateLimit feature flag")
1235+
defer features.Reset()
1236+
1237+
// Request an order for the test domain
1238+
newOrder := &rapb.NewOrderRequest{
1239+
RegistrationID: &Registration.ID,
1240+
Names: []string{domain},
1241+
}
1242+
1243+
// With the feature flag enabled the NewOrder request should fail because of
1244+
// the CertificatesPerNamePolicy.
1245+
_, err = ra.NewOrder(ctx, newOrder)
1246+
test.AssertError(t, err, "NewOrder did not apply cert rate limits with feature flag enabled")
1247+
1248+
// The err should be the expected rate limit error
1249+
expectedErrPrefix := "too many certificates already issued for: " +
1250+
"early-ratelimit-example.com"
1251+
test.Assert(t,
1252+
strings.HasPrefix(err.Error(), expectedErrPrefix),
1253+
fmt.Sprintf("expected error to have prefix %q got %q", expectedErrPrefix, err))
1254+
1255+
// Reset the feature flags explicitly to disable EarlyOrderRateLimit
1256+
features.Reset()
1257+
1258+
// The same NewOrder request should now succeed because EarlyOrderRateLimit
1259+
// isn't enabled and the CertificatesPerNamePolicy won't be enforced until
1260+
// finalization time.
1261+
_, err = ra.NewOrder(ctx, newOrder)
1262+
test.AssertNotError(t, err, "NewOrder applied cert rate limits with feature flag disabled")
1263+
}
1264+
12021265
func TestAuthzFailedRateLimiting(t *testing.T) {
12031266
_, _, ra, _, cleanUp := initAuthorities(t)
12041267
defer cleanUp()

‎test/config-next/ra.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@
4646
]
4747
},
4848
"features": {
49-
"RevokeAtRA": true
49+
"RevokeAtRA": true,
50+
"EarlyOrderRateLimit": true
5051
},
5152
"CTLogGroups2": [
5253
{

0 commit comments

Comments
 (0)