Skip to content

Commit

Permalink
Merge pull request #137 from SigmaGmbH/feat/disable-unsafe-eth-endpoints
Browse files Browse the repository at this point in the history
Feat/disable unsafe eth endpoints
  • Loading branch information
MikkySnow authored Oct 8, 2024
2 parents e6ac60a + 84a2ed0 commit 3f0134e
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 135 deletions.
3 changes: 0 additions & 3 deletions rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import (
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethereum/go-ethereum/signer/core/apitypes"

rpctypes "swisstronik/rpc/types"
"swisstronik/server/config"
ethermint "swisstronik/types"
Expand Down Expand Up @@ -75,7 +73,6 @@ type EVMBackend interface {
// Sign Tx
Sign(address common.Address, data hexutil.Bytes) (hexutil.Bytes, error)
SendTransaction(args evmtypes.TransactionArgs) (common.Hash, error)
SignTypedData(address common.Address, typedData apitypes.TypedData) (hexutil.Bytes, error)

// Blocks Info
BlockNumber() (hexutil.Uint64, error)
Expand Down
3 changes: 3 additions & 0 deletions rpc/backend/backend_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ func (suite *BackendTestSuite) SetupTest() {
suite.backend.queryClient.FeeMarket = mocks.NewFeeMarketQueryClient(suite.T())
suite.backend.ctx = rpctypes.ContextWithHeight(1)

// Enable unsafe eth endpoints for testing
suite.backend.cfg.JSONRPC.UnsafeEthEndpointsEnabled = true

// Add codec
encCfg := encoding.MakeConfig(app.ModuleBasics)
suite.backend.clientCtx.Codec = encCfg.Codec
Expand Down
34 changes: 7 additions & 27 deletions rpc/backend/sign_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ import (
"github.com/ethereum/go-ethereum/common/hexutil"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/signer/core/apitypes"

evmtypes "swisstronik/x/evm/types"
)

// SendTransaction sends transaction based on received args using Node's key to sign it
func (b *Backend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash, error) {
if !b.cfg.JSONRPC.UnsafeEthEndpointsEnabled {
return common.Hash{}, fmt.Errorf("eth_sendTransaction not enabled")
}

// Look up the wallet containing the requested signer
_, err := b.clientCtx.Keyring.KeyByAddress(sdk.AccAddress(args.GetFrom().Bytes()))
if err != nil {
Expand Down Expand Up @@ -121,27 +123,10 @@ func (b *Backend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash, e

// Sign signs the provided data using the private key of address via Geth's signature standard.
func (b *Backend) Sign(address common.Address, data hexutil.Bytes) (hexutil.Bytes, error) {
from := sdk.AccAddress(address.Bytes())

_, err := b.clientCtx.Keyring.KeyByAddress(from)
if err != nil {
b.logger.Error("failed to find key in keyring", "address", address.String())
return nil, fmt.Errorf("%s; %s", keystore.ErrNoMatch, err.Error())
if !b.cfg.JSONRPC.UnsafeEthEndpointsEnabled {
return nil, fmt.Errorf("eth_sign not enabled")
}

// Sign the requested hash with the wallet
signature, _, err := b.clientCtx.Keyring.SignByAddress(from, data)
if err != nil {
b.logger.Error("keyring.SignByAddress failed", "address", address.Hex())
return nil, err
}

signature[crypto.RecoveryIDOffset] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
return signature, nil
}

// SignTypedData signs EIP-712 conformant typed data
func (b *Backend) SignTypedData(address common.Address, typedData apitypes.TypedData) (hexutil.Bytes, error) {
from := sdk.AccAddress(address.Bytes())

_, err := b.clientCtx.Keyring.KeyByAddress(from)
Expand All @@ -150,13 +135,8 @@ func (b *Backend) SignTypedData(address common.Address, typedData apitypes.Typed
return nil, fmt.Errorf("%s; %s", keystore.ErrNoMatch, err.Error())
}

sigHash, _, err := apitypes.TypedDataAndHash(typedData)
if err != nil {
return nil, err
}

// Sign the requested hash with the wallet
signature, _, err := b.clientCtx.Keyring.SignByAddress(from, sigHash)
signature, _, err := b.clientCtx.Keyring.SignByAddress(from, data)
if err != nil {
b.logger.Error("keyring.SignByAddress failed", "address", address.Hex())
return nil, err
Expand Down
50 changes: 0 additions & 50 deletions rpc/backend/sign_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/ethereum/go-ethereum/common/hexutil"
ethtypes "github.com/ethereum/go-ethereum/core/types"
goethcrypto "github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/signer/core/apitypes"
"google.golang.org/grpc/metadata"

"swisstronik/crypto/ethsecp256k1"
Expand Down Expand Up @@ -227,52 +226,3 @@ func (suite *BackendTestSuite) TestSign() {
})
}
}

func (suite *BackendTestSuite) TestSignTypedData() {
from, priv := tests.RandomEthAddressWithPrivateKey()
testCases := []struct {
name string
registerMock func()
fromAddr common.Address
inputTypedData apitypes.TypedData
expPass bool
}{
{
"fail - can't find key in Keyring",
func() {},
from,
apitypes.TypedData{},
false,
},
{
"fail - empty TypeData",
func() {
armor := crypto.EncryptArmorPrivKey(priv, "", "eth_secp256k1")
err := suite.backend.clientCtx.Keyring.ImportPrivKey("test_key", armor, "")
suite.Require().NoError(err)
},
from,
apitypes.TypedData{},
false,
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("case %s", tc.name), func() {
suite.SetupTest() // reset test and queries
tc.registerMock()

responseBz, err := suite.backend.SignTypedData(tc.fromAddr, tc.inputTypedData)

if tc.expPass {
sigHash, _, err := apitypes.TypedDataAndHash(tc.inputTypedData)
signature, _, err := suite.backend.clientCtx.Keyring.SignByAddress((sdk.AccAddress)(from.Bytes()), sigHash)
signature[goethcrypto.RecoveryIDOffset] += 27
suite.Require().NoError(err)
suite.Require().Equal((hexutil.Bytes)(signature), responseBz)
} else {
suite.Require().Error(err)
}
})
}
}
9 changes: 0 additions & 9 deletions rpc/namespaces/ethereum/eth/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package eth
import (
"context"

"github.com/ethereum/go-ethereum/signer/core/apitypes"

"github.com/ethereum/go-ethereum/rpc"

"github.com/cometbft/cometbft/libs/log"
Expand Down Expand Up @@ -113,7 +111,6 @@ type EthereumAPI interface {
Coinbase() (string, error)
Sign(address common.Address, data hexutil.Bytes) (hexutil.Bytes, error)
GetTransactionLogs(txHash common.Hash) ([]*ethtypes.Log, error)
SignTypedData(address common.Address, typedData apitypes.TypedData) (hexutil.Bytes, error)
FillTransaction(args evmtypes.TransactionArgs) (*rpctypes.SignTransactionResult, error)
Resend(ctx context.Context, args evmtypes.TransactionArgs, gasPrice *hexutil.Big, gasLimit *hexutil.Uint64) (common.Hash, error)
GetPendingTransactions() ([]*rpctypes.RPCTransaction, error)
Expand Down Expand Up @@ -454,12 +451,6 @@ func (e *PublicAPI) GetTransactionLogs(txHash common.Hash) ([]*ethtypes.Log, err
return backend.TxLogsFromEvents(resBlockResult.TxsResults[res.TxIndex].Events, int(res.MsgIndex))
}

// SignTypedData signs EIP-712 conformant typed data
func (e *PublicAPI) SignTypedData(address common.Address, typedData apitypes.TypedData) (hexutil.Bytes, error) {
e.logger.Debug("eth_signTypedData", "address", address.Hex(), "data", typedData)
return e.backend.SignTypedData(address, typedData)
}

// FillTransaction fills the defaults (nonce, gas, gasPrice or 1559 fields)
// on a given unsigned transaction, and returns it to the caller for further
// processing (signing + broadcast).
Expand Down
7 changes: 5 additions & 2 deletions scripts/local-node.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ sed -i 's/prometheus = false/prometheus = true/' "$CONFIG"
sed -i 's/prometheus-retention-time = "0"/prometheus-retention-time = "1000000000000"/g' "$APP_TOML"
sed -i 's/enabled = false/enabled = true/g' "$APP_TOML"

# disable unsafe eth endpoints
sed -i 's/unsafe-eth-endpoints-enabled = true/unsafe-eth-endpoints-enabled = false/' "$APP_TOML"

# set min gas price
sed -i 's/minimum-gas-prices = ""/minimum-gas-prices = "0aswtr"/' "$APP_TOML"

Expand All @@ -66,8 +69,8 @@ sed -i.bak 's/"voting_period": "172800s"/"voting_period": "30s"/g' "$HOMEDIR"/co

# set custom pruning settings
sed -i.bak 's/pruning = "default"/pruning = "custom"/g' "$APP_TOML"
sed -i.bak 's/pruning-keep-recent = "0"/pruning-keep-recent = "2"/g' "$APP_TOML"
sed -i.bak 's/pruning-interval = "0"/pruning-interval = "10"/g' "$APP_TOML"
sed -i.bak 's/pruning-keep-recent = "0"/pruning-keep-recent = "100"/g' "$APP_TOML"
sed -i.bak 's/pruning-interval = "0"/pruning-interval = "500"/g' "$APP_TOML"

# Allocate genesis accounts
$BINARY add-genesis-account alice 100000000swtr --keyring-backend $KEYRING --home "$HOMEDIR"
Expand Down
85 changes: 43 additions & 42 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ const (

// DefaultMaxOpenConnections represents the amount of open connections (unlimited = 0)
DefaultMaxOpenConnections = 0

// DefaultSeedExchangeServerAddress is the default address the seed exchange server binds to.
DefaultSeedExchangeServerAddress = "127.0.0.1:8999"
)

var evmTracers = []string{"json", "markdown", "struct", "access_list"}
Expand Down Expand Up @@ -151,6 +148,8 @@ type JSONRPCConfig struct {
MetricsAddress string `mapstructure:"metrics-address"`
// FixRevertGasRefundHeight defines the upgrade height for fix of revert gas refund logic when transaction reverted
FixRevertGasRefundHeight int64 `mapstructure:"fix-revert-gas-refund-height"`
// UnsafeEthEndpointsEnabled defines if eth_sendTransaction endpoint is enabled
UnsafeEthEndpointsEnabled bool `mapstructure:"unsafe-eth-endpoints-enabled"`
}

// TLSConfig defines the certificate and matching private key for the server.
Expand Down Expand Up @@ -236,26 +235,27 @@ func GetAPINamespaces() []string {
// DefaultJSONRPCConfig returns an EVM config with the JSON-RPC API enabled by default
func DefaultJSONRPCConfig() *JSONRPCConfig {
return &JSONRPCConfig{
Enable: true,
API: GetDefaultAPINamespaces(),
Address: DefaultJSONRPCAddress,
WsAddress: DefaultJSONRPCWsAddress,
UnencryptedAddress: DefaultUnencryptedJSONRPCAddress,
UnencryptedWsAddress: DefaultUnencryptedJSONRPCWsAddress,
GasCap: DefaultGasCap,
EVMTimeout: DefaultEVMTimeout,
TxFeeCap: DefaultTxFeeCap,
FilterCap: DefaultFilterCap,
FeeHistoryCap: DefaultFeeHistoryCap,
BlockRangeCap: DefaultBlockRangeCap,
LogsCap: DefaultLogsCap,
HTTPTimeout: DefaultHTTPTimeout,
HTTPIdleTimeout: DefaultHTTPIdleTimeout,
AllowUnprotectedTxs: DefaultAllowUnprotectedTxs,
MaxOpenConnections: DefaultMaxOpenConnections,
EnableIndexer: false,
MetricsAddress: DefaultJSONRPCMetricsAddress,
FixRevertGasRefundHeight: DefaultFixRevertGasRefundHeight,
Enable: true,
API: GetDefaultAPINamespaces(),
Address: DefaultJSONRPCAddress,
WsAddress: DefaultJSONRPCWsAddress,
UnencryptedAddress: DefaultUnencryptedJSONRPCAddress,
UnencryptedWsAddress: DefaultUnencryptedJSONRPCWsAddress,
GasCap: DefaultGasCap,
EVMTimeout: DefaultEVMTimeout,
TxFeeCap: DefaultTxFeeCap,
FilterCap: DefaultFilterCap,
FeeHistoryCap: DefaultFeeHistoryCap,
BlockRangeCap: DefaultBlockRangeCap,
LogsCap: DefaultLogsCap,
HTTPTimeout: DefaultHTTPTimeout,
HTTPIdleTimeout: DefaultHTTPIdleTimeout,
AllowUnprotectedTxs: DefaultAllowUnprotectedTxs,
MaxOpenConnections: DefaultMaxOpenConnections,
EnableIndexer: false,
MetricsAddress: DefaultJSONRPCMetricsAddress,
FixRevertGasRefundHeight: DefaultFixRevertGasRefundHeight,
UnsafeEthEndpointsEnabled: false, // eth_sendTransaction, eth_sign, eth_signTypedData are disabled by default to prevent stealing funds
}
}

Expand Down Expand Up @@ -349,25 +349,26 @@ func GetConfig(v *viper.Viper) (Config, error) {
MaxTxGasWanted: v.GetUint64("evm.max-tx-gas-wanted"),
},
JSONRPC: JSONRPCConfig{
Enable: v.GetBool("json-rpc.enable"),
API: v.GetStringSlice("json-rpc.api"),
Address: v.GetString("json-rpc.address"),
WsAddress: v.GetString("json-rpc.ws-address"),
UnencryptedAddress: v.GetString("json-rpc.address-unencrypted"),
UnencryptedWsAddress: v.GetString("json-rpc.ws-address-unencrypted"),
GasCap: v.GetUint64("json-rpc.gas-cap"),
FilterCap: v.GetInt32("json-rpc.filter-cap"),
FeeHistoryCap: v.GetInt32("json-rpc.feehistory-cap"),
TxFeeCap: v.GetFloat64("json-rpc.txfee-cap"),
EVMTimeout: v.GetDuration("json-rpc.evm-timeout"),
LogsCap: v.GetInt32("json-rpc.logs-cap"),
BlockRangeCap: v.GetInt32("json-rpc.block-range-cap"),
HTTPTimeout: v.GetDuration("json-rpc.http-timeout"),
HTTPIdleTimeout: v.GetDuration("json-rpc.http-idle-timeout"),
MaxOpenConnections: v.GetInt("json-rpc.max-open-connections"),
EnableIndexer: v.GetBool("json-rpc.enable-indexer"),
MetricsAddress: v.GetString("json-rpc.metrics-address"),
FixRevertGasRefundHeight: v.GetInt64("json-rpc.fix-revert-gas-refund-height"),
Enable: v.GetBool("json-rpc.enable"),
API: v.GetStringSlice("json-rpc.api"),
Address: v.GetString("json-rpc.address"),
WsAddress: v.GetString("json-rpc.ws-address"),
UnencryptedAddress: v.GetString("json-rpc.address-unencrypted"),
UnencryptedWsAddress: v.GetString("json-rpc.ws-address-unencrypted"),
GasCap: v.GetUint64("json-rpc.gas-cap"),
FilterCap: v.GetInt32("json-rpc.filter-cap"),
FeeHistoryCap: v.GetInt32("json-rpc.feehistory-cap"),
TxFeeCap: v.GetFloat64("json-rpc.txfee-cap"),
EVMTimeout: v.GetDuration("json-rpc.evm-timeout"),
LogsCap: v.GetInt32("json-rpc.logs-cap"),
BlockRangeCap: v.GetInt32("json-rpc.block-range-cap"),
HTTPTimeout: v.GetDuration("json-rpc.http-timeout"),
HTTPIdleTimeout: v.GetDuration("json-rpc.http-idle-timeout"),
MaxOpenConnections: v.GetInt("json-rpc.max-open-connections"),
EnableIndexer: v.GetBool("json-rpc.enable-indexer"),
MetricsAddress: v.GetString("json-rpc.metrics-address"),
FixRevertGasRefundHeight: v.GetInt64("json-rpc.fix-revert-gas-refund-height"),
UnsafeEthEndpointsEnabled: v.GetBool("json-rpc.unsafe-eth-endpoints-enabled"),
},
TLS: TLSConfig{
CertificatePath: v.GetString("tls.certificate-path"),
Expand Down
1 change: 1 addition & 0 deletions server/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ func TestDefaultConfig(t *testing.T) {
require.True(t, cfg.JSONRPC.Enable)
require.Equal(t, cfg.JSONRPC.Address, DefaultJSONRPCAddress)
require.Equal(t, cfg.JSONRPC.WsAddress, DefaultJSONRPCWsAddress)
require.Equal(t, cfg.JSONRPC.UnsafeEthEndpointsEnabled, false)
}
3 changes: 3 additions & 0 deletions server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ metrics-address = "{{ .JSONRPC.MetricsAddress }}"
# Upgrade height for fix of revert gas refund logic when transaction reverted.
fix-revert-gas-refund-height = {{ .JSONRPC.FixRevertGasRefundHeight }}
# UnsafeEthEndpointsEnabled enables eth_sendTransaction, eth_sign, eth_signTypedData. Enable it only if you are really need those endpoint
unsafe-eth-endpoints-enabled = {{ .JSONRPC.UnsafeEthEndpointsEnabled }}
###############################################################################
### TLS Configuration ###
###############################################################################
Expand Down
1 change: 1 addition & 0 deletions server/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const (
// https://github.com/ethereum/go-ethereum/blob/master/metrics/metrics.go#L35-L55
JSONRPCEnableMetrics = "metrics"
JSONRPCFixRevertGasRefundHeight = "json-rpc.fix-revert-gas-refund-height"
JSONRPCEnableUnsafeEndpoints = "json-rpc.enable-unsafe-endpoints"
)

// EVM flags
Expand Down
1 change: 1 addition & 0 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ which accepts a path for the resulting pprof file.
cmd.Flags().Bool(srvflags.JSONRPCEnableIndexer, false, "Enable the custom tx indexer for json-rpc")
cmd.Flags().Bool(srvflags.JSONRPCEnableMetrics, false, "Define if EVM rpc metrics server should be enabled")
cmd.Flags().Int32(srvflags.JSONRPCFeeHistoryCap, config.DefaultFeeHistoryCap, "Sets a max fee history depth")
cmd.Flags().Bool(srvflags.JSONRPCEnableUnsafeEndpoints, false, "Enable eth_sendTransaction, eth_sign, eth_signTypedData")

cmd.Flags().String(srvflags.EVMTracer, config.DefaultEVMTracer, "the EVM tracer type to collect execution traces from the EVM transaction execution (json|struct|access_list|markdown)") //nolint:lll
cmd.Flags().Uint64(srvflags.EVMMaxTxGasWanted, config.DefaultMaxTxGasWanted, "the gas wanted for each eth tx returned in ante handler in check tx mode") //nolint:lll
Expand Down
24 changes: 23 additions & 1 deletion tests/rpc/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestMain(m *testing.M) {
}

if HOST == "" {
HOST = "http://localhost:8545"
HOST = "http://localhost:8547"
}

var err error
Expand Down Expand Up @@ -347,6 +347,28 @@ func TestEth_IncompleteSendTransaction(t *testing.T) {
require.NotEqual(t, err.Error(), "method handler crashed", "no from field dealt with incorrectly")
}

func TestEth_UnsafeEndpoints(t *testing.T) {
// eth_sendTransaction
gasPrice := GetGasPrice(t)
param := make([]map[string]string, 1)
param[0] = make(map[string]string)
param[0]["from"] = "0x1122334455667788990011223344556677889900"
param[0]["to"] = "0x1122334455667788990011223344556677889900"
param[0]["value"] = "0x1"
param[0]["gasPrice"] = gasPrice
_, err := callWithError("eth_sendTransaction", param)

// require well-formatted error (should not be "method handler crashed")
require.Error(t, err)
require.Equal(t, "eth_sendTransaction not enabled", err.Error(), "unsafe endpoint still enabled")

// eth_sign
signParam := []string{"0x9b2055d370f73ec7d8a03e965129118dc8f5bf83", "0xdeadbeaf"}
_, err = callWithError("eth_sign", signParam)
require.Error(t, err)
require.Equal(t, "eth_sign not enabled", err.Error(), "unsafe endpoint still enabled")
}

func TestEth_GetFilterChanges_NoTopics(t *testing.T) {
rpcRes := call(t, "eth_blockNumber", []string{})

Expand Down
2 changes: 1 addition & 1 deletion tests/rpc/ws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var (

func init() {
if HOST_WS == "" {
HOST_WS = "localhost:8542"
HOST_WS = "localhost:8548"
}

u := url.URL{Scheme: "ws", Host: HOST_WS, Path: ""}
Expand Down

0 comments on commit 3f0134e

Please sign in to comment.