Skip to content

Commit

Permalink
Added checks to RPC requests and introduced new flags to customise th…
Browse files Browse the repository at this point in the history
…e parameters (#657)

* added a check to reject rpc requests with batch size > the one set using a newly added flag (rpcbatchlimit)

* added a check to reject rpc requests whose result size > the one set using a newly added flag (rpcreturndatalimit)

* updated the config files and docs
  • Loading branch information
pratikspatil024 authored Jan 19, 2023
1 parent 66c23d1 commit e4dd2ee
Show file tree
Hide file tree
Showing 24 changed files with 121 additions and 24 deletions.
2 changes: 2 additions & 0 deletions builder/files/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ chain = "mainnet"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = "/var/lib/bor/keystore"
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions docs/cli/example_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ log-level = "INFO" # Set log level for the server
datadir = "var/lib/bor" # Path of the data directory to store information
ancient = "" # Data directory for ancient chain segments (default = inside chaindata)
keystore = "" # Path of the directory where keystores are located
"rpc.batchlimit" = 100 # Maximum number of messages in a batch (default=100, use 0 for no limits)
"rpc.returndatalimit" = 100000 # Maximum size (in bytes) a result of an rpc request could have (default=100000, use 0 for no limits)
syncmode = "full" # Blockchain sync mode (only "full" sync supported)
gcmode = "full" # Blockchain garbage collection mode ("full", "archive")
snapshot = true # Enables the snapshot-database mode
Expand Down
4 changes: 4 additions & 0 deletions docs/cli/server.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ The ```bor server``` command runs the Bor client.

- ```keystore```: Path of the directory where keystores are located

- ```rpc.batchlimit```: Maximum number of messages in a batch (default=100, use 0 for no limits) (default: 100)

- ```rpc.returndatalimit```: Maximum size (in bytes) a result of an rpc request could have (default=100000, use 0 for no limits) (default: 100000)

- ```config```: File for the config file

- ```syncmode```: Blockchain sync mode (only "full" sync supported) (default: full)
Expand Down
4 changes: 4 additions & 0 deletions eth/api_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ func (b *EthAPIBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}

func (b *EthAPIBackend) RPCRpcReturnDataLimit() uint64 {
return b.eth.config.RPCReturnDataLimit
}

func (b *EthAPIBackend) RPCEVMTimeout() time.Duration {
return b.eth.config.RPCEVMTimeout
}
Expand Down
14 changes: 9 additions & 5 deletions eth/ethconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,12 @@ var Defaults = Config{
GasPrice: big.NewInt(params.GWei),
Recommit: 125 * time.Second,
},
TxPool: core.DefaultTxPoolConfig,
RPCGasCap: 50000000,
RPCEVMTimeout: 5 * time.Second,
GPO: FullNodeGPO,
RPCTxFeeCap: 5, // 5 matic
TxPool: core.DefaultTxPoolConfig,
RPCGasCap: 50000000,
RPCReturnDataLimit: 100000,
RPCEVMTimeout: 5 * time.Second,
GPO: FullNodeGPO,
RPCTxFeeCap: 5, // 5 matic
}

