cmd/geth: remove unlock commandline flag (#30737)

This is one further step towards removing account management from
`geth`. This PR deprecates the flag `unlock`, and makes the flag moot:
unlock via geth is no longer possible.

Author: holiman

accounts/manager.go

@@ -29,12 +29,9 @@ import (
 // the manager will buffer in its channel.
 const managerSubBufferSize = 50
 
-// 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.
+// Config is a legacy struct which is not used
 type Config struct {
-	InsecureUnlockAllowed bool // Whether account unlocking in insecure environment is allowed
+	InsecureUnlockAllowed bool // Unused legacy-parameter
 }
 
 // newBackendEvent lets the manager know it should
@@ -47,7 +44,6 @@ type newBackendEvent struct {
 // 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
@@ -78,7 +74,6 @@ func NewManager(config *Config, backends ...Backend) *Manager {
 	}
 	// Assemble the account manager and return
 	am := &Manager{
-		config:      config,
 		backends:    make(map[reflect.Type][]Backend),
 		updaters:    subs,
 		updates:     updates,
@@ -106,11 +101,6 @@ func (am *Manager) Close() error {
 	return <-errc
 }
 
-// Config returns the configuration of account manager.
-func (am *Manager) Config() *Config {
-	return am.config
-}
-
 // AddBackend starts the tracking of an additional backend for wallet updates.
 // cmd/geth assumes once this func returns the backends have been already integrated.
 func (am *Manager) AddBackend(backend Backend) {

cmd/geth/accountcmd.go

@@ -17,12 +17,15 @@
 package main
 
 import (
+	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/cmd/utils"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/urfave/cli/v2"
@@ -191,7 +194,7 @@ nodes.
 // makeAccountManager creates an account manager with backends
 func makeAccountManager(ctx *cli.Context) *accounts.Manager {
 	cfg := loadBaseConfig(ctx)
-	am := accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: cfg.Node.InsecureUnlockAllowed})
+	am := accounts.NewManager(nil)
 	keydir, isEphemeral, err := cfg.Node.GetKeyStoreDir()
 	if err != nil {
 		utils.Fatalf("Failed to get the keystore directory: %v", err)
@@ -275,6 +278,24 @@ func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrErr
 	return *match
 }
 
+// readPasswordFromFile reads the first line of the given file, trims line endings,
+// and returns the password and whether the reading was successful.
+func readPasswordFromFile(path string) (string, bool) {
+	if path == "" {
+		return "", false
+	}
+	text, err := os.ReadFile(path)
+	if err != nil {
+		utils.Fatalf("Failed to read password file: %v", err)
+	}
+	lines := strings.Split(string(text), "\n")
+	if len(lines) == 0 {
+		return "", false
+	}
+	// Sanitise DOS line endings.
+	return strings.TrimRight(lines[0], "\r"), true
+}
+
 // accountCreate creates a new account into the keystore defined by the CLI flags.
 func accountCreate(ctx *cli.Context) error {
 	cfg := loadBaseConfig(ctx)
@@ -292,8 +313,10 @@ func accountCreate(ctx *cli.Context) error {
 		scryptP = keystore.LightScryptP
 	}
 
-	password := utils.GetPassPhraseWithList("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
-
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true)
+	}
 	account, err := keystore.StoreKey(keydir, password, scryptN, scryptP)
 
 	if err != nil {
@@ -323,10 +346,23 @@ func accountUpdate(ctx *cli.Context) error {
 	ks := backends[0].(*keystore.KeyStore)
 
 	for _, addr := range ctx.Args().Slice() {
-		account, oldPassword := unlockAccount(ks, addr, 0, nil)
-		newPassword := utils.GetPassPhraseWithList("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)
+		if !common.IsHexAddress(addr) {
+			return errors.New("address must be specified in hexadecimal form")
+		}
+		account := accounts.Account{Address: common.HexToAddress(addr)}
+		newPassword := utils.GetPassPhrase("Please give a NEW password. Do not forget this password.", true)
+		updateFn := func(attempt int) error {
+			prompt := fmt.Sprintf("Please provide the OLD password for account %s | Attempt %d/%d", addr, attempt+1, 3)
+			password := utils.GetPassPhrase(prompt, false)
+			return ks.Update(account, password, newPassword)
+		}
+		// let user attempt unlock thrice.
+		err := updateFn(0)
+		for attempts := 1; attempts < 3 && errors.Is(err, keystore.ErrDecrypt); attempts++ {
+			err = updateFn(attempts)
+		}
+		if err != nil {
+			return fmt.Errorf("could not update account: %w", err)
 		}
 	}
 	return nil
@@ -347,10 +383,12 @@ func importWallet(ctx *cli.Context) error {
 	if len(backends) == 0 {
 		utils.Fatalf("Keystore is not available")
 	}
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("", false)
+	}
 	ks := backends[0].(*keystore.KeyStore)
-	passphrase := utils.GetPassPhraseWithList("", false, 0, utils.MakePasswordList(ctx))
-
-	acct, err := ks.ImportPreSaleKey(keyJSON, passphrase)
+	acct, err := ks.ImportPreSaleKey(keyJSON, password)
 	if err != nil {
 		utils.Fatalf("%v", err)
 	}
@@ -373,9 +411,11 @@ func accountImport(ctx *cli.Context) error {
 		utils.Fatalf("Keystore is not available")
 	}
 	ks := backends[0].(*keystore.KeyStore)
-	passphrase := utils.GetPassPhraseWithList("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
-
-	acct, err := ks.ImportECDSA(key, passphrase)
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true)
+	}
+	acct, err := ks.ImportECDSA(key, password)
 	if err != nil {
 		utils.Fatalf("Could not create the account: %v", err)
 	}

internal/ethapi/api_test.go

@@ -416,7 +416,7 @@ func allBlobTxs(addr common.Address, config *params.ChainConfig) []txData {
 func newTestAccountManager(t *testing.T) (*accounts.Manager, accounts.Account) {
 	var (
 		dir        = t.TempDir()
-		am         = accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: true})
+		am         = accounts.NewManager(nil)
 		b          = keystore.NewKeyStore(dir, 2, 1)
 		testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
 	)

node/node.go

@@ -133,7 +133,7 @@ func New(conf *Config) (*Node, error) {
 	node.keyDirTemp = isEphem
 	// Creates an empty AccountManager with no backends. Callers (e.g. cmd/geth)
 	// are required to add the backends later on.
-	node.accman = accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: conf.InsecureUnlockAllowed})
+	node.accman = accounts.NewManager(nil)
 
 	// Create an empty antimev keystore with only file path.
 	antimevKeyDir, err := conf.GetAntiMEVKeyStorePath()

signer/core/api.go

@@ -184,9 +184,7 @@ func StartClefAccountManager(ksLocation string, nousb, lightKDF bool, scpath str
 			}
 		}
 	}
-
-	// Clef doesn't allow insecure http account unlock.
-	return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: false}, backends...)
+	return accounts.NewManager(nil, backends...)
 }
 
 // MetadataFromContext extracts Metadata from a given context.Context