Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 148 additions & 4 deletions sei-cosmos/server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"runtime"
"strings"
"time"

storetypes "github.com/sei-protocol/sei-chain/sei-cosmos/store/types"
"github.com/sei-protocol/sei-chain/sei-cosmos/telemetry"
Expand All @@ -30,6 +31,49 @@ const (
// simultaneous open connections for the gRPC-web server.
DefaultGRPCWebMaxOpenConnections = 1000

// DefaultGRPCMaxOpenConnections defines the default maximum number of
// simultaneous open connections for the gRPC server. 0 means unlimited.
DefaultGRPCMaxOpenConnections = 1000

// DefaultGRPCMaxRecvMsgSize defines the default maximum message size in bytes
// that the gRPC server can receive (4 MB), mirroring gRPC's own default.
DefaultGRPCMaxRecvMsgSize = 4 * 1024 * 1024

// DefaultGRPCMaxConnectionIdle is the default duration after which an idle
// connection (one with no in-flight RPCs) is closed via GoAway. It is bounded
// by default to reclaim abandoned connection slots — which matter now that the
// listener is capped by MaxOpenConnections — while staying long enough not to
// churn clients that query on a shorter cadence. The real DoS bound is the
// connection cap; this only reaps dormant connections. 0 means infinity.
DefaultGRPCMaxConnectionIdle = 5 * time.Minute

// The remaining keepalive defaults below mirror gRPC's own defaults, so they
// are opt-in for operators and do not change behavior unless configured.

// DefaultGRPCMaxConnectionAge is the default maximum duration a connection may
// exist before it is closed. 0 means infinity.
DefaultGRPCMaxConnectionAge = time.Duration(0)

// DefaultGRPCMaxConnectionAgeGrace is the default additive period after
// MaxConnectionAge during which the connection is forcibly closed. 0 means infinity.
DefaultGRPCMaxConnectionAgeGrace = time.Duration(0)

// DefaultGRPCKeepaliveTime is the default interval after which, if the server
// sees no activity, it pings the client to check liveness.
DefaultGRPCKeepaliveTime = 2 * time.Hour

// DefaultGRPCKeepaliveTimeout is the default duration the server waits for a
// keepalive ping ack before closing the connection.
DefaultGRPCKeepaliveTimeout = 20 * time.Second

// DefaultGRPCKeepaliveMinTime is the default minimum interval a client must
// wait between keepalive pings; pings more frequent than this are penalized.
DefaultGRPCKeepaliveMinTime = 5 * time.Minute

// DefaultGRPCKeepalivePermitWithoutStream defines whether the server allows
// keepalive pings even when there are no active streams.
DefaultGRPCKeepalivePermitWithoutStream = false

// DefaultOccEanbled defines whether to use OCC for tx processing
DefaultOccEnabled = true
)
Expand Down Expand Up @@ -172,6 +216,43 @@ type GRPCConfig struct {

// Address defines the API server to listen on
Address string `mapstructure:"address"`

// MaxRecvMsgSize defines the maximum message size in bytes the server can
// receive. It bounds per-request memory allocation before the rate limiter
// fires. Defaults to 4 MB.
MaxRecvMsgSize int `mapstructure:"max-recv-msg-size"`

// MaxOpenConnections defines the maximum number of simultaneous open
// connections. 0 means unlimited.
MaxOpenConnections uint `mapstructure:"max-open-connections"`

// MaxConnectionIdle is the duration after which an idle connection is closed.
// 0 means infinity.
MaxConnectionIdle time.Duration `mapstructure:"max-connection-idle"`

// MaxConnectionAge is the maximum duration a connection may exist before it
// is closed (a jitter is added by gRPC). 0 means infinity.
MaxConnectionAge time.Duration `mapstructure:"max-connection-age"`

// MaxConnectionAgeGrace is an additive period after MaxConnectionAge during
// which the connection is forcibly closed. 0 means infinity.
MaxConnectionAgeGrace time.Duration `mapstructure:"max-connection-age-grace"`

// KeepaliveTime is the interval after which, if the server sees no activity,
// it pings the client to check liveness.
KeepaliveTime time.Duration `mapstructure:"keepalive-time"`

// KeepaliveTimeout is the duration the server waits for a keepalive ping ack
// before closing the connection.
KeepaliveTimeout time.Duration `mapstructure:"keepalive-timeout"`

// KeepaliveMinTime is the minimum interval a client must wait between
// keepalive pings; clients pinging more frequently are penalized.
KeepaliveMinTime time.Duration `mapstructure:"keepalive-min-time"`

// KeepalivePermitWithoutStream defines whether the server allows keepalive
// pings even when there are no active streams.
KeepalivePermitWithoutStream bool `mapstructure:"keepalive-permit-without-stream"`
}

