diff --git a/accounts/manager.go b/accounts/manager.go index af104fc634..a41b63c438 100644 --- a/accounts/manager.go +++ b/accounts/manager.go @@ -45,7 +45,7 @@ type Manager struct { // NewManager creates a generic account manager to sign transaction via various // supported backends. -func NewManager(insecureUnlockAllowed bool, backends ...Backend) *Manager { +func NewManager(config *Config, backends ...Backend) *Manager { // Retrieve the initial list of wallets from the backends and sort by URL var wallets []Wallet for _, backend := range backends { @@ -60,7 +60,7 @@ func NewManager(insecureUnlockAllowed bool, backends ...Backend) *Manager { } // Assemble the account manager and return am := &Manager{ - config: &Config{insecureUnlockAllowed}, + config: config, backends: make(map[reflect.Type][]Backend), updaters: subs, updates: updates, diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 28fbc4acdb..d3c60838da 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -397,18 +397,9 @@ func startNode(ctx *cli.Context, stack *node.Node) { // unlockAccounts unlocks any account specifically requested. func unlockAccounts(ctx *cli.Context, stack *node.Node) { - // personalExposed checks whether personal API is exposed by http. - personalExposed := func() bool { - for _, module := range stack.Config().HTTPModules { - if module == "personal" { - return true - } - } - return false - } - // If insecure account unlocking is not allowed and account-related APIs are exposed by http, - // print warning log to user and skip unlocking. - if !stack.Config().InsecureUnlockAllowed && personalExposed() { + // If insecure account unlocking is not allowed if node's APIs are exposed to external. + // Print warning log to user and skip unlocking. + if !stack.Config().InsecureUnlockAllowed && stack.Config().ExtRPCEnabled() { log.Warn("Not safe to unlock account, please enable `allow-insecure-unlock` flag if necessary") return } diff --git a/eth/api_backend.go b/eth/api_backend.go index 7e90f80101..29ce19e28f 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -38,8 +38,9 @@ import ( // EthAPIBackend implements ethapi.Backend for full nodes type EthAPIBackend struct { - eth *Ethereum - gpo *gasprice.Oracle + extRPCEnabled bool + eth *Ethereum + gpo *gasprice.Oracle } // ChainConfig returns the active chain configuration. @@ -213,6 +214,10 @@ func (b *EthAPIBackend) AccountManager() *accounts.Manager { return b.eth.AccountManager() } +func (b *EthAPIBackend) ExtRPCEnabled() bool { + return b.extRPCEnabled +} + func (b *EthAPIBackend) BloomStatus() (uint64, uint64) { sections, _, _ := b.eth.bloomIndexer.Sections() return params.BloomBitsBlocks, sections diff --git a/eth/backend.go b/eth/backend.go index af963fa495..b13cb10289 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -197,7 +197,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { eth.miner = miner.New(eth, chainConfig, eth.EventMux(), eth.engine, config.MinerRecommit, config.MinerGasFloor, config.MinerGasCeil, eth.isLocalBlock) eth.miner.SetExtra(makeExtraData(config.MinerExtraData)) - eth.APIBackend = &EthAPIBackend{eth, nil} + eth.APIBackend = &EthAPIBackend{ctx.ExtRPCEnabled(), eth, nil} gpoParams := config.GPO if gpoParams.Default == nil { gpoParams.Default = config.MinerGasPrice diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 034d6290fd..178322623a 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -318,9 +318,10 @@ func (s *PrivateAccountAPI) ImportRawKey(privkey string, password string) (commo // the given password for duration seconds. If duration is nil it will use a // default of 300 seconds. It returns an indication if the account was unlocked. func (s *PrivateAccountAPI) UnlockAccount(ctx context.Context, addr common.Address, password string, duration *uint64) (bool, error) { - // When the API is exposed by HTTP, unless the user explicitly specifies to - // allow the insecure account unlocking, otherwise it is disabled. - if isHttp, ok := ctx.Value("http").(bool); ok && isHttp && !s.b.AccountManager().Config().InsecureUnlockAllowed { + // When the API is exposed by external RPC(http, ws etc), unless the user + // explicitly specifies to allow the insecure account unlocking, otherwise + // it is disabled. + if s.b.ExtRPCEnabled() && !s.b.AccountManager().Config().InsecureUnlockAllowed { return false, errors.New("not safe to unlock account, please enable `allow-insecure-unlock` flag if necessary") } diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index e23ee03b15..e88207f87d 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -44,6 +44,7 @@ type Backend interface { ChainDb() ethdb.Database EventMux() *event.TypeMux AccountManager() *accounts.Manager + ExtRPCEnabled() bool // BlockChain API SetHead(number uint64) diff --git a/les/api_backend.go b/les/api_backend.go index 7531396235..8b03979a29 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -39,8 +39,9 @@ import ( ) type LesApiBackend struct { - eth *LightEthereum - gpo *gasprice.Oracle + extRPCEnabled bool + eth *LightEthereum + gpo *gasprice.Oracle } func (b *LesApiBackend) ChainConfig() *params.ChainConfig { @@ -187,6 +188,10 @@ func (b *LesApiBackend) AccountManager() *accounts.Manager { return b.eth.accountManager } +func (b *LesApiBackend) ExtRPCEnabled() bool { + return b.extRPCEnabled +} + func (b *LesApiBackend) BloomStatus() (uint64, uint64) { if b.eth.bloomIndexer == nil { return 0, 0 diff --git a/les/backend.go b/les/backend.go index f0f8a6a6ec..944e7695d2 100644 --- a/les/backend.go +++ b/les/backend.go @@ -166,7 +166,7 @@ func New(ctx *node.ServiceContext, config *eth.Config) (*LightEthereum, error) { log.Warn("Ultra light client is enabled", "trustedNodes", len(leth.protocolManager.ulc.trustedKeys), "minTrustedFraction", leth.protocolManager.ulc.minTrustedFraction) leth.blockchain.DisableCheckFreq() } - leth.ApiBackend = &LesApiBackend{leth, nil} + leth.ApiBackend = &LesApiBackend{ctx.ExtRPCEnabled(), leth, nil} gpoParams := config.GPO if gpoParams.Default == nil { diff --git a/node/config.go b/node/config.go index 347320e32e..46876c1578 100644 --- a/node/config.go +++ b/node/config.go @@ -109,29 +109,6 @@ type Config struct { // for ephemeral nodes). HTTPPort int `toml:",omitempty"` - // GraphQLHost is the host interface on which to start the GraphQL server. If this - // field is empty, no GraphQL API endpoint will be started. - GraphQLHost string `toml:",omitempty"` - - // GraphQLPort is the TCP port number on which to start the GraphQL server. The - // default zero value is/ valid and will pick a port number randomly (useful - // for ephemeral nodes). - GraphQLPort int `toml:",omitempty"` - - // GraphQLCors is the Cross-Origin Resource Sharing header to send to requesting - // clients. Please be aware that CORS is a browser enforced security, it's fully - // useless for custom HTTP clients. - GraphQLCors []string `toml:",omitempty"` - - // GraphQLVirtualHosts is the list of virtual hostnames which are allowed on incoming requests. - // This is by default {'localhost'}. Using this prevents attacks like - // DNS rebinding, which bypasses SOP by simply masquerading as being within the same - // origin. These attacks do not utilize CORS, since they are not cross-domain. - // By explicitly checking the Host-header, the server will not allow requests - // made against the server with a malicious host domain. - // Requests using ip address directly are not affected - GraphQLVirtualHosts []string `toml:",omitempty"` - // HTTPCors is the Cross-Origin Resource Sharing header to send to requesting // clients. Please be aware that CORS is a browser enforced security, it's fully // useless for custom HTTP clients. @@ -181,6 +158,29 @@ type Config struct { // private APIs to untrusted users is a major security risk. WSExposeAll bool `toml:",omitempty"` + // GraphQLHost is the host interface on which to start the GraphQL server. If this + // field is empty, no GraphQL API endpoint will be started. + GraphQLHost string `toml:",omitempty"` + + // GraphQLPort is the TCP port number on which to start the GraphQL server. The + // default zero value is/ valid and will pick a port number randomly (useful + // for ephemeral nodes). + GraphQLPort int `toml:",omitempty"` + + // GraphQLCors is the Cross-Origin Resource Sharing header to send to requesting + // clients. Please be aware that CORS is a browser enforced security, it's fully + // useless for custom HTTP clients. + GraphQLCors []string `toml:",omitempty"` + + // GraphQLVirtualHosts is the list of virtual hostnames which are allowed on incoming requests. + // This is by default {'localhost'}. Using this prevents attacks like + // DNS rebinding, which bypasses SOP by simply masquerading as being within the same + // origin. These attacks do not utilize CORS, since they are not cross-domain. + // By explicitly checking the Host-header, the server will not allow requests + // made against the server with a malicious host domain. + // Requests using ip address directly are not affected + GraphQLVirtualHosts []string `toml:",omitempty"` + // Logger is a custom logger to use with the p2p.Server. Logger log.Logger `toml:",omitempty"` @@ -273,6 +273,12 @@ func DefaultWSEndpoint() string { return config.WSEndpoint() } +// ExtRPCEnabled returns the indicator whether node enables the external +// RPC(http, ws or graphql). +func (c *Config) ExtRPCEnabled() bool { + return c.HTTPHost != "" || c.WSHost != "" || c.GraphQLHost != "" +} + // NodeName returns the devp2p node identifier. func (c *Config) NodeName() string { name := c.name() @@ -500,7 +506,7 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) { } } - return accounts.NewManager(conf.InsecureUnlockAllowed, backends...), ephemeral, nil + return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: conf.InsecureUnlockAllowed}, backends...), ephemeral, nil } var warnLock sync.Mutex diff --git a/node/service.go b/node/service.go index 4f6cb66769..24f8097435 100644 --- a/node/service.go +++ b/node/service.go @@ -68,6 +68,12 @@ func (ctx *ServiceContext) Service(service interface{}) error { return ErrServiceUnknown } +// ExtRPCEnabled returns the indicator whether node enables the external +// RPC(http, ws or graphql). +func (ctx *ServiceContext) ExtRPCEnabled() bool { + return ctx.config.ExtRPCEnabled() +} + // ServiceConstructor is the function signature of the constructors needed to be // registered for service instantiation. type ServiceConstructor func(ctx *ServiceContext) (Service, error) diff --git a/rpc/http.go b/rpc/http.go index bc6376dcf5..518b3b874f 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -256,7 +256,6 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { ctx = context.WithValue(ctx, "remote", r.RemoteAddr) ctx = context.WithValue(ctx, "scheme", r.Proto) ctx = context.WithValue(ctx, "local", r.Host) - ctx = context.WithValue(ctx, "http", true) if ua := r.Header.Get("User-Agent"); ua != "" { ctx = context.WithValue(ctx, "User-Agent", ua) } diff --git a/signer/core/api.go b/signer/core/api.go index 6a11c881ed..9da6ee2a23 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -140,7 +140,7 @@ func StartClefAccountManager(ksLocation string, nousb, lightKDF bool) *accounts. } } // Clef doesn't allow insecure http account unlock. - return accounts.NewManager(false, backends...) + return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: false}, backends...) } // MetadataFromContext extracts Metadata from a given context.Context