letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

wfe/features: Deprecate UseKvLimitsForNewOrder (#7765) 

Default code paths that depended on this flag to be true.

Part of #5545
This commit is contained in:
Samantha Frank 2024-10-23 18:13:24 -04:00 committed by GitHub
parent 844334e04a
commit e5edb7077f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 9 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
features/features.go
View File

@ -29,6 +29,7 @@ type Config struct {
CertCheckerRequiresCorrespondence bool
ECDSAForAll bool
CheckRenewalExemptionAtWFE bool
UseKvLimitsForNewAccount bool
// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
// GET requests. WARNING: This feature is a draft and highly unstable.
@ -104,11 +105,6 @@ type Config struct {
// fqdnSets tables at Finalize time.
UseKvLimitsForNewOrder bool
// UseKvLimitsForNewAccount when enabled, causes the key-value rate limiter
// to be the authoritative source of rate limiting information for
// new-account callers and disables the legacy rate limiting checks.
UseKvLimitsForNewAccount bool
// DisableLegacyLimitWrites when enabled, disables writes to:
// - the newOrdersRL table at new-order time, and
// - the certificatesPerName table at finalize time.

66
ra/ra.go
View File

@ -415,58 +415,6 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Contex
return nil
}
// checkRegistrationLimits enforces the RegistrationsPerIP and
// RegistrationsPerIPRange limits
func (ra *RegistrationAuthorityImpl) checkRegistrationLimits(ctx context.Context, ip net.IP) error {
// Check the registrations per IP limit using the CountRegistrationsByIP SA
// function that matches IP addresses exactly
exactRegLimit := ra.rlPolicies.RegistrationsPerIP()
if exactRegLimit.Enabled() {
started := ra.clk.Now()
err := ra.checkRegistrationIPLimit(ctx, exactRegLimit, ip, ra.SA.CountRegistrationsByIP)
elapsed := ra.clk.Since(started)
if err != nil {
if errors.Is(err, berrors.RateLimit) {
ra.rlCheckLatency.WithLabelValues(ratelimit.RegistrationsPerIP, ratelimits.Denied).Observe(elapsed.Seconds())
ra.log.Infof("Rate limit exceeded, RegistrationsPerIP, by IP: %q", ip)
}
return err
}
ra.rlCheckLatency.WithLabelValues(ratelimit.RegistrationsPerIP, ratelimits.Allowed).Observe(elapsed.Seconds())
}
// We only apply the fuzzy reg limit to IPv6 addresses.
// Per https://golang.org/pkg/net/#IP.To4 "If ip is not an IPv4 address, To4
// returns nil"
if ip.To4() != nil {
return nil
}
// Check the registrations per IP range limit using the
// CountRegistrationsByIPRange SA function that fuzzy-matches IPv6 addresses
// within a larger address range
fuzzyRegLimit := ra.rlPolicies.RegistrationsPerIPRange()
if fuzzyRegLimit.Enabled() {
started := ra.clk.Now()
err := ra.checkRegistrationIPLimit(ctx, fuzzyRegLimit, ip, ra.SA.CountRegistrationsByIPRange)
elapsed := ra.clk.Since(started)
if err != nil {
if errors.Is(err, berrors.RateLimit) {
ra.rlCheckLatency.WithLabelValues(ratelimit.RegistrationsPerIPRange, ratelimits.Denied).Observe(elapsed.Seconds())
ra.log.Infof("Rate limit exceeded, RegistrationsByIPRange, IP: %q", ip)
// For the fuzzyRegLimit we use a new error message that specifically
// mentions that the limit being exceeded is applied to a *range* of IPs
return berrors.RateLimitError(0, "too many registrations for this IP range")
}
return err
}
ra.rlCheckLatency.WithLabelValues(ratelimit.RegistrationsPerIPRange, ratelimits.Allowed).Observe(elapsed.Seconds())
}
return nil
}
// NewRegistration constructs a new Registration from a request.
func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, request *corepb.Registration) (*corepb.Registration, error) {
// Error if the request is nil, there is no account key or IP address
@ -485,20 +433,6 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, reques
return nil, berrors.MalformedError("invalid public key: %s", err.Error())
}
// Check IP address rate limits.
var ipAddr net.IP
err = ipAddr.UnmarshalText(request.InitialIP)
if err != nil {
return nil, berrors.InternalServerError("failed to unmarshal ip address: %s", err.Error())
}
if !features.Get().UseKvLimitsForNewAccount {
err = ra.checkRegistrationLimits(ctx, ipAddr)
if err != nil {
return nil, err
}
}
// Check that contacts conform to our expectations.
err = validateContactsPresent(request.Contact, request.ContactsPresent)
if err != nil {

84
ra/ra_test.go
View File

@ -628,90 +628,6 @@ func TestNewRegistrationBadKey(t *testing.T) {
test.AssertError(t, err, "Should have rejected authorization with short key")
}
func TestNewRegistrationRateLimit(t *testing.T) {
_, _, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()
// Specify a dummy rate limit policy that allows 1 registration per exact IP
// match, and 2 per range.
ra.rlPolicies = &dummyRateLimitConfig{
RegistrationsPerIPPolicy: ratelimit.RateLimitPolicy{
Threshold: 1,
Window: config.Duration{Duration: 24 * 90 * time.Hour},
},
RegistrationsPerIPRangePolicy: ratelimit.RateLimitPolicy{
Threshold: 2,
Window: config.Duration{Duration: 24 * 90 * time.Hour},
},
}
// Create one registration for an IPv4 address
mailto := "mailto:foo@letsencrypt.org"
reg := &corepb.Registration{
Contact: []string{mailto},
ContactsPresent: true,
Key: newAcctKey(t),
InitialIP: parseAndMarshalIP(t, "7.6.6.5"),
}
// There should be no errors - it is within the RegistrationsPerIP rate limit
_, err := ra.NewRegistration(ctx, reg)
test.AssertNotError(t, err, "Unexpected error adding new IPv4 registration")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "decision": ratelimits.Allowed}, 1)
// There are no overrides for this IP, so the override usage gauge should
// contain 0 entries with labels matching it.
test.AssertMetricWithLabelsEquals(t, ra.rlOverrideUsageGauge, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "override_key": "7.6.6.5"}, 0)
// Create another registration for the same IPv4 address by changing the key
reg.Key = newAcctKey(t)
// There should be an error since a 2nd registration will exceed the
// RegistrationsPerIP rate limit
_, err = ra.NewRegistration(ctx, reg)
test.AssertError(t, err, "No error adding duplicate IPv4 registration")
test.AssertEquals(t, err.Error(), "too many registrations for this IP: see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "decision": ratelimits.Denied}, 1)
// Create a registration for an IPv6 address
reg.Key = newAcctKey(t)
reg.InitialIP = parseAndMarshalIP(t, "2001:cdba:1234:5678:9101:1121:3257:9652")
// There should be no errors - it is within the RegistrationsPerIP rate limit
_, err = ra.NewRegistration(ctx, reg)
test.AssertNotError(t, err, "Unexpected error adding a new IPv6 registration")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "decision": ratelimits.Allowed}, 2)
// Create a 2nd registration for the IPv6 address by changing the key
reg.Key = newAcctKey(t)
// There should be an error since a 2nd reg for the same IPv6 address will
// exceed the RegistrationsPerIP rate limit
_, err = ra.NewRegistration(ctx, reg)
test.AssertError(t, err, "No error adding duplicate IPv6 registration")
test.AssertEquals(t, err.Error(), "too many registrations for this IP: see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "decision": ratelimits.Denied}, 2)
// Create a registration for an IPv6 address in the same /48
reg.Key = newAcctKey(t)
reg.InitialIP = parseAndMarshalIP(t, "2001:cdba:1234:5678:9101:1121:3257:9653")
// There should be no errors since two IPv6 addresses in the same /48 is
// within the RegistrationsPerIPRange limit
_, err = ra.NewRegistration(ctx, reg)
test.AssertNotError(t, err, "Unexpected error adding second IPv6 registration in the same /48")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIPRange, "decision": ratelimits.Allowed}, 2)
// Create a registration for yet another IPv6 address in the same /48
reg.Key = newAcctKey(t)
reg.InitialIP = parseAndMarshalIP(t, "2001:cdba:1234:5678:9101:1121:3257:9654")
// There should be an error since three registrations within the same IPv6
// /48 is outside of the RegistrationsPerIPRange limit
_, err = ra.NewRegistration(ctx, reg)
test.AssertError(t, err, "No error adding a third IPv6 registration in the same /48")
test.AssertEquals(t, err.Error(), "too many registrations for this IP range: see https://letsencrypt.org/docs/rate-limits/")
test.AssertMetricWithLabelsEquals(t, ra.rlCheckLatency, prometheus.Labels{"limit": ratelimit.RegistrationsPerIPRange, "decision": ratelimits.Denied}, 1)
}
func TestRegistrationsPerIPOverrideUsage(t *testing.T) {
_, _, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()

3
test/config-next/ra.json
View File

@ -130,8 +130,7 @@
},
"features": {
"AsyncFinalize": true,
"UseKvLimitsForNewOrder": true,
"UseKvLimitsForNewAccount": true
"UseKvLimitsForNewOrder": true
},
"ctLogs": {
"stagger": "500ms",

3
test/config-next/wfe2.json
View File

@ -130,8 +130,7 @@
"ServeRenewalInfo": true,
"TrackReplacementCertificatesARI": true,
"CheckIdentifiersPaused": true,
"UseKvLimitsForNewOrder": true,
"UseKvLimitsForNewAccount": true
"UseKvLimitsForNewOrder": true
},
"certProfiles": {
"legacy": "The normal profile you know and love",

3
test/config/ra.json
View File

@ -107,7 +107,8 @@
}
},
"features": {
"CheckRenewalExemptionAtWFE": true
"CheckRenewalExemptionAtWFE": true,
"UseKvLimitsForNewAccount": true
},
"ctLogs": {
"stagger": "500ms",

3
test/config/wfe2.json
View File

@ -106,7 +106,8 @@
"pendingAuthorizationLifetimeDays": 7,
"features": {
"ServeRenewalInfo": true,
"CheckRenewalExemptionAtWFE": true
"CheckRenewalExemptionAtWFE": true,
"UseKvLimitsForNewAccount": true
}
},
"syslog": {

2
wfe2/wfe.go
View File

@ -792,10 +792,8 @@ func (wfe *WebFrontEndImpl) NewAccount(
refundLimits, err := wfe.checkNewAccountLimits(ctx, ip)
if err != nil {
if errors.Is(err, berrors.RateLimit) {
if features.Get().UseKvLimitsForNewAccount {
wfe.sendError(response, logEvent, probs.RateLimited(err.Error()), err)
return
}
} else {
wfe.log.Warning(err.Error())
}