// GRPCWebConfig defines configuration for the gRPC-web server.
Expand Down Expand Up @@ -287,8 +368,17 @@ func DefaultConfig() *Config {
RPCMaxBodyBytes: 1000000,
},
GRPC: GRPCConfig{
Enable: true,
Address: DefaultGRPCAddress,
Enable: true,
Address: DefaultGRPCAddress,
MaxRecvMsgSize: DefaultGRPCMaxRecvMsgSize,
MaxOpenConnections: DefaultGRPCMaxOpenConnections,
MaxConnectionIdle: DefaultGRPCMaxConnectionIdle,
MaxConnectionAge: DefaultGRPCMaxConnectionAge,
MaxConnectionAgeGrace: DefaultGRPCMaxConnectionAgeGrace,
KeepaliveTime: DefaultGRPCKeepaliveTime,
KeepaliveTimeout: DefaultGRPCKeepaliveTimeout,
KeepaliveMinTime: DefaultGRPCKeepaliveMinTime,
KeepalivePermitWithoutStream: DefaultGRPCKeepalivePermitWithoutStream,
},
Rosetta: RosettaConfig{
Enable: false,
Expand Down Expand Up @@ -317,6 +407,16 @@ func DefaultConfig() *Config {
}
}

// clampNonNegativeDuration returns fallback when d is negative; zero and
// positive values (zero is gRPC's "infinity" for several keepalive fields) pass
// through unchanged.
func clampNonNegativeDuration(d, fallback time.Duration) time.Duration {
if d < 0 {
return fallback
}
return d
}

