Block some JSON-RPC methods completely

api/backend_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/status-im/status-go/node"
 	"github.com/status-im/status-go/params"
+	"github.com/status-im/status-go/rpc"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -335,4 +336,20 @@ func TestPrepareTxArgs(t *testing.T) {
 	}
 }
 
+func TestBlockedRPCMethods(t *testing.T) {
+	backend := NewStatusBackend()
+	err := backend.StartNode(&params.NodeConfig{})
+	require.NoError(t, err)
+	defer func() { require.NoError(t, backend.StopNode()) }()
+
+	for idx, m := range rpc.BlockedMethods() {
+		result := backend.CallRPC(fmt.Sprintf(
+			`{"jsonrpc":"2.0","method":"%s","params":[],"id":%d}`,
+			m,
+			idx+1,
+		))
+		assert.Contains(t, result, fmt.Sprintf(`{"code":-32700,"message":"%s"}`, rpc.ErrMethodNotFound))
+	}
+}
+
 // TODO(adam): add concurrent tests for: SendTransaction, ApproveSignRequest, DiscardSignRequest

rpc/client.go

@@ -20,6 +20,11 @@ const (
 	DefaultCallTimeout = time.Minute
 )
 
+// List of RPC client errors.
+var (
+	ErrMethodNotFound = fmt.Errorf("The method does not exist/is not available")
+)
+
 // Handler defines handler for RPC methods.
 type Handler func(context.Context, ...interface{}) (interface{}, error)
 
@@ -89,6 +94,10 @@ func (c *Client) Call(result interface{}, method string, args ...interface{}) er
 // It uses custom routing scheme for calls.
 // If there are any local handlers registered for this call, they will handle it.
 func (c *Client) CallContext(ctx context.Context, result interface{}, method string, args ...interface{}) error {
+	if c.router.routeBlocked(method) {
+		return ErrMethodNotFound
+	}
+
 	// check locally registered handlers first
 	if handler, ok := c.handler(method); ok {
 		return c.callMethod(ctx, result, handler, args...)
@@ -105,6 +114,10 @@ func (c *Client) CallContext(ctx context.Context, result interface{}, method str
 // handler itself.
 // Upstream calls routing will be used anyway.
 func (c *Client) CallContextIgnoringLocalHandlers(ctx context.Context, result interface{}, method string, args ...interface{}) error {
+	if c.router.routeBlocked(method) {
+		return ErrMethodNotFound
+	}
+
 	if c.router.routeRemote(method) {
 		return c.upstream.CallContext(ctx, result, method, args...)
 	}

rpc/client_test.go

@@ -0,0 +1,78 @@
+package rpc
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/status-im/status-go/params"
+
+	gethrpc "github.com/ethereum/go-ethereum/rpc"
+)
+
+func TestBlockedRoutesCall(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, `{
+			"id": 1,
+			"jsonrpc": "2.0",
+			"result": "0x234234e22b9ffc2387e18636e0534534a3d0c56b0243567432453264c16e78a2adc"
+		}`)
+	}))
+	defer ts.Close()
+
+	gethRPCClient, err := gethrpc.Dial(ts.URL)
+	require.NoError(t, err)
+
+	c, err := NewClient(gethRPCClient, params.UpstreamRPCConfig{Enabled: false, URL: ""})
+	require.NoError(t, err)
+
+	for _, m := range blockedMethods {
+		var (
+			result interface{}
+			err    error
+		)
+
+		err = c.Call(&result, m)
+		require.EqualError(t, err, ErrMethodNotFound.Error())
+		require.Nil(t, result)
+
+		err = c.CallContext(context.Background(), &result, m)
+		require.EqualError(t, err, ErrMethodNotFound.Error())
+		require.Nil(t, result)
+
+		err = c.CallContextIgnoringLocalHandlers(context.Background(), &result, m)
+		require.EqualError(t, err, ErrMethodNotFound.Error())
+		require.Nil(t, result)
+	}
+}
+
+func TestBlockedRoutesRawCall(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, `{
+			"id": 1,
+			"jsonrpc": "2.0",
+			"result": "0x234234e22b9ffc2387e18636e0534534a3d0c56b0243567432453264c16e78a2adc"
+		}`)
+	}))
+	defer ts.Close()
+
+	gethRPCClient, err := gethrpc.Dial(ts.URL)
+	require.NoError(t, err)
+
+	c, err := NewClient(gethRPCClient, params.UpstreamRPCConfig{Enabled: false, URL: ""})
+	require.NoError(t, err)
+
+	for _, m := range blockedMethods {
+		rawResult := c.CallRaw(fmt.Sprintf(`{
+			"jsonrpc": "2.0",
+			"id": 1,
+			"method": "%s",
+			"params": ["0xc862bf3cf4565d46abcbadaf4712a8940bfea729a91b9b0e338eab5166341ab5"]
+		}`, m))
+		require.Contains(t, rawResult, fmt.Sprintf(`{"code":-32700,"message":"%s"}`, ErrMethodNotFound))
+	}
+}

rpc/route.go

@@ -5,6 +5,7 @@ package rpc
 // Local node.
 type router struct {
 	methods         map[string]bool
+	blockedMethods  map[string]struct{}
 	upstreamEnabled bool
 }
 
@@ -12,6 +13,7 @@ type router struct {
 func newRouter(upstreamEnabled bool) *router {
 	r := &router{
 		methods:         make(map[string]bool),
+		blockedMethods:  make(map[string]struct{}),
 		upstreamEnabled: upstreamEnabled,
 	}
 
@@ -19,6 +21,10 @@ func newRouter(upstreamEnabled bool) *router {
 		r.methods[m] = true
 	}
 
+	for _, m := range blockedMethods {
+		r.blockedMethods[m] = struct{}{}
+	}
+
 	return r
 }
 
@@ -32,6 +38,23 @@ func (r *router) routeRemote(method string) bool {
 	return r.methods[method]
 }
 
+func (r *router) routeBlocked(method string) bool {
+	_, ok := r.blockedMethods[method]
+	return ok
+}
+
+// blockedMethods is a list of dangerous or having security implications JSON-RPC methods
+// that are not allowed to be called.
+var blockedMethods = [...]string{
+	"shh_getPrivateKey",
+}
+
+// BlockedMethods returns a list of methods that are not allowed to be called.
+// A copy of a slice is returned in order to prevent from changing it from outside.
+func BlockedMethods() []string {
+	return append([]string(nil), blockedMethods[:]...)
+}
+
 // remoteMethods contains methods that should be routed to
 // the upstream node; the rest is considered to be routed to
 // the local node.

rpc/route_test.go

@@ -34,3 +34,16 @@ func TestRouteWithoutUpstream(t *testing.T) {
 		require.False(t, router.routeRemote(method), "method "+method+" should routed to local")
 	}
 }
+
+func TestBlockedRoutes(t *testing.T) {
+	// Be explicit as any change to `blockedMethods`
+	// should be confirmed with a unit test fail.
+	expectedBlockedMethods := [...]string{"shh_getPrivateKey"}
+	require.Equal(t, expectedBlockedMethods, blockedMethods)
+	require.Equal(t, expectedBlockedMethods[:], BlockedMethods())
+
+	router := newRouter(false)
+	for _, method := range blockedMethods {
+		require.True(t, router.routeBlocked(method))
+	}
+}