Address feedback

jprenken · jprenken · commit 8ba83475f7c3 · 2024-12-19T18:12:06.000-08:00
diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go
@@ -31,12 +31,12 @@ import (
 
 type Config struct {
 	RA struct {
-		// RateLimitPoliciesFilename is deprecated.
-		RateLimitPoliciesFilename string
-
 		cmd.ServiceConfig
 		cmd.HostnamePolicyConfig
 
+		// RateLimitPoliciesFilename is deprecated.
+		RateLimitPoliciesFilename string
+
 		MaxContactsPerRegistration int
 
 		SAService           *cmd.GRPCClientConfig
diff --git a/sa/proto/sa.proto b/sa/proto/sa.proto
@@ -146,11 +146,6 @@ message Timestamps {
   repeated google.protobuf.Timestamp timestamps = 2;
 }
 
-message CountByNames {
-  map<string, int64> counts = 1;
-  google.protobuf.Timestamp earliest = 2; // Unix timestamp (nanoseconds)
-}
-
 message CountInvalidAuthorizationsRequest {
   int64 registrationID = 1;
   string dnsName = 2;
diff --git a/sa/sa.go b/sa/sa.go
@@ -494,7 +494,7 @@ func (ssa *SQLStorageAuthority) AddCertificate(ctx context.Context, req *sapb.Ad
 	// but don't return an error from AddCertificate.
 	if fqdnTransactionErr != nil {
 		ssa.rateLimitWriteErrors.Inc()
-		ssa.log.AuditErrf("failed AddCertificate FQDN sets update transaction: %v", fqdnTransactionErr)
+		ssa.log.AuditErrf("failed AddCertificate FQDN sets insert transaction: %v", fqdnTransactionErr)
 	}
 
 	return &emptypb.Empty{}, nil
diff --git a/test/config/ra.json b/test/config/ra.json
@@ -22,8 +22,8 @@
 					"keyFile": "test/certs/ipki/wfe.boulder/key.pem"
 				}
 			},
-			"Defaults": "test/config-next/wfe2-ratelimit-defaults.yml",
-			"Overrides": "test/config-next/wfe2-ratelimit-overrides.yml"
+			"Defaults": "test/config/wfe2-ratelimit-defaults.yml",
+			"Overrides": "test/config/wfe2-ratelimit-overrides.yml"
 		},
 		"maxContactsPerRegistration": 3,
 		"debugAddr": ":8002",
@@ -129,6 +129,7 @@
 			}
 		},
 		"features": {
+			"UseKvLimitsForNewOrder": true,
 			"IncrementRateLimits": true
 		},
 		"ctLogs": {
diff --git a/test/config/sa.json b/test/config/sa.json
@@ -46,6 +46,9 @@
 					]
 				}
 			}