func init() {
Expand Down Expand Up @@ -199,6 +200,9 @@ type Config struct {
// RPCGasCap is the global gas cap for eth-call variants.
RPCGasCap uint64

// Maximum size (in bytes) a result of an rpc request could have
RPCReturnDataLimit uint64

// RPCEVMTimeout is the global timeout for eth-call.
RPCEVMTimeout time.Duration

Expand Down
4 changes: 4 additions & 0 deletions eth/tracers/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func (b *testBackend) RPCGasCap() uint64 {
return 25000000
}

func (b *testBackend) RPCRpcReturnDataLimit() uint64 {
return 100000
}

func (b *testBackend) ChainConfig() *params.ChainConfig {
return b.chainConfig
}
Expand Down
23 changes: 17 additions & 6 deletions internal/cli/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ type Config struct {
// KeyStoreDir is the directory to store keystores
KeyStoreDir string `hcl:"keystore,optional" toml:"keystore,optional"`

// Maximum number of messages in a batch (default=100, use 0 for no limits)
RPCBatchLimit uint64 `hcl:"rpc.batchlimit,optional" toml:"rpc.batchlimit,optional"`

// Maximum size (in bytes) a result of an rpc request could have (default=100000, use 0 for no limits)
RPCReturnDataLimit uint64 `hcl:"rpc.returndatalimit,optional" toml:"rpc.returndatalimit,optional"`

// SyncMode selects the sync protocol
SyncMode string `hcl:"syncmode,optional" toml:"syncmode,optional"`

Expand Down Expand Up @@ -435,12 +441,14 @@ type DeveloperConfig struct {

func DefaultConfig() *Config {
return &Config{
Chain: "mainnet",
Identity: Hostname(),
RequiredBlocks: map[string]string{},
LogLevel: "INFO",
DataDir: DefaultDataDir(),
Ancient: "",
Chain: "mainnet",
Identity: Hostname(),
RequiredBlocks: map[string]string{},
LogLevel: "INFO",
DataDir: DefaultDataDir(),
Ancient: "",
RPCBatchLimit: 100,
RPCReturnDataLimit: 100000,
P2P: &P2PConfig{
MaxPeers: 50,
MaxPendPeers: 50,
Expand Down Expand Up @@ -936,6 +944,8 @@ func (c *Config) buildEth(stack *node.Node, accountManager *accounts.Manager) (*
n.BorLogs = c.BorLogs
n.DatabaseHandles = dbHandles

n.RPCReturnDataLimit = c.RPCReturnDataLimit

if c.Ancient != "" {
n.DatabaseFreezer = c.Ancient
}
Expand Down Expand Up @@ -986,6 +996,7 @@ func (c *Config) buildNode() (*node.Config, error) {
WriteTimeout: c.JsonRPC.HttpTimeout.WriteTimeout,
IdleTimeout: c.JsonRPC.HttpTimeout.IdleTimeout,
},
RPCBatchLimit: c.RPCBatchLimit,
}

// dev mode
Expand Down
12 changes: 12 additions & 0 deletions internal/cli/server/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ func (c *Command) Flags() *flagset.Flagset {
Usage: "Path of the directory where keystores are located",
Value: &c.cliConfig.KeyStoreDir,
})
f.Uint64Flag(&flagset.Uint64Flag{
Name: "rpc.batchlimit",
Usage: "Maximum number of messages in a batch (default=100, use 0 for no limits)",
Value: &c.cliConfig.RPCBatchLimit,
Default: c.cliConfig.RPCBatchLimit,
})
f.Uint64Flag(&flagset.Uint64Flag{
Name: "rpc.returndatalimit",
Usage: "Maximum size (in bytes) a result of an rpc request could have (default=100000, use 0 for no limits)",
Value: &c.cliConfig.RPCReturnDataLimit,
Default: c.cliConfig.RPCReturnDataLimit,
})
f.StringFlag(&flagset.StringFlag{
Name: "config",
Usage: "File for the config file",
Expand Down
5 changes: 5 additions & 0 deletions internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,11 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args TransactionArgs, bl
if err != nil {
return nil, err
}

if int(s.b.RPCRpcReturnDataLimit()) > 0 && len(result.ReturnData) > int(s.b.RPCRpcReturnDataLimit()) {
return nil, fmt.Errorf("call returned result of length %d exceeding limit %d", len(result.ReturnData), int(s.b.RPCRpcReturnDataLimit()))
}

// If the result contains a revert reason, try to unpack and return it.
if len(result.Revert()) > 0 {
return nil, newRevertError(result)
Expand Down
9 changes: 5 additions & 4 deletions internal/ethapi/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ type Backend interface {
ChainDb() ethdb.Database
AccountManager() *accounts.Manager
ExtRPCEnabled() bool
RPCGasCap() uint64 // global gas cap for eth_call over rpc: DoS protection
RPCEVMTimeout() time.Duration // global timeout for eth_call over rpc: DoS protection
RPCTxFeeCap() float64 // global tx fee cap for all transaction related APIs
UnprotectedAllowed() bool // allows only for EIP155 transactions.
RPCGasCap() uint64 // global gas cap for eth_call over rpc: DoS protection
RPCRpcReturnDataLimit() uint64 // Maximum size (in bytes) a result of an rpc request could have
RPCEVMTimeout() time.Duration // global timeout for eth_call over rpc: DoS protection
RPCTxFeeCap() float64 // global tx fee cap for all transaction related APIs
UnprotectedAllowed() bool // allows only for EIP155 transactions.

// Blockchain API
SetHead(number uint64)
Expand Down
4 changes: 4 additions & 0 deletions les/api_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ func (b *LesApiBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}

func (b *LesApiBackend) RPCRpcReturnDataLimit() uint64 {
return b.eth.config.RPCReturnDataLimit
}

func (b *LesApiBackend) RPCEVMTimeout() time.Duration {
return b.eth.config.RPCEVMTimeout
}
Expand Down
3 changes: 3 additions & 0 deletions node/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ type Config struct {

// JWTSecret is the hex-encoded jwt secret.
JWTSecret string `toml:",omitempty"`

// Maximum number of messages in a batch
RPCBatchLimit uint64 `toml:",omitempty"`
}

// IPCEndpoint resolves an IPC endpoint based on a configured value, taking into
Expand Down
11 changes: 7 additions & 4 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ func New(conf *Config) (*Node, error) {
databases: make(map[*closeTrackingDB]struct{}),
}

// set RPC batch limit
node.inprocHandler.SetRPCBatchLimit(conf.RPCBatchLimit)

// Register built-in APIs.
node.rpcAPIs = append(node.rpcAPIs, node.apis()...)

Expand Down Expand Up @@ -153,10 +156,10 @@ func New(conf *Config) (*Node, error) {
}

// Configure RPC servers.
node.http = newHTTPServer(node.log, conf.HTTPTimeouts)
node.httpAuth = newHTTPServer(node.log, conf.HTTPTimeouts)
node.ws = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts)
node.wsAuth = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts)
node.http = newHTTPServer(node.log, conf.HTTPTimeouts, conf.RPCBatchLimit)
node.httpAuth = newHTTPServer(node.log, conf.HTTPTimeouts, conf.RPCBatchLimit)
node.ws = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts, conf.RPCBatchLimit)
node.wsAuth = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts, conf.RPCBatchLimit)
node.ipc = newIPCServer(node.log, conf.IPCEndpoint())

return node, nil
Expand Down
8 changes: 6 additions & 2 deletions node/rpcstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ type httpServer struct {
port int

handlerNames map[string]string

RPCBatchLimit uint64
}

func newHTTPServer(log log.Logger, timeouts rpc.HTTPTimeouts) *httpServer {
h := &httpServer{log: log, timeouts: timeouts, handlerNames: make(map[string]string)}
func newHTTPServer(log log.Logger, timeouts rpc.HTTPTimeouts, rpcBatchLimit uint64) *httpServer {
h := &httpServer{log: log, timeouts: timeouts, handlerNames: make(map[string]string), RPCBatchLimit: rpcBatchLimit}

h.httpHandler.Store((*rpcHandler)(nil))
h.wsHandler.Store((*rpcHandler)(nil))
Expand Down Expand Up @@ -283,6 +285,7 @@ func (h *httpServer) enableRPC(apis []rpc.API, config httpConfig) error {

// Create RPC server and handler.
srv := rpc.NewServer()
srv.SetRPCBatchLimit(h.RPCBatchLimit)
if err := RegisterApis(apis, config.Modules, srv, false); err != nil {
return err
}
Expand Down Expand Up @@ -314,6 +317,7 @@ func (h *httpServer) enableWS(apis []rpc.API, config wsConfig) error {
}
// Create RPC server and handler.
srv := rpc.NewServer()
srv.SetRPCBatchLimit(h.RPCBatchLimit)
if err := RegisterApis(apis, config.Modules, srv, false); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion node/rpcstack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func Test_checkPath(t *testing.T) {
func createAndStartServer(t *testing.T, conf *httpConfig, ws bool, wsConf *wsConfig) *httpServer {
t.Helper()

srv := newHTTPServer(testlog.Logger(t, log.LvlDebug), rpc.DefaultHTTPTimeouts)
srv := newHTTPServer(testlog.Logger(t, log.LvlDebug), rpc.DefaultHTTPTimeouts, 100)
assert.NoError(t, srv.enableRPC(nil, *conf))
if ws {
assert.NoError(t, srv.enableWS(nil, *wsConf))
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/mainnet-v1/archive/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ chain = "mainnet"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = ""
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
gcmode = "archive"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/mainnet-v1/sentry/sentry/bor/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ chain = "mainnet"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = ""
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ chain = "mainnet"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = "$BOR_DIR/keystore"
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/mainnet-v1/without-sentry/bor/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ chain = "mainnet"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = "$BOR_DIR/keystore"
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/testnet-v4/archive/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ chain = "mumbai"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = ""
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
gcmode = "archive"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/testnet-v4/sentry/sentry/bor/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ chain = "mumbai"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = ""
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ chain = "mumbai"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = "$BOR_DIR/keystore"
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
2 changes: 2 additions & 0 deletions packaging/templates/testnet-v4/without-sentry/bor/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ chain = "mumbai"
datadir = "/var/lib/bor/data"
# ancient = ""
# keystore = "$BOR_DIR/keystore"
# "rpc.batchlimit" = 100
# "rpc.returndatalimit" = 100000
syncmode = "full"
# gcmode = "full"
# snapshot = true
Expand Down
22 changes: 20 additions & 2 deletions rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rpc

import (
"context"
"fmt"
"io"
"sync/atomic"

Expand Down Expand Up @@ -47,6 +48,8 @@ type Server struct {
idgen func() ID
run int32
codecs mapset.Set

BatchLimit uint64
}

// NewServer creates a new server instance with no registered handlers.
Expand All @@ -59,6 +62,10 @@ func NewServer() *Server {
return server
}

func (s *Server) SetRPCBatchLimit(batchLimit uint64) {
s.BatchLimit = batchLimit
}

// RegisterName creates a service for the given receiver type under the given name. When no
// methods on the given receiver match the criteria to be either a RPC method or a
// subscription an error is returned. Otherwise a new service is created and added to the
Expand Down Expand Up @@ -105,12 +112,23 @@ func (s *Server) serveSingleRequest(ctx context.Context, codec ServerCodec) {
reqs, batch, err := codec.readBatch()
if err != nil {
if err != io.EOF {
codec.writeJSON(ctx, errorMessage(&invalidMessageError{"parse error"}))
if err1 := codec.writeJSON(ctx, err); err1 != nil {
log.Warn("WARNING - error in reading batch", "err", err1)
return
}
}
return
}

if batch {
h.handleBatch(reqs)
if s.BatchLimit > 0 && len(reqs) > int(s.BatchLimit) {
if err1 := codec.writeJSON(ctx, errorMessage(fmt.Errorf("batch limit %d exceeded: %d requests given", s.BatchLimit, len(reqs)))); err1 != nil {
log.Warn("WARNING - requests given exceeds the batch limit", "err", err1)
log.Debug("batch limit %d exceeded: %d requests given", s.BatchLimit, len(reqs))
}
} else {
h.handleBatch(reqs)
}
} else {
h.handleMsg(reqs[0])
}
Expand Down

0 comments on commit e4dd2ee

Please sign in to comment.