cmd, accounts, internal, node, rpc, signer: insecure unlock protect

accounts/manager.go

@@ -24,9 +24,14 @@ import (
 	"github.com/ethereum/go-ethereum/event"
 )
 
+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                    // Account manager relative configs
 	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 +45,7 @@ type Manager struct {
 
 // NewManager creates a generic account manager to sign transaction via various
 // supported backends.
-func NewManager(backends ...Backend) *Manager {
+func NewManager(insecureUnlockAllowed bool, 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 +60,7 @@ func NewManager(backends ...Backend) *Manager {
 	}
 	// Assemble the account manager and return
 	am := &Manager{
+		config:   &Config{insecureUnlockAllowed},
 		backends: make(map[reflect.Type][]Backend),
 		updaters: subs,
 		updates:  updates,
@@ -77,6 +83,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() {

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 {
+	unlockAccounts(ctx, stack)
-		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)
-			}
-		}
-	}
 	// Register wallet event handlers to open and auto-derive wallets
 	events := make(chan accounts.WalletEvent, 16)
 	stack.AccountManager().Subscribe(events)
@@ -401,3 +394,30 @@ 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() {
+		log.Warn("Not safe to unlock account, please enable `allow-insecure-unlock` flag if necessary")
+		return
+	}
+	ks := stack.AccountManager().Backends(keystore.KeyStoreType)[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)
+		}
+	}
+}

cmd/geth/usage.go

@@ -148,6 +148,7 @@ var AppHelpFlagGroups = []flagGroup{
 			utils.UnlockedAccountFlag,
 			utils.PasswordFileFlag,
 			utils.ExternalSignerFlag,
+			utils.InsecureUnlockAllowedFlag,
 		},
 	},
 	{

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) {

internal/ethapi/api.go

@@ -317,7 +317,13 @@ 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 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 {
+		return false, errors.New("not safe to unlock account, please enable `allow-insecure-unlock` flag if necessary")
+	}
+
 	const max = uint64(time.Duration(math.MaxInt64) / time.Second)
 	var d time.Duration
 	if duration == nil {

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"`
 
@@ -497,7 +500,7 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
 		}
 	}
 
-	return accounts.NewManager(backends...), ephemeral, nil
+	return accounts.NewManager(conf.InsecureUnlockAllowed, backends...), ephemeral, nil
 }
 
 var warnLock sync.Mutex

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

rpc/http.go

@@ -256,6 +256,7 @@ 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)
 	}

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(false, backends...)
 }
 
 // MetadataFromContext extracts Metadata from a given context.Context