+		},
+		"features": {
+			"UseKvLimitsForNewOrder": true
 		}
 	},
 	"syslog": {
diff --git a/test/config/wfe2-ratelimit-defaults.yml b/test/config/wfe2-ratelimit-defaults.yml
@@ -0,0 +1,36 @@
+NewRegistrationsPerIPAddress:
+  count: 10000
+  burst: 10000
+  period: 168h
+NewRegistrationsPerIPv6Range:
+  count: 99999
+  burst: 99999
+  period: 168h
+CertificatesPerDomain:
+  count: 2
+  burst: 2
+  period: 2160h
+FailedAuthorizationsPerDomainPerAccount:
+  count: 3
+  burst: 3
+  period: 5m
+# The burst represents failing 40 times per day for 90 days. The count and
+# period grant one "freebie" failure per day. In combination, these parameters
+# mean that:
+# - Failing 120 times per day results in being paused after 30.25 days
+# - Failing 40 times per day results in being paused after 92.3 days
+# - Failing 20 times per day results in being paused after 6.2 months
+# - Failing 4 times per day results in being paused after 3.3 years
+# - Failing once per day results in never being paused
+FailedAuthorizationsForPausingPerDomainPerAccount:
+  count: 1
+  burst: 3600
+  period: 24h
+NewOrdersPerAccount:
+  count: 1500
+  burst: 1500
+  period: 3h
+CertificatesPerFQDNSet:
+  count: 2
+  burst: 2
+  period: 3h
diff --git a/test/config/wfe2-ratelimit-overrides.yml b/test/config/wfe2-ratelimit-overrides.yml
@@ -0,0 +1,64 @@
+- NewRegistrationsPerIPAddress:
+    burst: 1000000
+    count: 1000000
+    period: 168h
+    ids:
+      - id: 127.0.0.1
+        comment: localhost
+      - id: 10.77.77.77
+        comment: test
+      - id: 10.88.88.88
+        comment: test
+- CertificatesPerDomain:
+    burst: 1
+    count: 1
+    period: 2160h
+    ids:
+      - id: ratelimit.me
+        comment: Rate Limit Test Domain
+- CertificatesPerDomain:
+    burst: 10000
+    count: 10000
+    period: 2160h
+    ids:
+      - id: le.wtf
+        comment: Let's Encrypt Test Domain
+      - id: le1.wtf
+        comment: Let's Encrypt Test Domain 1
+      - id: le2.wtf
+        comment: Let's Encrypt Test Domain 2
+      - id: le3.wtf
+        comment: Let's Encrypt Test Domain 3
+      - id: nginx.wtf
+        comment: Nginx Test Domain
+      - id: good-caa-reserved.com
+        comment: Good CAA Reserved Domain
+      - id: bad-caa-reserved.com
+        comment: Bad CAA Reserved Domain
+      - id: ecdsa.le.wtf
+        comment: ECDSA Let's Encrypt Test Domain
+      - id: must-staple.le.wtf
+        comment: Must-Staple Let's Encrypt Test Domain
+- CertificatesPerFQDNSet:
+    burst: 10000
+    count: 10000
+    period: 168h
+    ids:
+      - id: le.wtf
+        comment: Let's Encrypt Test Domain
+      - id: le1.wtf
+        comment: Let's Encrypt Test Domain 1
+      - id: le2.wtf
+        comment: Let's Encrypt Test Domain 2
+      - id: le3.wtf
+        comment: Let's Encrypt Test Domain 3
+      - id: le.wtf,le1.wtf
+        comment: Let's Encrypt Test Domain, Let's Encrypt Test Domain 1
+      - id: good-caa-reserved.com
+        comment: Good CAA Reserved Domain
+      - id: nginx.wtf
+        comment: Nginx Test Domain
+      - id: ecdsa.le.wtf
+        comment: ECDSA Let's Encrypt Test Domain
+      - id: must-staple.le.wtf
+        comment: Must-Staple Let's Encrypt Test Domain
diff --git a/test/config/wfe2.json b/test/config/wfe2.json
@@ -123,10 +123,11 @@
 					"keyFile": "test/certs/ipki/wfe.boulder/key.pem"
 				}
 			},
-			"Defaults": "test/config-next/wfe2-ratelimit-defaults.yml",
-			"Overrides": "test/config-next/wfe2-ratelimit-overrides.yml"
+			"Defaults": "test/config/wfe2-ratelimit-defaults.yml",
+			"Overrides": "test/config/wfe2-ratelimit-overrides.yml"
 		},
 		"features": {
+			"UseKvLimitsForNewOrder": true,
 			"ServeRenewalInfo": true,
 			"IncrementRateLimits": true,
 			"CheckIdentifiersPaused": true

Original file line number	Diff line number	Diff line change
`@@ -494,7 +494,7 @@ func (ssa SQLStorageAuthority) AddCertificate(ctx context.Context, req sapb.Ad`
`494`	`494`	`// but don't return an error from AddCertificate.`
`495`	`495`	`if fqdnTransactionErr != nil {`
`496`	`496`	`ssa.rateLimitWriteErrors.Inc()`
`497`		`- ssa.log.AuditErrf("failed AddCertificate FQDN sets update transaction: %v", fqdnTransactionErr)`
	`497`	`+ ssa.log.AuditErrf("failed AddCertificate FQDN sets insert transaction: %v", fqdnTransactionErr)`
`498`	`498`	`}`
`499`	`499`
`500`	`500`	`return &emptypb.Empty{}, nil`
Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,9 @@`
`46`	`46`	`]`
`47`	`47`	`}`
`48`	`48`	`}`
	`49`	`+ },`
	`50`	`+ "features": {`
	`51`	`+ "UseKvLimitsForNewOrder": true`
`49`	`52`	`}`
`50`	`53`	`},`
`51`	`54`	`"syslog": {`