diff --git a/accounts/manager.go b/accounts/manager.go index 96ca298fc5..3cf3422e77 100644 --- a/accounts/manager.go +++ b/accounts/manager.go @@ -24,9 +24,18 @@ import ( "github.com/ethereum/go-ethereum/event" ) +// Config contains the settings of the global account manager. +// +// TODO(rjl493456442, karalabe, holiman): Get rid of this when account management +// is removed in favor of Clef. +type Config struct { + InsecureUnlockAllowed bool // Whether account unlocking in insecure environment is allowed +} + // Manager is an overarching account manager that can communicate with various // backends for signing transactions. type Manager struct { + config *Config // Global account manager configurations backends map[reflect.Type][]Backend // Index of backends currently registered updaters []event.Subscription // Wallet update subscriptions for all backends updates chan WalletEvent // Subscription sink for backend wallet changes @@ -40,7 +49,7 @@ type Manager struct { // NewManager creates a generic account manager to sign transaction via various // supported backends. -func NewManager(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 { @@ -55,6 +64,7 @@ func NewManager(backends ...Backend) *Manager { } // Assemble the account manager and return am := &Manager{ + config: config, backends: make(map[reflect.Type][]Backend), updaters: subs, updates: updates, @@ -77,6 +87,11 @@ func (am *Manager) Close() error { return <-errc } +// Config returns the configuration of account manager. +func (am *Manager) Config() *Config { + return am.config +} + // update is the wallet event loop listening for notifications from the backends // and updating the cache of wallets. func (am *Manager) update() { diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go index 071be85397..940290899c 100644 --- a/cmd/geth/accountcmd.go +++ b/cmd/geth/accountcmd.go @@ -205,7 +205,7 @@ func accountList(ctx *cli.Context) error { } // tries unlocking the specified account a few times. -func unlockAccount(ctx *cli.Context, ks *keystore.KeyStore, address string, i int, passwords []string) (accounts.Account, string) { +func unlockAccount(ks *keystore.KeyStore, address string, i int, passwords []string) (accounts.Account, string) { account, err := utils.MakeAddress(ks, address) if err != nil { utils.Fatalf("Could not list accounts: %v", err) @@ -326,7 +326,7 @@ func accountUpdate(ctx *cli.Context) error { ks := stack.AccountManager().Backends(keystore.KeyStoreType)[0].(*keystore.KeyStore) for _, addr := range ctx.Args() { - account, oldPassword := unlockAccount(ctx, ks, addr, 0, nil) + account, oldPassword := unlockAccount(ks, addr, 0, nil) newPassword := getPassPhrase("Please give a new password. Do not forget this password.", true, 0, nil) if err := ks.Update(account, oldPassword, newPassword); err != nil { utils.Fatalf("Could not update the account: %v", err) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 3d96de3128..1963a1a7f8 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -57,6 +57,7 @@ var ( utils.IdentityFlag, utils.UnlockedAccountFlag, utils.PasswordFileFlag, + utils.InsecureUnlockAllowedFlag, utils.BootnodesFlag, utils.BootnodesV4Flag, utils.BootnodesV5Flag, @@ -298,16 +299,8 @@ func startNode(ctx *cli.Context, stack *node.Node) { utils.StartNode(stack) // Unlock any account specifically requested - if keystores := stack.AccountManager().Backends(keystore.KeyStoreType); len(keystores) > 0 { - ks := keystores[0].(*keystore.KeyStore) - passwords := utils.MakePasswordList(ctx) - unlocks := strings.Split(ctx.GlobalString(utils.UnlockedAccountFlag.Name), ",") - for i, account := range unlocks { - if trimmed := strings.TrimSpace(account); trimmed != "" { - unlockAccount(ctx, ks, trimmed, i, passwords) - } - } - } + unlockAccounts(ctx, stack) + // Register wallet event handlers to open and auto-derive wallets events := make(chan accounts.WalletEvent, 16) stack.AccountManager().Subscribe(events) @@ -401,3 +394,28 @@ func startNode(ctx *cli.Context, stack *node.Node) { } } } + +// unlockAccounts unlocks any account specifically requested. +func unlockAccounts(ctx *cli.Context, stack *node.Node) { + var unlocks []string + inputs := strings.Split(ctx.GlobalString(utils.UnlockedAccountFlag.Name), ",") + for _, input := range inputs { + if trimmed := strings.TrimSpace(input); trimmed != "" { + unlocks = append(unlocks, trimmed) + } + } + // Short circuit if there is no account to unlock. + if len(unlocks) == 0 { + return + } + // 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() { + utils.Fatalf("Account unlock with HTTP access is forbidden!") + } + ks := stack.AccountManager().Backends(keystore.KeyStoreType)[0].(*keystore.KeyStore) + passwords := utils.MakePasswordList(ctx) + for i, account := range unlocks { + unlockAccount(ks, account, i, passwords) + } +} diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index f1fb22f18e..6d039ba040 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -148,6 +148,7 @@ var AppHelpFlagGroups = []flagGroup{ utils.UnlockedAccountFlag, utils.PasswordFileFlag, utils.ExternalSignerFlag, + utils.InsecureUnlockAllowedFlag, }, }, { diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 2ce3a88014..f6e4288692 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -444,6 +444,10 @@ var ( Name: "vmdebug", Usage: "Record information useful for VM and contract debugging", } + InsecureUnlockAllowedFlag = cli.BoolFlag{ + Name: "allow-insecure-unlock", + Usage: "Allow insecure account unlocking when account-related RPCs are exposed by http", + } // Logging and debug settings EthStatsURLFlag = cli.StringFlag{ Name: "ethstats", @@ -1130,6 +1134,9 @@ func SetNodeConfig(ctx *cli.Context, cfg *node.Config) { if ctx.GlobalIsSet(NoUSBFlag.Name) { cfg.NoUSB = ctx.GlobalBool(NoUSBFlag.Name) } + if ctx.GlobalIsSet(InsecureUnlockAllowedFlag.Name) { + cfg.InsecureUnlockAllowed = ctx.GlobalBool(InsecureUnlockAllowedFlag.Name) + } } func setDataDir(ctx *cli.Context, cfg *node.Config) { 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 b6f01b7539..e5a8124b1f 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -317,7 +317,14 @@ func (s *PrivateAccountAPI) ImportRawKey(privkey string, password string) (commo // UnlockAccount will unlock the account associated with the given address with // 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(addr common.Address, password string, duration *uint64) (bool, error) { +func (s *PrivateAccountAPI) UnlockAccount(ctx context.Context, addr common.Address, password string, duration *uint64) (bool, error) { + // 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("account unlock with HTTP access is forbidden") + } + const max = uint64(time.Duration(math.MaxInt64) / time.Second) var d time.Duration if duration == nil { 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 2f871e478b..46876c1578 100644 --- a/node/config.go +++ b/node/config.go @@ -88,6 +88,9 @@ type Config struct { // scrypt KDF at the expense of security. UseLightweightKDF bool `toml:",omitempty"` + // InsecureUnlockAllowed allows user to unlock accounts in unsafe http environment. + InsecureUnlockAllowed bool `toml:",omitempty"` + // NoUSB disables hardware wallet monitoring and connectivity. NoUSB bool `toml:",omitempty"` @@ -106,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. @@ -178,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"` @@ -270,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() @@ -497,7 +506,7 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) { } } - return accounts.NewManager(backends...), ephemeral, nil + return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: conf.InsecureUnlockAllowed}, backends...), ephemeral, nil } var warnLock sync.Mutex diff --git a/node/node.go b/node/node.go index bd031bd0fe..f4c7d8c726 100644 --- a/node/node.go +++ b/node/node.go @@ -251,6 +251,11 @@ func (n *Node) Start() error { return nil } +// Config returns the configuration of node. +func (n *Node) Config() *Config { + return n.config +} + func (n *Node) openDataDir() error { if n.config.DataDir == "" { return nil // ephemeral 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/signer/core/api.go b/signer/core/api.go index 184b903103..9da6ee2a23 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -139,7 +139,8 @@ func StartClefAccountManager(ksLocation string, nousb, lightKDF bool) *accounts. log.Debug("Trezor support enabled") } } - return accounts.NewManager(backends...) + // Clef doesn't allow insecure http account unlock. + return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: false}, backends...) } // MetadataFromContext extracts Metadata from a given context.Context