diff --git a/accounts/manager.go b/accounts/manager.go index 96ca298fc5..af104fc634 100644 --- a/accounts/manager.go +++ b/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() { diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 3d96de3128..28fbc4acdb 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,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) + } + } +} 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/internal/ethapi/api.go b/internal/ethapi/api.go index b6f01b7539..034d6290fd 100644 --- a/internal/ethapi/api.go +++ b/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 { diff --git a/node/config.go b/node/config.go index 2f871e478b..347320e32e 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"` @@ -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 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/rpc/http.go b/rpc/http.go index 518b3b874f..bc6376dcf5 100644 --- a/rpc/http.go +++ b/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) } diff --git a/signer/core/api.go b/signer/core/api.go index 184b903103..6a11c881ed 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(false, backends...) } // MetadataFromContext extracts Metadata from a given context.Context