// GetConfig returns a fully parsed Config object.
func GetConfig(v *viper.Viper) (Config, error) {
globalLabelsRaw, ok := v.Get("telemetry.global-labels").([]interface{})
Expand Down Expand Up @@ -373,6 +473,41 @@ func GetConfig(v *viper.Viper) (Config, error) {
grpcWebMaxOpenConnections = v.GetUint("grpc-web.max-open-connections")
}

// Apply in-code defaults when keys are absent so that nodes upgrading with an
// older app.toml (which lacks these keys) remain bounded rather than running
// with unlimited connections / message sizes.
grpcMaxRecvMsgSize := DefaultGRPCMaxRecvMsgSize
if v.IsSet("grpc.max-recv-msg-size") {
grpcMaxRecvMsgSize = v.GetInt("grpc.max-recv-msg-size")
}
grpcMaxOpenConnections := uint(DefaultGRPCMaxOpenConnections)
Comment on lines +480 to +483

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Negative max-open-connections in app.toml (e.g. -1) silently sets the cap to unlimited, defeating the DoS bound this PR installs. v.GetUint("grpc.max-open-connections") returns 0 for negative inputs (cast.ToUintE swallows errNegativeNotAllowed), and then the if cfg.MaxOpenConnections > 0 guard in StartGRPCServer skips the netutil.LimitListener wrap entirely. Apply the same defensive pattern clampNonNegativeDuration uses for durations: read as a signed int and fall back to DefaultGRPCMaxOpenConnections on negative values. (The same gap also exists for the new grpc.max-recv-msg-size field.)

Extended reasoning...

Bug

server/config/config.go:480-483 reads grpc.max-open-connections via v.GetUint. When an operator writes a negative value such as max-open-connections = -1 in app.toml (typo, stale config, or -1 means unlimited assumption from systems like Postgres / Go's http.Server), the value flows through the cast/viper chain to a silent 0:

  1. cast@v1.10.0/number.go:287 toUnsignedNumberE returns (0, errNegativeNotAllowed) when given a negative integer.
  2. viper.GetUint calls cast.ToUint which discards that error and returns 0.
  3. v.IsSet("grpc.max-open-connections") is true for an explicit -1, so the IsSet guard does not fall through to DefaultGRPCMaxOpenConnections. grpcMaxOpenConnections is assigned the silently-coerced uint(0).

Then at server/grpc/server.go:68-74:

if cfg.MaxOpenConnections > 0 {
    listener = netutil.LimitListener(listener, int(maxConn))
}

0 skips the wrap entirely — the explicitly documented unlimited branch (see the field doc at config.go:225-226 and the constant doc at config.go:33-35). Net effect: a single-character typo in app.toml silently converts the connection cap to UNLIMITED — the exact DoS surface this PR is meant to bound.

Why this contradicts the PR's own pattern

The PR explicitly defends against negative-misconfiguration for durations via clampNonNegativeDuration (config.go:410-416), with the comment:

A negative keepalive/connection-age value is a misconfiguration that gRPC would otherwise accept verbatim, so fall back to the safe default instead.

That reasoning applies more strongly to max-open-connections, since the PR description identifies the connection cap as "the actual DoS bound" — not the keepalive durations. The asymmetry is the bug.

Step-by-step proof

v := viper.New()
v.SetConfigType("toml")
v.ReadConfig(strings.NewReader("[grpc]\nmax-open-connections = -1\n"))

v.IsSet("grpc.max-open-connections")    // true  -> IsSet guard does NOT fall through
v.GetInt("grpc.max-open-connections")   // -1    (the actual stored value)
v.GetUint("grpc.max-open-connections")  // 0     (cast swallows errNegativeNotAllowed)

// In GetConfig:
grpcMaxOpenConnections := uint(DefaultGRPCMaxOpenConnections)  // 1000
if v.IsSet("grpc.max-open-connections") {                      // true
    grpcMaxOpenConnections = v.GetUint("grpc.max-open-connections")  // 0
}
// cfg.MaxOpenConnections == 0

// In StartGRPCServer:
if cfg.MaxOpenConnections > 0 { /* unreached */ }
// listener is the raw net.Listener — no cap.

Operator intent: "set a finite limit" (or "unlimited" via the -1 convention from other systems). Actual behavior: unlimited, silently, with no warning logged.

Fix

Mirror the duration-clamp pattern with a signed read. Roughly:

grpcMaxOpenConnections := uint(DefaultGRPCMaxOpenConnections)
if v.IsSet("grpc.max-open-connections") {
    if raw := v.GetInt64("grpc.max-open-connections"); raw >= 0 {
        grpcMaxOpenConnections = uint(raw)
    }
    // negative -> keep DefaultGRPCMaxOpenConnections (or log + fall back)
}

The same gap also exists for grpc.max-recv-msg-size (config.go:478): a negative override there would be clamped at the server.go:24-27 layer (if maxRecvMsgSize <= 0 { maxRecvMsgSize = DefaultGRPCMaxRecvMsgSize }) rather than at config-parse time, so the symptom is muted but the asymmetry with the duration clamp remains — worth fixing in the same pass.

if v.IsSet("grpc.max-open-connections") {
grpcMaxOpenConnections = v.GetUint("grpc.max-open-connections")
}
// Clamp negative durations back to their in-code defaults. A negative
// keepalive/connection-age value is a misconfiguration that gRPC would
// otherwise accept verbatim, so fall back to the safe default instead.
grpcMaxConnectionIdle := DefaultGRPCMaxConnectionIdle
if v.IsSet("grpc.max-connection-idle") {
grpcMaxConnectionIdle = clampNonNegativeDuration(v.GetDuration("grpc.max-connection-idle"), DefaultGRPCMaxConnectionIdle)
}
grpcKeepaliveTime := DefaultGRPCKeepaliveTime
if v.IsSet("grpc.keepalive-time") {
grpcKeepaliveTime = clampNonNegativeDuration(v.GetDuration("grpc.keepalive-time"), DefaultGRPCKeepaliveTime)
}
grpcKeepaliveTimeout := DefaultGRPCKeepaliveTimeout
if v.IsSet("grpc.keepalive-timeout") {
grpcKeepaliveTimeout = clampNonNegativeDuration(v.GetDuration("grpc.keepalive-timeout"), DefaultGRPCKeepaliveTimeout)
}
grpcKeepaliveMinTime := DefaultGRPCKeepaliveMinTime
if v.IsSet("grpc.keepalive-min-time") {
grpcKeepaliveMinTime = clampNonNegativeDuration(v.GetDuration("grpc.keepalive-min-time"), DefaultGRPCKeepaliveMinTime)
}
// MaxConnectionAge and MaxConnectionAgeGrace default to 0 (gRPC's "infinity"),
// which is a valid value, so only a negative override needs clamping.
grpcMaxConnectionAge := clampNonNegativeDuration(v.GetDuration("grpc.max-connection-age"), DefaultGRPCMaxConnectionAge)
grpcMaxConnectionAgeGrace := clampNonNegativeDuration(v.GetDuration("grpc.max-connection-age-grace"), DefaultGRPCMaxConnectionAgeGrace)

return Config{
BaseConfig: BaseConfig{
MinGasPrices: v.GetString("minimum-gas-prices"),
Expand Down Expand Up @@ -416,8 +551,17 @@ func GetConfig(v *viper.Viper) (Config, error) {
Offline: v.GetBool("rosetta.offline"),
},
GRPC: GRPCConfig{
Enable: v.GetBool("grpc.enable"),
Address: v.GetString("grpc.address"),
Enable: v.GetBool("grpc.enable"),
Address: v.GetString("grpc.address"),
MaxRecvMsgSize: grpcMaxRecvMsgSize,
MaxOpenConnections: grpcMaxOpenConnections,
MaxConnectionIdle: grpcMaxConnectionIdle,
MaxConnectionAge: grpcMaxConnectionAge,
MaxConnectionAgeGrace: grpcMaxConnectionAgeGrace,
KeepaliveTime: grpcKeepaliveTime,
KeepaliveTimeout: grpcKeepaliveTimeout,
KeepaliveMinTime: grpcKeepaliveMinTime,
KeepalivePermitWithoutStream: v.GetBool("grpc.keepalive-permit-without-stream"),
},
GRPCWeb: GRPCWebConfig{
Enable: v.GetBool("grpc-web.enable"),
Expand Down
112 changes: 112 additions & 0 deletions sei-cosmos/server/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config
import (
"bytes"
"testing"
"time"

tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config"
"github.com/spf13/viper"
Expand Down Expand Up @@ -68,6 +69,117 @@ func TestDefaultGRPCConfig(t *testing.T) {
cfg := DefaultConfig()
require.True(t, cfg.GRPC.Enable)
require.Equal(t, DefaultGRPCAddress, cfg.GRPC.Address)
require.Equal(t, DefaultGRPCMaxRecvMsgSize, cfg.GRPC.MaxRecvMsgSize)
require.Equal(t, uint(DefaultGRPCMaxOpenConnections), cfg.GRPC.MaxOpenConnections)
require.Equal(t, DefaultGRPCMaxConnectionIdle, cfg.GRPC.MaxConnectionIdle)
require.Equal(t, 5*time.Minute, cfg.GRPC.MaxConnectionIdle)
require.Equal(t, DefaultGRPCMaxConnectionAge, cfg.GRPC.MaxConnectionAge)
require.Equal(t, DefaultGRPCMaxConnectionAgeGrace, cfg.GRPC.MaxConnectionAgeGrace)
require.Equal(t, DefaultGRPCKeepaliveTime, cfg.GRPC.KeepaliveTime)
require.Equal(t, DefaultGRPCKeepaliveTimeout, cfg.GRPC.KeepaliveTimeout)
require.Equal(t, DefaultGRPCKeepaliveMinTime, cfg.GRPC.KeepaliveMinTime)
require.Equal(t, DefaultGRPCKeepalivePermitWithoutStream, cfg.GRPC.KeepalivePermitWithoutStream)
}

// seedViperWithDefaultConfig renders the default app config template and reads
// it into a fresh viper instance, mirroring how seid loads app.toml.
func seedViperWithDefaultConfig(t *testing.T) *viper.Viper {
t.Helper()
var buf bytes.Buffer
require.NoError(t, configTemplate.Execute(&buf, DefaultConfig()))
v := viper.New()
v.SetConfigType("toml")
require.NoError(t, v.ReadConfig(&buf))
return v
}

// TestGetConfigGRPCDefaultsWhenAbsent ensures a node upgrading with an older
// app.toml (which lacks the new gRPC keys) still gets the bounded in-code
// defaults rather than zero/unlimited values.
func TestGetConfigGRPCDefaultsWhenAbsent(t *testing.T) {
// Minimal app.toml that predates the new gRPC keys. global-labels is the
// only key GetConfig hard-requires.
const legacyAppToml = `
[telemetry]
global-labels = []

[grpc]
enable = true
address = "0.0.0.0:9090"
`
v := viper.New()
v.SetConfigType("toml")
require.NoError(t, v.ReadConfig(bytes.NewBufferString(legacyAppToml)))
require.False(t, v.IsSet("grpc.max-recv-msg-size"))
require.False(t, v.IsSet("grpc.max-open-connections"))
require.False(t, v.IsSet("grpc.max-connection-idle"))
require.False(t, v.IsSet("grpc.max-connection-age"))
require.False(t, v.IsSet("grpc.max-connection-age-grace"))
require.False(t, v.IsSet("grpc.keepalive-permit-without-stream"))

cfg, err := GetConfig(v)
require.NoError(t, err)
require.Equal(t, DefaultGRPCMaxRecvMsgSize, cfg.GRPC.MaxRecvMsgSize)
require.Equal(t, uint(DefaultGRPCMaxOpenConnections), cfg.GRPC.MaxOpenConnections)
// The bounded idle default must survive an older app.toml that omits the key.
require.Equal(t, DefaultGRPCMaxConnectionIdle, cfg.GRPC.MaxConnectionIdle)
require.Equal(t, DefaultGRPCKeepaliveTime, cfg.GRPC.KeepaliveTime)
require.Equal(t, DefaultGRPCKeepaliveTimeout, cfg.GRPC.KeepaliveTimeout)
require.Equal(t, DefaultGRPCKeepaliveMinTime, cfg.GRPC.KeepaliveMinTime)
// The directly-read fields (no IsSet guard) must still resolve to their
// in-code defaults when the keys are absent.
require.Equal(t, DefaultGRPCMaxConnectionAge, cfg.GRPC.MaxConnectionAge)
require.Equal(t, DefaultGRPCMaxConnectionAgeGrace, cfg.GRPC.MaxConnectionAgeGrace)
require.Equal(t, DefaultGRPCKeepalivePermitWithoutStream, cfg.GRPC.KeepalivePermitWithoutStream)
}

// TestGetConfigGRPCClampsNegativeDurations ensures a misconfigured negative
// keepalive/connection-age duration falls back to the safe in-code default
// rather than being passed verbatim to the gRPC server.
func TestGetConfigGRPCClampsNegativeDurations(t *testing.T) {
v := seedViperWithDefaultConfig(t)
v.Set("grpc.max-connection-idle", "-1s")
v.Set("grpc.max-connection-age", "-1s")
v.Set("grpc.max-connection-age-grace", "-1s")
v.Set("grpc.keepalive-time", "-1s")
v.Set("grpc.keepalive-timeout", "-1s")
v.Set("grpc.keepalive-min-time", "-1s")

cfg, err := GetConfig(v)
require.NoError(t, err)
require.Equal(t, DefaultGRPCMaxConnectionIdle, cfg.GRPC.MaxConnectionIdle)
require.Equal(t, DefaultGRPCMaxConnectionAge, cfg.GRPC.MaxConnectionAge)
require.Equal(t, DefaultGRPCMaxConnectionAgeGrace, cfg.GRPC.MaxConnectionAgeGrace)
require.Equal(t, DefaultGRPCKeepaliveTime, cfg.GRPC.KeepaliveTime)
require.Equal(t, DefaultGRPCKeepaliveTimeout, cfg.GRPC.KeepaliveTimeout)
require.Equal(t, DefaultGRPCKeepaliveMinTime, cfg.GRPC.KeepaliveMinTime)
}

// TestGetConfigGRPCOverrides ensures operator-provided values override the
// in-code defaults.
func TestGetConfigGRPCOverrides(t *testing.T) {
v := seedViperWithDefaultConfig(t)
v.Set("grpc.max-recv-msg-size", 8*1024*1024)
v.Set("grpc.max-open-connections", 50)
v.Set("grpc.max-connection-idle", "5m")
v.Set("grpc.max-connection-age", "30m")
v.Set("grpc.max-connection-age-grace", "1m")
v.Set("grpc.keepalive-time", "1m")
v.Set("grpc.keepalive-timeout", "10s")
v.Set("grpc.keepalive-min-time", "30s")
v.Set("grpc.keepalive-permit-without-stream", true)

cfg, err := GetConfig(v)
require.NoError(t, err)
require.Equal(t, 8*1024*1024, cfg.GRPC.MaxRecvMsgSize)
require.Equal(t, uint(50), cfg.GRPC.MaxOpenConnections)
require.Equal(t, 5*time.Minute, cfg.GRPC.MaxConnectionIdle)
require.Equal(t, 30*time.Minute, cfg.GRPC.MaxConnectionAge)
require.Equal(t, time.Minute, cfg.GRPC.MaxConnectionAgeGrace)
require.Equal(t, time.Minute, cfg.GRPC.KeepaliveTime)
require.Equal(t, 10*time.Second, cfg.GRPC.KeepaliveTimeout)
require.Equal(t, 30*time.Second, cfg.GRPC.KeepaliveMinTime)
require.True(t, cfg.GRPC.KeepalivePermitWithoutStream)
}

func TestDefaultGRPCWebConfig(t *testing.T) {
Expand Down
28 changes: 28 additions & 0 deletions sei-cosmos/server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,34 @@ enable = {{ .GRPC.Enable }}
# Address defines the gRPC server address to bind to.
address = "{{ .GRPC.Address }}"

# MaxRecvMsgSize defines the maximum message size in bytes the server can receive.
# It bounds per-request memory allocation before the rate limiter fires. Default 4 MB.
max-recv-msg-size = {{ .GRPC.MaxRecvMsgSize }}

# MaxOpenConnections defines the maximum number of simultaneous open connections. 0 means unlimited.
max-open-connections = {{ .GRPC.MaxOpenConnections }}

# MaxConnectionIdle is the duration after which an idle connection is closed (e.g. "5m"). 0 means infinity.
max-connection-idle = "{{ .GRPC.MaxConnectionIdle }}"

# MaxConnectionAge is the maximum duration a connection may exist before it is closed (e.g. "30m"). 0 means infinity.
max-connection-age = "{{ .GRPC.MaxConnectionAge }}"

# MaxConnectionAgeGrace is an additive period after max-connection-age during which the connection is forcibly closed. 0 means infinity.
max-connection-age-grace = "{{ .GRPC.MaxConnectionAgeGrace }}"

# KeepaliveTime is the interval after which, with no activity, the server pings the client to check liveness.
keepalive-time = "{{ .GRPC.KeepaliveTime }}"

# KeepaliveTimeout is the duration the server waits for a keepalive ping ack before closing the connection.
keepalive-timeout = "{{ .GRPC.KeepaliveTimeout }}"

# KeepaliveMinTime is the minimum interval a client must wait between keepalive pings; more frequent pings are penalized.
keepalive-min-time = "{{ .GRPC.KeepaliveMinTime }}"

# KeepalivePermitWithoutStream defines whether the server allows keepalive pings even when there are no active streams.
keepalive-permit-without-stream = {{ .GRPC.KeepalivePermitWithoutStream }}

###############################################################################
### gRPC Web Configuration (Auto-managed) ###
###############################################################################
Expand Down
Loading
Loading