From 18c6c2a6d4028a49cc6df35ad355741c5b297be4 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Tue, 26 May 2026 16:16:25 -0700 Subject: [PATCH 01/15] Added rate limiter registry and test --- evmrpc/config/config.go | 69 ++++++++++++ ratelimiter/registry.go | 203 +++++++++++++++++++++++++++++++++++ ratelimiter/registry_test.go | 203 +++++++++++++++++++++++++++++++++++ 3 files changed, 475 insertions(+) create mode 100644 ratelimiter/registry.go create mode 100644 ratelimiter/registry_test.go diff --git a/evmrpc/config/config.go b/evmrpc/config/config.go index 9b67a55fc8..0a3335106b 100644 --- a/evmrpc/config/config.go +++ b/evmrpc/config/config.go @@ -156,6 +156,23 @@ type Config struct { // SS-pebble. Requires MemiavlOnly write mode; falls back transparently. TraceBakeUseSnapshot bool `mapstructure:"trace_bake_use_snapshot"` TraceBakeSnapshotWindow int64 `mapstructure:"trace_bake_snapshot_window"` // recent snapshots to keep (default 64) + + // RateLimitingEnabled is a temporary Phase-1 rollout gate for the RateLimiterRegistry. + // Set to false to pass all requests through without rate limiting. + // Will be removed in Phase 3 once the feature has stabilised in production. + RateLimitingEnabled bool `mapstructure:"rate_limiting_enabled"` + + // IPRateLimitRPS is the per-IP sustained request rate in requests/second. + // Zero disables per-IP rate limiting (all requests pass through). + IPRateLimitRPS float64 `mapstructure:"ip_rate_limit_rps"` + + // IPRateLimitBurst is the maximum per-IP burst size. + IPRateLimitBurst int `mapstructure:"ip_rate_limit_burst"` + + // TrustedProxyCIDRs lists CIDR ranges whose X-Forwarded-For headers are trusted + // for real-client-IP extraction. Defaults to RFC-1918 + loopback. + // Set to an empty list when no reverse proxy sits in front of the node. + TrustedProxyCIDRs []string `mapstructure:"trusted_proxy_cidrs"` } var DefaultConfig = Config{ @@ -200,6 +217,17 @@ var DefaultConfig = Config{ TraceBakeWindowBlocks: 0, TraceBakeUseSnapshot: false, TraceBakeSnapshotWindow: 64, + RateLimitingEnabled: true, + IPRateLimitRPS: 200, + IPRateLimitBurst: 400, + TrustedProxyCIDRs: []string{ + "127.0.0.0/8", + "::1/128", + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "fc00::/7", + }, } const ( @@ -240,6 +268,10 @@ const ( flagTraceBakeWindowBlocks = "evm.trace_bake_window_blocks" flagTraceBakeUseSnapshot = "evm.trace_bake_use_snapshot" flagTraceBakeSnapshotWindow = "evm.trace_bake_snapshot_window" + flagRateLimitingEnabled = "evm.rate_limiting_enabled" + flagIPRateLimitRPS = "evm.ip_rate_limit_rps" + flagIPRateLimitBurst = "evm.ip_rate_limit_burst" + flagTrustedProxyCIDRs = "evm.trusted_proxy_cidrs" ) func ReadConfig(opts servertypes.AppOptions) (Config, error) { @@ -430,6 +462,26 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) { return cfg, err } } + if v := opts.Get(flagRateLimitingEnabled); v != nil { + if cfg.RateLimitingEnabled, err = cast.ToBoolE(v); err != nil { + return cfg, err + } + } + if v := opts.Get(flagIPRateLimitRPS); v != nil { + if cfg.IPRateLimitRPS, err = cast.ToFloat64E(v); err != nil { + return cfg, err + } + } + if v := opts.Get(flagIPRateLimitBurst); v != nil { + if cfg.IPRateLimitBurst, err = cast.ToIntE(v); err != nil { + return cfg, err + } + } + if v := opts.Get(flagTrustedProxyCIDRs); v != nil { + if cfg.TrustedProxyCIDRs, err = cast.ToStringSliceE(v); err != nil { + return cfg, err + } + } return cfg, nil } @@ -618,4 +670,21 @@ trace_bake_use_snapshot = {{ .EVM.TraceBakeUseSnapshot }} # Number of recent memiavl snapshots to retain for trace baking. trace_bake_snapshot_window = {{ .EVM.TraceBakeSnapshotWindow }} + +# rate_limiting_enabled is a temporary Phase-1 rollout gate for the per-IP RateLimiterRegistry. +# Set to false to disable rate limiting entirely without tuning individual RPS values. +# This flag will be removed in Phase 3 once the feature has stabilised in production. +rate_limiting_enabled = {{ .EVM.RateLimitingEnabled }} + +# ip_rate_limit_rps is the per-IP sustained request rate in requests/second. +# Set to 0 to disable per-IP rate limiting (all requests pass through). +ip_rate_limit_rps = {{ .EVM.IPRateLimitRPS }} + +# ip_rate_limit_burst is the maximum per-IP burst above the sustained rate. +ip_rate_limit_burst = {{ .EVM.IPRateLimitBurst }} + +# trusted_proxy_cidrs lists CIDR ranges whose X-Forwarded-For headers are trusted +# for real-client-IP extraction. Defaults to RFC-1918 + loopback. +# Set to an empty list when no reverse proxy sits in front of the node. +trusted_proxy_cidrs = [{{- range .EVM.TrustedProxyCIDRs }}"{{ . }}", {{- end }}] ` diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go new file mode 100644 index 0000000000..84b80fe44a --- /dev/null +++ b/ratelimiter/registry.go @@ -0,0 +1,203 @@ +package ratelimiter + +import ( + "context" + "net" + "net/http" + "strings" + "time" + + "github.com/hashicorp/golang-lru/v2/expirable" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "golang.org/x/time/rate" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" +) + +const ( + DefaultRPS = 200.0 + DefaultBurst = 400 + + // lruSize bounds memory to ~8 MB at 50k entries (~160 bytes each). + lruSize = 50_000 + // lruTTL evicts inactive IP entries after 1 hour. + lruTTL = time.Hour +) + +// DefaultTrustedProxyCIDRs contains RFC-1918 ranges and loopback addresses. +// Requests arriving from these CIDRs are trusted to supply a valid X-Forwarded-For header. +var DefaultTrustedProxyCIDRs = []string{ + "127.0.0.0/8", + "::1/128", + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "fc00::/7", +} + +// Config holds the configuration for a Registry +type Config struct { + // Enabled is a temporary rollout gate (Phase 1 only). False passes all requests through. + Enabled bool + // RPS is the sustained request rate allowed per IP in requests/second. + // Zero disables per-IP rate limiting (all requests pass). + RPS float64 + // Burst is the maximum number of requests allowed in a single burst. + Burst int + // TrustedProxyCIDRs lists CIDRs whose X-Forwarded-For headers are trusted. + // Empty means trust no proxy; use RemoteAddr / peer address directly. + TrustedProxyCIDRs []string +} + +var DefaultConfig = Config{ + Enabled: true, + RPS: DefaultRPS, + Burst: DefaultBurst, + TrustedProxyCIDRs: DefaultTrustedProxyCIDRs, +} + +var ( + registryMeter = otel.Meter("ratelimiter") + + registryMetrics = struct { + rejectedCounter metric.Int64Counter + }{ + rejectedCounter: must(registryMeter.Int64Counter( + "rpc_rate_limit_rejected_total", + metric.WithDescription("Total RPC requests rejected by the per-IP rate limiter"), + metric.WithUnit("{request}"), + )), + } +) + +func must[V any](v V, err error) V { + if err != nil { + panic(err) + } + return v +} + +// Registry is a per-IP token-bucket rate limiter backed by an expirable LRU. +// It is safe for concurrent use. +type Registry struct { + cfg Config + networks []*net.IPNet + lru *expirable.LRU[string, *rate.Limiter] +} + +// New creates a Registry from cfg. Invalid CIDRs in TrustedProxyCIDRs are silently skipped. +func New(cfg Config) *Registry { + return &Registry{ + cfg: cfg, + networks: parseCIDRs(cfg.TrustedProxyCIDRs), + lru: expirable.NewLRU[string, *rate.Limiter](lruSize, nil, lruTTL), + } +} + +// Allow reports whether the request from ip should be allowed for the given plane and method. +// Rejections increment rpc_rate_limit_rejected_total{plane, method}. +func (r *Registry) Allow(ctx context.Context, ip, plane, method string) bool { + if !r.cfg.Enabled || r.cfg.RPS == 0 { + return true + } + if r.getOrCreate(ip).Allow() { + return true + } + registryMetrics.rejectedCounter.Add( + ctx, + 1, + metric.WithAttributes( + attribute.String("plane", plane), + attribute.String("method", method), + ), + ) + return false +} + +// IPFromHTTPRequest extracts the client IP from an HTTP request. +// If RemoteAddr belongs to a trusted proxy CIDR, the leftmost X-Forwarded-For entry is used. +func (r *Registry) IPFromHTTPRequest(req *http.Request) string { + remoteIP := stripPort(req.RemoteAddr) + if r.isTrustedProxy(remoteIP) { + if xff := req.Header.Get("X-Forwarded-For"); xff != "" { + return firstEntry(xff) + } + } + return remoteIP +} + +// IPFromGRPCContext extracts the client IP from a gRPC request context. +// If the transport peer belongs to a trusted proxy CIDR, the leftmost x-forwarded-for +// metadata value is used. +func (r *Registry) IPFromGRPCContext(ctx context.Context) string { + peerIP := grpcPeerIP(ctx) + if peerIP != "" && r.isTrustedProxy(peerIP) { + if md, ok := metadata.FromIncomingContext(ctx); ok { + if vals := md.Get("x-forwarded-for"); len(vals) > 0 { + return firstEntry(vals[0]) + } + } + } + return peerIP +} + +// getOrCreate returns the existing limiter for ip or creates a fresh one. +// A brief TOCTOU race can occur under high concurrency: at most one extra burst token +// leaks per race window, which has no meaningful security impact. +func (r *Registry) getOrCreate(ip string) *rate.Limiter { + if l, ok := r.lru.Get(ip); ok { + return l + } + l := rate.NewLimiter(rate.Limit(r.cfg.RPS), r.cfg.Burst) + r.lru.Add(ip, l) + return l +} + +func (r *Registry) isTrustedProxy(ip string) bool { + parsed := net.ParseIP(ip) + if parsed == nil { + return false + } + for _, n := range r.networks { + if n.Contains(parsed) { + return true + } + } + return false +} + +func parseCIDRs(cidrs []string) []*net.IPNet { + out := make([]*net.IPNet, 0, len(cidrs)) + for _, cidr := range cidrs { + if _, network, err := net.ParseCIDR(cidr); err == nil { + out = append(out, network) + } + } + return out +} + +func stripPort(addr string) string { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return addr + } + return host +} + +func firstEntry(xff string) string { + before, _, found := strings.Cut(xff, ",") + if found { + return strings.TrimSpace(before) + } + return strings.TrimSpace(xff) +} + +func grpcPeerIP(ctx context.Context) string { + p, ok := peer.FromContext(ctx) + if !ok || p.Addr == nil { + return "" + } + return stripPort(p.Addr.String()) +} diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go new file mode 100644 index 0000000000..cc768a72b6 --- /dev/null +++ b/ratelimiter/registry_test.go @@ -0,0 +1,203 @@ +package ratelimiter + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" + + "net" +) + +// disabledCfg has rate limiting turned off. +var disabledCfg = Config{Enabled: false, RPS: 100, Burst: 10} + +// zeroCfg has RPS=0 which also disables limiting. +var zeroCfg = Config{Enabled: true, RPS: 0, Burst: 10} + +func cfg(rps float64, burst int, cidrs ...string) Config { + return Config{Enabled: true, RPS: rps, Burst: burst, TrustedProxyCIDRs: cidrs} +} + +// --- Allow --- + +func TestAllow_DisabledAlwaysPasses(t *testing.T) { + r := New(disabledCfg) + for range 1000 { + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm", "eth_call")) + } +} + +func TestAllow_ZeroRPSAlwaysPasses(t *testing.T) { + r := New(zeroCfg) + for range 1000 { + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm", "eth_call")) + } +} + +func TestAllow_BurstThenReject(t *testing.T) { + // burst=3, RPS tiny so no token refill during test + r := New(cfg(0.001, 3)) + ip := "10.0.0.1" + require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "first request in burst") + require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "second request in burst") + require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "third request in burst") + require.False(t, r.Allow(t.Context(), ip, "evm", "m"), "must be rejected after burst exhausted") +} + +func TestAllow_PerIPIsolation(t *testing.T) { + r := New(cfg(0.001, 1)) + require.True(t, r.Allow(t.Context(), "1.1.1.1", "evm", "m")) + require.False(t, r.Allow(t.Context(), "1.1.1.1", "evm", "m"), "1.1.1.1 exhausted") + // Different IP has its own independent bucket. + require.True(t, r.Allow(t.Context(), "2.2.2.2", "evm", "m"), "2.2.2.2 should still pass") +} + +// --- IPFromHTTPRequest --- + +func TestIPFromHTTPRequest_DirectConnection(t *testing.T) { + r := New(cfg(100, 200)) // no trusted proxies + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "203.0.113.5:44321" + req.Header.Set("X-Forwarded-For", "1.2.3.4") + // RemoteAddr is not in trusted CIDRs, so XFF should be ignored. + require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) +} + +func TestIPFromHTTPRequest_TrustedProxy_XFF(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:12345" + req.Header.Set("X-Forwarded-For", "203.0.113.5, 10.0.0.1") + // Leftmost entry is the original client IP. + require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) +} + +func TestIPFromHTTPRequest_TrustedProxy_NoXFF(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:12345" + // No XFF header: fall back to RemoteAddr. + require.Equal(t, "10.0.0.1", r.IPFromHTTPRequest(req)) +} + +func TestIPFromHTTPRequest_UntrustedProxy_IgnoresXFF(t *testing.T) { + // Only loopback is trusted. + r := New(cfg(100, 200, "127.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "203.0.113.1:9999" + req.Header.Set("X-Forwarded-For", "1.2.3.4") + require.Equal(t, "203.0.113.1", r.IPFromHTTPRequest(req)) +} + +func TestIPFromHTTPRequest_SingleXFFEntry(t *testing.T) { + r := New(cfg(100, 200, "127.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "127.0.0.1:8080" + req.Header.Set("X-Forwarded-For", " 203.0.113.5 ") + require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) +} + +// --- IPFromGRPCContext --- + +func grpcCtx(peerAddr string, xff ...string) context.Context { + ctx := context.Background() + ctx = peer.NewContext(ctx, &peer.Peer{ + Addr: mockAddr(peerAddr), + }) + if len(xff) > 0 { + md := metadata.Pairs("x-forwarded-for", xff[0]) + ctx = metadata.NewIncomingContext(ctx, md) + } + return ctx +} + +type mockAddr string + +func (a mockAddr) Network() string { return "tcp" } +func (a mockAddr) String() string { return string(a) } + +func TestIPFromGRPCContext_DirectPeer(t *testing.T) { + r := New(cfg(100, 200)) // no trusted proxies + ctx := grpcCtx("203.0.113.5:9000", "1.2.3.4") + // Peer is not trusted, XFF must be ignored. + require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) +} + +func TestIPFromGRPCContext_TrustedPeer_XFF(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + ctx := grpcCtx("10.0.0.2:9000", "203.0.113.5, 10.0.0.2") + require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) +} + +func TestIPFromGRPCContext_TrustedPeer_NoMetadata(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + ctx := grpcCtx("10.0.0.2:9000") + require.Equal(t, "10.0.0.2", r.IPFromGRPCContext(ctx)) +} + +func TestIPFromGRPCContext_NoPeer(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + require.Equal(t, "", r.IPFromGRPCContext(t.Context())) +} + +// --- isTrustedProxy / parseCIDRs --- + +func TestIsTrustedProxy_DefaultCIDRs(t *testing.T) { + r := New(DefaultConfig) + cases := []struct { + ip string + trusted bool + }{ + {"127.0.0.1", true}, + {"::1", true}, + {"10.1.2.3", true}, + {"172.16.0.1", true}, + {"192.168.1.1", true}, + {"203.0.113.1", false}, + {"8.8.8.8", false}, + } + for _, tc := range cases { + require.Equal(t, tc.trusted, r.isTrustedProxy(tc.ip), "ip=%s", tc.ip) + } +} + +func TestParseCIDRs_SkipsInvalid(t *testing.T) { + nets := parseCIDRs([]string{"10.0.0.0/8", "not-a-cidr", "192.168.0.0/16"}) + require.Len(t, nets, 2) +} + +func TestParseCIDRs_Empty(t *testing.T) { + require.Empty(t, parseCIDRs(nil)) +} + +// --- helpers --- + +func TestStripPort(t *testing.T) { + require.Equal(t, "1.2.3.4", stripPort("1.2.3.4:8080")) + require.Equal(t, "::1", stripPort("[::1]:9090")) + require.Equal(t, "1.2.3.4", stripPort("1.2.3.4")) +} + +func TestFirstEntry(t *testing.T) { + require.Equal(t, "a", firstEntry("a, b, c")) + require.Equal(t, "solo", firstEntry("solo")) + require.Equal(t, "trimmed", firstEntry(" trimmed ")) +} + +// --- New validates TrustedProxyCIDRs --- + +func TestNew_InvalidCIDRSkipped(t *testing.T) { + r := New(Config{ + Enabled: true, + RPS: 100, + Burst: 10, + TrustedProxyCIDRs: []string{"bad", "10.0.0.0/8"}, + }) + require.Len(t, r.networks, 1) + require.True(t, r.networks[0].Contains(net.ParseIP("10.1.1.1"))) +} From cef47db0279db1fb4a0e644332faf4f15c723f7c Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Tue, 26 May 2026 16:32:59 -0700 Subject: [PATCH 02/15] fixed config_test --- evmrpc/config/config_test.go | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/evmrpc/config/config_test.go b/evmrpc/config/config_test.go index baac9655f2..d404ac5d18 100644 --- a/evmrpc/config/config_test.go +++ b/evmrpc/config/config_test.go @@ -39,6 +39,10 @@ type opts struct { rpcStatsInterval interface{} workerPoolSize interface{} workerQueueSize interface{} + rateLimitingEnabled interface{} + ipRateLimitRPS interface{} + ipRateLimitBurst interface{} + trustedProxyCIDRs interface{} } func (o *opts) Get(k string) interface{} { @@ -144,6 +148,18 @@ func (o *opts) Get(k string) interface{} { k == "evm.trace_bake_snapshot_window" { return nil } + if k == "evm.rate_limiting_enabled" { + return o.rateLimitingEnabled + } + if k == "evm.ip_rate_limit_rps" { + return o.ipRateLimitRPS + } + if k == "evm.ip_rate_limit_burst" { + return o.ipRateLimitBurst + } + if k == "evm.trusted_proxy_cidrs" { + return o.trustedProxyCIDRs + } panic("unknown key") } @@ -180,6 +196,10 @@ func getDefaultOpts() opts { 10 * time.Second, 32, 1000, + true, + 200.0, + 400, + []string{"127.0.0.0/8"}, } } @@ -294,6 +314,27 @@ func TestReadConfig(t *testing.T) { badOpts.workerQueueSize = "bad" _, err = config.ReadConfig(&badOpts) require.NotNil(t, err) + + // Test bad types for rate limit config + badOpts = goodOpts + badOpts.rateLimitingEnabled = "bad" + _, err = config.ReadConfig(&badOpts) + require.NotNil(t, err) + + badOpts = goodOpts + badOpts.ipRateLimitRPS = "bad" + _, err = config.ReadConfig(&badOpts) + require.NotNil(t, err) + + badOpts = goodOpts + badOpts.ipRateLimitBurst = "bad" + _, err = config.ReadConfig(&badOpts) + require.NotNil(t, err) + + badOpts = goodOpts + badOpts.trustedProxyCIDRs = map[string]interface{}{} + _, err = config.ReadConfig(&badOpts) + require.NotNil(t, err) } // Test worker pool configuration values From b70a1603f13f5ab55d157372ca4d92756c622d7f Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Tue, 26 May 2026 16:40:37 -0700 Subject: [PATCH 03/15] Removed method name attribute to bound cardinality of metric --- ratelimiter/registry.go | 7 +++---- ratelimiter/registry_test.go | 18 +++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index 84b80fe44a..05fe6a2f28 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -96,9 +96,9 @@ func New(cfg Config) *Registry { } } -// Allow reports whether the request from ip should be allowed for the given plane and method. -// Rejections increment rpc_rate_limit_rejected_total{plane, method}. -func (r *Registry) Allow(ctx context.Context, ip, plane, method string) bool { +// Allow reports whether the request from ip should be allowed for the given plane. +// Rejections increment rpc_rate_limit_rejected_total{plane}. +func (r *Registry) Allow(ctx context.Context, ip, plane string) bool { if !r.cfg.Enabled || r.cfg.RPS == 0 { return true } @@ -110,7 +110,6 @@ func (r *Registry) Allow(ctx context.Context, ip, plane, method string) bool { 1, metric.WithAttributes( attribute.String("plane", plane), - attribute.String("method", method), ), ) return false diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index cc768a72b6..359bbccabe 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -28,14 +28,14 @@ func cfg(rps float64, burst int, cidrs ...string) Config { func TestAllow_DisabledAlwaysPasses(t *testing.T) { r := New(disabledCfg) for range 1000 { - require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm", "eth_call")) + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm")) } } func TestAllow_ZeroRPSAlwaysPasses(t *testing.T) { r := New(zeroCfg) for range 1000 { - require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm", "eth_call")) + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm")) } } @@ -43,18 +43,18 @@ func TestAllow_BurstThenReject(t *testing.T) { // burst=3, RPS tiny so no token refill during test r := New(cfg(0.001, 3)) ip := "10.0.0.1" - require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "first request in burst") - require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "second request in burst") - require.True(t, r.Allow(t.Context(), ip, "evm", "m"), "third request in burst") - require.False(t, r.Allow(t.Context(), ip, "evm", "m"), "must be rejected after burst exhausted") + require.True(t, r.Allow(t.Context(), ip, "evm"), "first request in burst") + require.True(t, r.Allow(t.Context(), ip, "evm"), "second request in burst") + require.True(t, r.Allow(t.Context(), ip, "evm"), "third request in burst") + require.False(t, r.Allow(t.Context(), ip, "evm"), "must be rejected after burst exhausted") } func TestAllow_PerIPIsolation(t *testing.T) { r := New(cfg(0.001, 1)) - require.True(t, r.Allow(t.Context(), "1.1.1.1", "evm", "m")) - require.False(t, r.Allow(t.Context(), "1.1.1.1", "evm", "m"), "1.1.1.1 exhausted") + require.True(t, r.Allow(t.Context(), "1.1.1.1", "evm")) + require.False(t, r.Allow(t.Context(), "1.1.1.1", "evm"), "1.1.1.1 exhausted") // Different IP has its own independent bucket. - require.True(t, r.Allow(t.Context(), "2.2.2.2", "evm", "m"), "2.2.2.2 should still pass") + require.True(t, r.Allow(t.Context(), "2.2.2.2", "evm"), "2.2.2.2 should still pass") } // --- IPFromHTTPRequest --- From ecfe1cc0f593472ed80dbcd0a563b47cd53dab15 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Tue, 26 May 2026 17:15:01 -0700 Subject: [PATCH 04/15] Updated clinet ip extraction algorithm --- ratelimiter/registry.go | 38 ++++++++++++++++++++++++------------ ratelimiter/registry_test.go | 31 ++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index 05fe6a2f28..e5d1b45bf1 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -116,32 +116,52 @@ func (r *Registry) Allow(ctx context.Context, ip, plane string) bool { } // IPFromHTTPRequest extracts the client IP from an HTTP request. -// If RemoteAddr belongs to a trusted proxy CIDR, the leftmost X-Forwarded-For entry is used. +// If RemoteAddr belongs to a trusted proxy CIDR, the rightmost untrusted X-Forwarded-For +// entry is used. Walking right-to-left and skipping trusted CIDRs prevents a client from +// spoofing their IP by pre-setting X-Forwarded-For before the request reaches the proxy. func (r *Registry) IPFromHTTPRequest(req *http.Request) string { remoteIP := stripPort(req.RemoteAddr) if r.isTrustedProxy(remoteIP) { if xff := req.Header.Get("X-Forwarded-For"); xff != "" { - return firstEntry(xff) + if ip := r.rightmostUntrustedIP(xff); ip != "" { + return ip + } } } return remoteIP } // IPFromGRPCContext extracts the client IP from a gRPC request context. -// If the transport peer belongs to a trusted proxy CIDR, the leftmost x-forwarded-for -// metadata value is used. +// If the transport peer belongs to a trusted proxy CIDR, the rightmost untrusted +// x-forwarded-for metadata entry is used. func (r *Registry) IPFromGRPCContext(ctx context.Context) string { peerIP := grpcPeerIP(ctx) if peerIP != "" && r.isTrustedProxy(peerIP) { if md, ok := metadata.FromIncomingContext(ctx); ok { if vals := md.Get("x-forwarded-for"); len(vals) > 0 { - return firstEntry(vals[0]) + if ip := r.rightmostUntrustedIP(vals[0]); ip != "" { + return ip + } } } } return peerIP } +// rightmostUntrustedIP walks the comma-separated XFF list from right to left and returns +// the first IP that is not in TrustedProxyCIDRs. This is the real client IP: proxies +// append their view of the source address, so the rightmost untrusted entry cannot be +// forged by the client. +func (r *Registry) rightmostUntrustedIP(xff string) string { + parts := strings.Split(xff, ",") + for i := len(parts) - 1; i >= 0; i-- { + if ip := strings.TrimSpace(parts[i]); !r.isTrustedProxy(ip) { + return ip + } + } + return "" +} + // getOrCreate returns the existing limiter for ip or creates a fresh one. // A brief TOCTOU race can occur under high concurrency: at most one extra burst token // leaks per race window, which has no meaningful security impact. @@ -185,14 +205,6 @@ func stripPort(addr string) string { return host } -func firstEntry(xff string) string { - before, _, found := strings.Cut(xff, ",") - if found { - return strings.TrimSpace(before) - } - return strings.TrimSpace(xff) -} - func grpcPeerIP(ctx context.Context) string { p, ok := peer.FromContext(ctx) if !ok || p.Addr == nil { diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index 359bbccabe..8a9d91c4dc 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -72,8 +72,18 @@ func TestIPFromHTTPRequest_TrustedProxy_XFF(t *testing.T) { r := New(cfg(100, 200, "10.0.0.0/8")) req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = "10.0.0.1:12345" + // Proxy appended 203.0.113.5 (real client) on the right; rightmost untrusted wins. req.Header.Set("X-Forwarded-For", "203.0.113.5, 10.0.0.1") - // Leftmost entry is the original client IP. + require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) +} + +func TestIPFromHTTPRequest_TrustedProxy_SpoofedXFF(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:12345" + // Client pre-set a spoofed IP; proxy appended the real client IP on the right. + // Must use rightmost untrusted (203.0.113.5), not the spoofed leftmost (1.2.3.4). + req.Header.Set("X-Forwarded-For", "1.2.3.4, 203.0.113.5") require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) } @@ -130,10 +140,18 @@ func TestIPFromGRPCContext_DirectPeer(t *testing.T) { func TestIPFromGRPCContext_TrustedPeer_XFF(t *testing.T) { r := New(cfg(100, 200, "10.0.0.0/8")) + // Proxy appended 203.0.113.5 (real client) on the right; rightmost untrusted wins. ctx := grpcCtx("10.0.0.2:9000", "203.0.113.5, 10.0.0.2") require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) } +func TestIPFromGRPCContext_TrustedPeer_SpoofedXFF(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + // Client pre-set a spoofed IP; proxy appended the real client IP on the right. + ctx := grpcCtx("10.0.0.2:9000", "1.2.3.4, 203.0.113.5") + require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) +} + func TestIPFromGRPCContext_TrustedPeer_NoMetadata(t *testing.T) { r := New(cfg(100, 200, "10.0.0.0/8")) ctx := grpcCtx("10.0.0.2:9000") @@ -183,10 +201,13 @@ func TestStripPort(t *testing.T) { require.Equal(t, "1.2.3.4", stripPort("1.2.3.4")) } -func TestFirstEntry(t *testing.T) { - require.Equal(t, "a", firstEntry("a, b, c")) - require.Equal(t, "solo", firstEntry("solo")) - require.Equal(t, "trimmed", firstEntry(" trimmed ")) +func TestRightmostUntrustedIP(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8", "127.0.0.0/8")) + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("203.0.113.5")) + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5")) + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5, 10.0.0.1")) + require.Equal(t, "trimmed", r.rightmostUntrustedIP(" trimmed ")) // whitespace stripped + require.Equal(t, "", r.rightmostUntrustedIP("10.0.0.1, 127.0.0.1")) // all trusted → empty } // --- New validates TrustedProxyCIDRs --- From acf81f7ddd983d21f37d77be69b7d3d95794f0b0 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Tue, 26 May 2026 17:29:05 -0700 Subject: [PATCH 05/15] Fixed config format --- evmrpc/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evmrpc/config/config.go b/evmrpc/config/config.go index 0a3335106b..8cdd6dfb73 100644 --- a/evmrpc/config/config.go +++ b/evmrpc/config/config.go @@ -686,5 +686,5 @@ ip_rate_limit_burst = {{ .EVM.IPRateLimitBurst }} # trusted_proxy_cidrs lists CIDR ranges whose X-Forwarded-For headers are trusted # for real-client-IP extraction. Defaults to RFC-1918 + loopback. # Set to an empty list when no reverse proxy sits in front of the node. -trusted_proxy_cidrs = [{{- range .EVM.TrustedProxyCIDRs }}"{{ . }}", {{- end }}] +trusted_proxy_cidrs = [{{- range $i, $c := .EVM.TrustedProxyCIDRs }}{{- if $i }}, {{ end }}"{{ $c }}"{{- end }}] ` From 305f418a680cfd56a8b47f01cca7309233a2e893 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 13:57:57 -0700 Subject: [PATCH 06/15] moved metrics to their own file --- ratelimiter/metrics.go | 27 +++++++++++++++++++++++++++ ratelimiter/registry.go | 22 ---------------------- 2 files changed, 27 insertions(+), 22 deletions(-) create mode 100644 ratelimiter/metrics.go diff --git a/ratelimiter/metrics.go b/ratelimiter/metrics.go new file mode 100644 index 0000000000..6f2371afc6 --- /dev/null +++ b/ratelimiter/metrics.go @@ -0,0 +1,27 @@ +package ratelimiter + +import ( + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/metric" +) + +var ( + registryMeter = otel.Meter("ratelimiter") + + registryMetrics = struct { + rejectedCounter metric.Int64Counter + }{ + rejectedCounter: must(registryMeter.Int64Counter( + "rpc_rate_limit_rejected_total", + metric.WithDescription("Total RPC requests rejected by the per-IP rate limiter"), + metric.WithUnit("{request}"), + )), + } +) + +func must[V any](v V, err error) V { + if err != nil { + panic(err) + } + return v +} diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index e5d1b45bf1..a662e8f860 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -8,7 +8,6 @@ import ( "time" "github.com/hashicorp/golang-lru/v2/expirable" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "golang.org/x/time/rate" @@ -58,27 +57,6 @@ var DefaultConfig = Config{ TrustedProxyCIDRs: DefaultTrustedProxyCIDRs, } -var ( - registryMeter = otel.Meter("ratelimiter") - - registryMetrics = struct { - rejectedCounter metric.Int64Counter - }{ - rejectedCounter: must(registryMeter.Int64Counter( - "rpc_rate_limit_rejected_total", - metric.WithDescription("Total RPC requests rejected by the per-IP rate limiter"), - metric.WithUnit("{request}"), - )), - } -) - -func must[V any](v V, err error) V { - if err != nil { - panic(err) - } - return v -} - // Registry is a per-IP token-bucket rate limiter backed by an expirable LRU. // It is safe for concurrent use. type Registry struct { From 9ca0c22151c1a695cb758c28a8a44eeee5ae1aee Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 14:35:11 -0700 Subject: [PATCH 07/15] validted the ip string before rate limiting it --- ratelimiter/registry.go | 22 +++++++++++++--------- ratelimiter/registry_test.go | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index a662e8f860..bb04c10350 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -60,17 +60,17 @@ var DefaultConfig = Config{ // Registry is a per-IP token-bucket rate limiter backed by an expirable LRU. // It is safe for concurrent use. type Registry struct { - cfg Config - networks []*net.IPNet - lru *expirable.LRU[string, *rate.Limiter] + cfg Config + trustedProxies []*net.IPNet + lru *expirable.LRU[string, *rate.Limiter] } // New creates a Registry from cfg. Invalid CIDRs in TrustedProxyCIDRs are silently skipped. func New(cfg Config) *Registry { return &Registry{ - cfg: cfg, - networks: parseCIDRs(cfg.TrustedProxyCIDRs), - lru: expirable.NewLRU[string, *rate.Limiter](lruSize, nil, lruTTL), + cfg: cfg, + trustedProxies: parseCIDRs(cfg.TrustedProxyCIDRs), + lru: expirable.NewLRU[string, *rate.Limiter](lruSize, nil, lruTTL), } } @@ -133,8 +133,12 @@ func (r *Registry) IPFromGRPCContext(ctx context.Context) string { func (r *Registry) rightmostUntrustedIP(xff string) string { parts := strings.Split(xff, ",") for i := len(parts) - 1; i >= 0; i-- { - if ip := strings.TrimSpace(parts[i]); !r.isTrustedProxy(ip) { - return ip + candidate := strings.TrimSpace(parts[i]) + if net.ParseIP(candidate) == nil { + continue + } + if !r.isTrustedProxy(candidate) { + return candidate } } return "" @@ -157,7 +161,7 @@ func (r *Registry) isTrustedProxy(ip string) bool { if parsed == nil { return false } - for _, n := range r.networks { + for _, n := range r.trustedProxies { if n.Contains(parsed) { return true } diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index 8a9d91c4dc..aa6f4b5697 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -219,6 +219,6 @@ func TestNew_InvalidCIDRSkipped(t *testing.T) { Burst: 10, TrustedProxyCIDRs: []string{"bad", "10.0.0.0/8"}, }) - require.Len(t, r.networks, 1) - require.True(t, r.networks[0].Contains(net.ParseIP("10.1.1.1"))) + require.Len(t, r.trustedProxies, 1) + require.True(t, r.trustedProxies[0].Contains(net.ParseIP("10.1.1.1"))) } From c70ba03ade0cdf2579108bc6bafeb1d9a90be553 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 14:39:02 -0700 Subject: [PATCH 08/15] updated tests --- ratelimiter/registry_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index aa6f4b5697..2d644a05bb 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -206,8 +206,11 @@ func TestRightmostUntrustedIP(t *testing.T) { require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("203.0.113.5")) require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5")) require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5, 10.0.0.1")) - require.Equal(t, "trimmed", r.rightmostUntrustedIP(" trimmed ")) // whitespace stripped - require.Equal(t, "", r.rightmostUntrustedIP("10.0.0.1, 127.0.0.1")) // all trusted → empty + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP(" 203.0.113.5 ")) // whitespace stripped + require.Equal(t, "", r.rightmostUntrustedIP("10.0.0.1, 127.0.0.1")) // all trusted → empty + require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip")) // non-IP skipped + require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip, 10.0.0.1")) // non-IP + trusted → empty, falls back to RemoteAddr + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("not-an-ip, 203.0.113.5")) // non-IP skipped, valid untrusted returned } // --- New validates TrustedProxyCIDRs --- From 9f05350e0264b03af4c8c908dc5a2133332eb2dd Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 16:00:03 -0700 Subject: [PATCH 09/15] Fixing go fmt --- ratelimiter/registry_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index 2d644a05bb..0db95e135d 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -206,10 +206,10 @@ func TestRightmostUntrustedIP(t *testing.T) { require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("203.0.113.5")) require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5")) require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("1.2.3.4, 203.0.113.5, 10.0.0.1")) - require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP(" 203.0.113.5 ")) // whitespace stripped - require.Equal(t, "", r.rightmostUntrustedIP("10.0.0.1, 127.0.0.1")) // all trusted → empty - require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip")) // non-IP skipped - require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip, 10.0.0.1")) // non-IP + trusted → empty, falls back to RemoteAddr + require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP(" 203.0.113.5 ")) // whitespace stripped + require.Equal(t, "", r.rightmostUntrustedIP("10.0.0.1, 127.0.0.1")) // all trusted → empty + require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip")) // non-IP skipped + require.Equal(t, "", r.rightmostUntrustedIP("not-an-ip, 10.0.0.1")) // non-IP + trusted → empty, falls back to RemoteAddr require.Equal(t, "203.0.113.5", r.rightmostUntrustedIP("not-an-ip, 203.0.113.5")) // non-IP skipped, valid untrusted returned } From b942624ecf34b08e4db35544579540d2996bca33 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 16:19:12 -0700 Subject: [PATCH 10/15] Added fix for multiple XFF header values --- ratelimiter/registry.go | 4 ++-- ratelimiter/registry_test.go | 21 ++++++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index bb04c10350..317040c1c7 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -100,7 +100,7 @@ func (r *Registry) Allow(ctx context.Context, ip, plane string) bool { func (r *Registry) IPFromHTTPRequest(req *http.Request) string { remoteIP := stripPort(req.RemoteAddr) if r.isTrustedProxy(remoteIP) { - if xff := req.Header.Get("X-Forwarded-For"); xff != "" { + if xff := strings.Join(req.Header.Values("X-Forwarded-For"), ", "); xff != "" { if ip := r.rightmostUntrustedIP(xff); ip != "" { return ip } @@ -117,7 +117,7 @@ func (r *Registry) IPFromGRPCContext(ctx context.Context) string { if peerIP != "" && r.isTrustedProxy(peerIP) { if md, ok := metadata.FromIncomingContext(ctx); ok { if vals := md.Get("x-forwarded-for"); len(vals) > 0 { - if ip := r.rightmostUntrustedIP(vals[0]); ip != "" { + if ip := r.rightmostUntrustedIP(strings.Join(vals, ", ")); ip != "" { return ip } } diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index 0db95e135d..cfbbca70fe 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -112,6 +112,18 @@ func TestIPFromHTTPRequest_SingleXFFEntry(t *testing.T) { require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) } +func TestIPFromHTTPRequest_MultipleXFFHeaders_SpoofPrevented(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:12345" + // Client pre-sets a spoofed IP as the first XFF header line. + // Proxy adds the real client IP as a separate header line (not appended to the existing one). + req.Header.Add("X-Forwarded-For", "spoofed-ip-ignored") + req.Header.Add("X-Forwarded-For", "203.0.113.5") + // Must use rightmost untrusted across all header lines, not just the first. + require.Equal(t, "203.0.113.5", r.IPFromHTTPRequest(req)) +} + // --- IPFromGRPCContext --- func grpcCtx(peerAddr string, xff ...string) context.Context { @@ -120,7 +132,7 @@ func grpcCtx(peerAddr string, xff ...string) context.Context { Addr: mockAddr(peerAddr), }) if len(xff) > 0 { - md := metadata.Pairs("x-forwarded-for", xff[0]) + md := metadata.MD{"x-forwarded-for": xff} ctx = metadata.NewIncomingContext(ctx, md) } return ctx @@ -152,6 +164,13 @@ func TestIPFromGRPCContext_TrustedPeer_SpoofedXFF(t *testing.T) { require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) } +func TestIPFromGRPCContext_MultipleXFFValues_SpoofPrevented(t *testing.T) { + r := New(cfg(100, 200, "10.0.0.0/8")) + // Client pre-sets a spoofed IP as the first metadata value; proxy appends real IP as a second value. + ctx := grpcCtx("10.0.0.2:9000", "spoofed-ip-ignored", "203.0.113.5") + require.Equal(t, "203.0.113.5", r.IPFromGRPCContext(ctx)) +} + func TestIPFromGRPCContext_TrustedPeer_NoMetadata(t *testing.T) { r := New(cfg(100, 200, "10.0.0.0/8")) ctx := grpcCtx("10.0.0.2:9000") From e28bc73de29214635b609cf4be6a90dff27bee04 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 16:23:57 -0700 Subject: [PATCH 11/15] Handled negative RPS config --- ratelimiter/registry.go | 2 +- ratelimiter/registry_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index 317040c1c7..d6d95c3ba5 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -77,7 +77,7 @@ func New(cfg Config) *Registry { // Allow reports whether the request from ip should be allowed for the given plane. // Rejections increment rpc_rate_limit_rejected_total{plane}. func (r *Registry) Allow(ctx context.Context, ip, plane string) bool { - if !r.cfg.Enabled || r.cfg.RPS == 0 { + if !r.cfg.Enabled || r.cfg.RPS <= 0 { return true } if r.getOrCreate(ip).Allow() { diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index cfbbca70fe..5186d51a97 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -19,6 +19,9 @@ var disabledCfg = Config{Enabled: false, RPS: 100, Burst: 10} // zeroCfg has RPS=0 which also disables limiting. var zeroCfg = Config{Enabled: true, RPS: 0, Burst: 10} +// negCfg has RPS<0 which also disables limiting. +var negCfg = Config{Enabled: true, RPS: -1, Burst: 10} + func cfg(rps float64, burst int, cidrs ...string) Config { return Config{Enabled: true, RPS: rps, Burst: burst, TrustedProxyCIDRs: cidrs} } @@ -39,6 +42,13 @@ func TestAllow_ZeroRPSAlwaysPasses(t *testing.T) { } } +func TestAllow_NegativeRPSAlwaysPasses(t *testing.T) { + r := New(negCfg) + for range 1000 { + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm")) + } +} + func TestAllow_BurstThenReject(t *testing.T) { // burst=3, RPS tiny so no token refill during test r := New(cfg(0.001, 3)) From 733097d977168c84a1c3919e778fe85981e267ff Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 16:55:04 -0700 Subject: [PATCH 12/15] removed evmrpc changes --- evmrpc/config/config.go | 23 ----------------------- evmrpc/config/config_test.go | 9 --------- 2 files changed, 32 deletions(-) diff --git a/evmrpc/config/config.go b/evmrpc/config/config.go index 8cdd6dfb73..6c8f768f5a 100644 --- a/evmrpc/config/config.go +++ b/evmrpc/config/config.go @@ -169,10 +169,6 @@ type Config struct { // IPRateLimitBurst is the maximum per-IP burst size. IPRateLimitBurst int `mapstructure:"ip_rate_limit_burst"` - // TrustedProxyCIDRs lists CIDR ranges whose X-Forwarded-For headers are trusted - // for real-client-IP extraction. Defaults to RFC-1918 + loopback. - // Set to an empty list when no reverse proxy sits in front of the node. - TrustedProxyCIDRs []string `mapstructure:"trusted_proxy_cidrs"` } var DefaultConfig = Config{ @@ -220,14 +216,6 @@ var DefaultConfig = Config{ RateLimitingEnabled: true, IPRateLimitRPS: 200, IPRateLimitBurst: 400, - TrustedProxyCIDRs: []string{ - "127.0.0.0/8", - "::1/128", - "10.0.0.0/8", - "172.16.0.0/12", - "192.168.0.0/16", - "fc00::/7", - }, } const ( @@ -271,7 +259,6 @@ const ( flagRateLimitingEnabled = "evm.rate_limiting_enabled" flagIPRateLimitRPS = "evm.ip_rate_limit_rps" flagIPRateLimitBurst = "evm.ip_rate_limit_burst" - flagTrustedProxyCIDRs = "evm.trusted_proxy_cidrs" ) func ReadConfig(opts servertypes.AppOptions) (Config, error) { @@ -477,12 +464,6 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) { return cfg, err } } - if v := opts.Get(flagTrustedProxyCIDRs); v != nil { - if cfg.TrustedProxyCIDRs, err = cast.ToStringSliceE(v); err != nil { - return cfg, err - } - } - return cfg, nil } @@ -683,8 +664,4 @@ ip_rate_limit_rps = {{ .EVM.IPRateLimitRPS }} # ip_rate_limit_burst is the maximum per-IP burst above the sustained rate. ip_rate_limit_burst = {{ .EVM.IPRateLimitBurst }} -# trusted_proxy_cidrs lists CIDR ranges whose X-Forwarded-For headers are trusted -# for real-client-IP extraction. Defaults to RFC-1918 + loopback. -# Set to an empty list when no reverse proxy sits in front of the node. -trusted_proxy_cidrs = [{{- range $i, $c := .EVM.TrustedProxyCIDRs }}{{- if $i }}, {{ end }}"{{ $c }}"{{- end }}] ` diff --git a/evmrpc/config/config_test.go b/evmrpc/config/config_test.go index d404ac5d18..4f4c31c36f 100644 --- a/evmrpc/config/config_test.go +++ b/evmrpc/config/config_test.go @@ -42,7 +42,6 @@ type opts struct { rateLimitingEnabled interface{} ipRateLimitRPS interface{} ipRateLimitBurst interface{} - trustedProxyCIDRs interface{} } func (o *opts) Get(k string) interface{} { @@ -157,9 +156,6 @@ func (o *opts) Get(k string) interface{} { if k == "evm.ip_rate_limit_burst" { return o.ipRateLimitBurst } - if k == "evm.trusted_proxy_cidrs" { - return o.trustedProxyCIDRs - } panic("unknown key") } @@ -199,7 +195,6 @@ func getDefaultOpts() opts { true, 200.0, 400, - []string{"127.0.0.0/8"}, } } @@ -331,10 +326,6 @@ func TestReadConfig(t *testing.T) { _, err = config.ReadConfig(&badOpts) require.NotNil(t, err) - badOpts = goodOpts - badOpts.trustedProxyCIDRs = map[string]interface{}{} - _, err = config.ReadConfig(&badOpts) - require.NotNil(t, err) } // Test worker pool configuration values From cff02e5e04ec252fb9f93ac4d8f19f08a6d360e8 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Wed, 27 May 2026 18:11:40 -0700 Subject: [PATCH 13/15] Fixed fmt --- evmrpc/config/config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/evmrpc/config/config.go b/evmrpc/config/config.go index 6c8f768f5a..b6369d47fa 100644 --- a/evmrpc/config/config.go +++ b/evmrpc/config/config.go @@ -168,7 +168,6 @@ type Config struct { // IPRateLimitBurst is the maximum per-IP burst size. IPRateLimitBurst int `mapstructure:"ip_rate_limit_burst"` - } var DefaultConfig = Config{ From fe5d3c9c93575ce00438ee856caec9794b9b652a Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Thu, 28 May 2026 16:36:12 -0700 Subject: [PATCH 14/15] added guard for burst = 0 to allow all traffic --- ratelimiter/registry.go | 3 ++- ratelimiter/registry_test.go | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index d6d95c3ba5..a18457d08c 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -44,6 +44,7 @@ type Config struct { // Zero disables per-IP rate limiting (all requests pass). RPS float64 // Burst is the maximum number of requests allowed in a single burst. + // Zero disables per-IP rate limiting (all requests pass). Burst int // TrustedProxyCIDRs lists CIDRs whose X-Forwarded-For headers are trusted. // Empty means trust no proxy; use RemoteAddr / peer address directly. @@ -77,7 +78,7 @@ func New(cfg Config) *Registry { // Allow reports whether the request from ip should be allowed for the given plane. // Rejections increment rpc_rate_limit_rejected_total{plane}. func (r *Registry) Allow(ctx context.Context, ip, plane string) bool { - if !r.cfg.Enabled || r.cfg.RPS <= 0 { + if !r.cfg.Enabled || r.cfg.RPS <= 0 || r.cfg.Burst <= 0 { return true } if r.getOrCreate(ip).Allow() { diff --git a/ratelimiter/registry_test.go b/ratelimiter/registry_test.go index 5186d51a97..e176f1c590 100644 --- a/ratelimiter/registry_test.go +++ b/ratelimiter/registry_test.go @@ -22,6 +22,9 @@ var zeroCfg = Config{Enabled: true, RPS: 0, Burst: 10} // negCfg has RPS<0 which also disables limiting. var negCfg = Config{Enabled: true, RPS: -1, Burst: 10} +// zeroBurstCfg has Burst=0 which also disables limiting. +var zeroBurstCfg = Config{Enabled: true, RPS: 100, Burst: 0} + func cfg(rps float64, burst int, cidrs ...string) Config { return Config{Enabled: true, RPS: rps, Burst: burst, TrustedProxyCIDRs: cidrs} } @@ -49,6 +52,13 @@ func TestAllow_NegativeRPSAlwaysPasses(t *testing.T) { } } +func TestAllow_ZeroBurstAlwaysPasses(t *testing.T) { + r := New(zeroBurstCfg) + for range 1000 { + require.True(t, r.Allow(t.Context(), "1.2.3.4", "evm")) + } +} + func TestAllow_BurstThenReject(t *testing.T) { // burst=3, RPS tiny so no token refill during test r := New(cfg(0.001, 3)) From 4756e1783007df4fc117ee137a3b52cf04d53046 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Thu, 28 May 2026 16:56:47 -0700 Subject: [PATCH 15/15] Updated TTL for active ips --- ratelimiter/registry.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ratelimiter/registry.go b/ratelimiter/registry.go index a18457d08c..17c3c02157 100644 --- a/ratelimiter/registry.go +++ b/ratelimiter/registry.go @@ -21,7 +21,7 @@ const ( // lruSize bounds memory to ~8 MB at 50k entries (~160 bytes each). lruSize = 50_000 - // lruTTL evicts inactive IP entries after 1 hour. + // lruTTL evicts IP entries that have been idle for 1 hour. lruTTL = time.Hour ) @@ -146,10 +146,10 @@ func (r *Registry) rightmostUntrustedIP(xff string) string { } // getOrCreate returns the existing limiter for ip or creates a fresh one. -// A brief TOCTOU race can occur under high concurrency: at most one extra burst token -// leaks per race window, which has no meaningful security impact. +// Add is called on every hit to refresh the TTL, ensuring only truly idle IPs expire. func (r *Registry) getOrCreate(ip string) *rate.Limiter { if l, ok := r.lru.Get(ip); ok { + r.lru.Add(ip, l) return l } l := rate.NewLimiter(rate.Limit(r.cfg.RPS), r.cfg.Burst)