From a5f00018451e7f5eab156785d93de18244a27b66 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Fri, 15 Nov 2024 10:15:15 +0100 Subject: [PATCH] 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. --- accounts/keystore/account_cache.go | 3 +- accounts/keystore/testdata/dupes/1 | 1 - accounts/keystore/testdata/dupes/2 | 1 - accounts/keystore/testdata/dupes/foo | 1 - accounts/manager.go | 14 +-- cmd/geth/accountcmd.go | 113 ++++++++--------- cmd/geth/accountcmd_test.go | 176 +-------------------------- cmd/geth/main.go | 40 +----- cmd/utils/flags.go | 71 ++--------- cmd/utils/flags_legacy.go | 11 ++ cmd/utils/prompt.go | 15 --- cmd/utils/prompt_test.go | 76 ------------ internal/ethapi/api_test.go | 2 +- node/config.go | 2 +- node/node.go | 2 +- signer/core/api.go | 4 +- 16 files changed, 82 insertions(+), 450 deletions(-) delete mode 100644 accounts/keystore/testdata/dupes/1 delete mode 100644 accounts/keystore/testdata/dupes/2 delete mode 100644 accounts/keystore/testdata/dupes/foo delete mode 100644 cmd/utils/prompt_test.go diff --git a/accounts/keystore/account_cache.go b/accounts/keystore/account_cache.go index f7cf688e62..d3a98850c7 100644 --- a/accounts/keystore/account_cache.go +++ b/accounts/keystore/account_cache.go @@ -44,8 +44,7 @@ func byURL(a, b accounts.Account) int { return a.URL.Cmp(b.URL) } -// AmbiguousAddrError is returned when attempting to unlock -// an address for which more than one file exists. +// AmbiguousAddrError is returned when an address matches multiple files. type AmbiguousAddrError struct { Addr common.Address Matches []accounts.Account diff --git a/accounts/keystore/testdata/dupes/1 b/accounts/keystore/testdata/dupes/1 deleted file mode 100644 index a3868ec6d5..0000000000 --- a/accounts/keystore/testdata/dupes/1 +++ /dev/null @@ -1 +0,0 @@ -{"address":"f466859ead1932d743d622cb74fc058882e8648a","crypto":{"cipher":"aes-128-ctr","ciphertext":"cb664472deacb41a2e995fa7f96fe29ce744471deb8d146a0e43c7898c9ddd4d","cipherparams":{"iv":"dfd9ee70812add5f4b8f89d0811c9158"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":8,"p":16,"r":8,"salt":"0d6769bf016d45c479213990d6a08d938469c4adad8a02ce507b4a4e7b7739f1"},"mac":"bac9af994b15a45dd39669fc66f9aa8a3b9dd8c22cb16e4d8d7ea089d0f1a1a9"},"id":"472e8b3d-afb6-45b5-8111-72c89895099a","version":3} \ No newline at end of file diff --git a/accounts/keystore/testdata/dupes/2 b/accounts/keystore/testdata/dupes/2 deleted file mode 100644 index a3868ec6d5..0000000000 --- a/accounts/keystore/testdata/dupes/2 +++ /dev/null @@ -1 +0,0 @@ -{"address":"f466859ead1932d743d622cb74fc058882e8648a","crypto":{"cipher":"aes-128-ctr","ciphertext":"cb664472deacb41a2e995fa7f96fe29ce744471deb8d146a0e43c7898c9ddd4d","cipherparams":{"iv":"dfd9ee70812add5f4b8f89d0811c9158"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":8,"p":16,"r":8,"salt":"0d6769bf016d45c479213990d6a08d938469c4adad8a02ce507b4a4e7b7739f1"},"mac":"bac9af994b15a45dd39669fc66f9aa8a3b9dd8c22cb16e4d8d7ea089d0f1a1a9"},"id":"472e8b3d-afb6-45b5-8111-72c89895099a","version":3} \ No newline at end of file diff --git a/accounts/keystore/testdata/dupes/foo b/accounts/keystore/testdata/dupes/foo deleted file mode 100644 index c57060aea0..0000000000 --- a/accounts/keystore/testdata/dupes/foo +++ /dev/null @@ -1 +0,0 @@ -{"address":"7ef5a6135f1fd6a02593eedc869c6d41d934aef8","crypto":{"cipher":"aes-128-ctr","ciphertext":"1d0839166e7a15b9c1333fc865d69858b22df26815ccf601b28219b6192974e1","cipherparams":{"iv":"8df6caa7ff1b00c4e871f002cb7921ed"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":8,"p":16,"r":8,"salt":"e5e6ef3f4ea695f496b643ebd3f75c0aa58ef4070e90c80c5d3fb0241bf1595c"},"mac":"6d16dfde774845e4585357f24bce530528bc69f4f84e1e22880d34fa45c273e5"},"id":"950077c7-71e3-4c44-a4a1-143919141ed4","version":3} \ No newline at end of file diff --git a/accounts/manager.go b/accounts/manager.go index cbe4f7c79d..ac21ecd985 100644 --- a/accounts/manager.go +++ b/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) { diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go index cc22684e0b..b564fa3b57 100644 --- a/cmd/geth/accountcmd.go +++ b/cmd/geth/accountcmd.go @@ -17,14 +17,16 @@ 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 +193,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) @@ -219,60 +221,22 @@ func accountList(ctx *cli.Context) error { return nil } -// tries unlocking the specified account a few times. -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) - } - for trials := 0; trials < 3; trials++ { - prompt := fmt.Sprintf("Unlocking account %s | Attempt %d/%d", address, trials+1, 3) - password := utils.GetPassPhraseWithList(prompt, false, i, passwords) - err = ks.Unlock(account, password) - if err == nil { - log.Info("Unlocked account", "address", account.Address.Hex()) - return account, password - } - if err, ok := err.(*keystore.AmbiguousAddrError); ok { - log.Info("Unlocked account", "address", account.Address.Hex()) - return ambiguousAddrRecovery(ks, err, password), password - } - if err != keystore.ErrDecrypt { - // No need to prompt again if the error is not decryption-related. - break - } - } - // All trials expended to unlock account, bail out - utils.Fatalf("Failed to unlock account %s (%v)", address, err) - - return accounts.Account{}, "" -} - -func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrError, auth string) accounts.Account { - fmt.Printf("Multiple key files exist for address %x:\n", err.Addr) - for _, a := range err.Matches { - fmt.Println(" ", a.URL) - } - fmt.Println("Testing your password against all of them...") - var match *accounts.Account - for i, a := range err.Matches { - if e := ks.Unlock(a, auth); e == nil { - match = &err.Matches[i] - break - } +// 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 } - if match == nil { - utils.Fatalf("None of the listed files could be unlocked.") - return accounts.Account{} + text, err := os.ReadFile(path) + if err != nil { + utils.Fatalf("Failed to read password file: %v", err) } - fmt.Printf("Your password unlocked %s\n", match.URL) - fmt.Println("In order to avoid this warning, you need to remove the following duplicate key files:") - for _, a := range err.Matches { - if a != *match { - fmt.Println(" ", a.URL) - } + lines := strings.Split(string(text), "\n") + if len(lines) == 0 { + return "", false } - return *match + // Sanitise DOS line endings. + return strings.TrimRight(lines[0], "\r"), true } // accountCreate creates a new account into the keystore defined by the CLI flags. @@ -292,8 +256,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 +289,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 +326,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 +354,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) } diff --git a/cmd/geth/accountcmd_test.go b/cmd/geth/accountcmd_test.go index 8416eb40ef..ab093b54ff 100644 --- a/cmd/geth/accountcmd_test.go +++ b/cmd/geth/accountcmd_test.go @@ -20,7 +20,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "testing" "github.com/cespare/cp" @@ -171,12 +170,12 @@ func TestAccountUpdate(t *testing.T) { "f466859ead1932d743d622cb74fc058882e8648a") defer geth.ExpectExit() geth.Expect(` -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 +Please give a NEW password. Do not forget this password. !! Unsupported terminal, password will be echoed. -Password: {{.InputLine "foobar"}} -Please give a new password. Do not forget this password. Password: {{.InputLine "foobar2"}} Repeat password: {{.InputLine "foobar2"}} +Please provide the OLD password for account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 +Password: {{.InputLine "foobar"}} `) } @@ -206,172 +205,3 @@ Password: {{.InputLine "wrong"}} Fatal: could not decrypt key with given password `) } - -func TestUnlockFlag(t *testing.T) { - t.Parallel() - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "console", "--exec", "loadScript('testdata/empty.js')") - geth.Expect(` -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 -!! Unsupported terminal, password will be echoed. -Password: {{.InputLine "foobar"}} -undefined -`) - geth.ExpectExit() - - wantMessages := []string{ - "Unlocked account", - "=0xf466859eAD1932D743d622CB74FC058882E8648A", - } - for _, m := range wantMessages { - if !strings.Contains(geth.StderrText(), m) { - t.Errorf("stderr text does not contain %q", m) - } - } -} - -func TestUnlockFlagWrongPassword(t *testing.T) { - t.Parallel() - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "console", "--exec", "loadScript('testdata/empty.js')") - - defer geth.ExpectExit() - geth.Expect(` -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 -!! Unsupported terminal, password will be echoed. -Password: {{.InputLine "wrong1"}} -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 2/3 -Password: {{.InputLine "wrong2"}} -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 3/3 -Password: {{.InputLine "wrong3"}} -Fatal: Failed to unlock account f466859ead1932d743d622cb74fc058882e8648a (could not decrypt key with given password) -`) -} - -// https://github.com/ethereum/go-ethereum/issues/1785 -func TestUnlockFlagMultiIndex(t *testing.T) { - t.Parallel() - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--unlock", "0,2", "console", "--exec", "loadScript('testdata/empty.js')") - - geth.Expect(` -Unlocking account 0 | Attempt 1/3 -!! Unsupported terminal, password will be echoed. -Password: {{.InputLine "foobar"}} -Unlocking account 2 | Attempt 1/3 -Password: {{.InputLine "foobar"}} -undefined -`) - geth.ExpectExit() - - wantMessages := []string{ - "Unlocked account", - "=0x7EF5A6135f1FD6a02593eEdC869c6D41D934aef8", - "=0x289d485D9771714CCe91D3393D764E1311907ACc", - } - for _, m := range wantMessages { - if !strings.Contains(geth.StderrText(), m) { - t.Errorf("stderr text does not contain %q", m) - } - } -} - -func TestUnlockFlagPasswordFile(t *testing.T) { - t.Parallel() - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--password", "testdata/passwords.txt", "--unlock", "0,2", "console", "--exec", "loadScript('testdata/empty.js')") - - geth.Expect(` -undefined -`) - geth.ExpectExit() - - wantMessages := []string{ - "Unlocked account", - "=0x7EF5A6135f1FD6a02593eEdC869c6D41D934aef8", - "=0x289d485D9771714CCe91D3393D764E1311907ACc", - } - for _, m := range wantMessages { - if !strings.Contains(geth.StderrText(), m) { - t.Errorf("stderr text does not contain %q", m) - } - } -} - -func TestUnlockFlagPasswordFileWrongPassword(t *testing.T) { - t.Parallel() - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--password", - "testdata/wrong-passwords.txt", "--unlock", "0,2") - defer geth.ExpectExit() - geth.Expect(` -Fatal: Failed to unlock account 0 (could not decrypt key with given password) -`) -} - -func TestUnlockFlagAmbiguous(t *testing.T) { - t.Parallel() - store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes") - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--keystore", - store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", - "console", "--exec", "loadScript('testdata/empty.js')") - defer geth.ExpectExit() - - // Helper for the expect template, returns absolute keystore path. - geth.SetTemplateFunc("keypath", func(file string) string { - abs, _ := filepath.Abs(filepath.Join(store, file)) - return abs - }) - geth.Expect(` -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 -!! Unsupported terminal, password will be echoed. -Password: {{.InputLine "foobar"}} -Multiple key files exist for address f466859ead1932d743d622cb74fc058882e8648a: - keystore://{{keypath "1"}} - keystore://{{keypath "2"}} -Testing your password against all of them... -Your password unlocked keystore://{{keypath "1"}} -In order to avoid this warning, you need to remove the following duplicate key files: - keystore://{{keypath "2"}} -undefined -`) - geth.ExpectExit() - - wantMessages := []string{ - "Unlocked account", - "=0xf466859eAD1932D743d622CB74FC058882E8648A", - } - for _, m := range wantMessages { - if !strings.Contains(geth.StderrText(), m) { - t.Errorf("stderr text does not contain %q", m) - } - } -} - -func TestUnlockFlagAmbiguousWrongPassword(t *testing.T) { - t.Parallel() - store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes") - geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t), - "--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--keystore", - store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a") - - defer geth.ExpectExit() - - // Helper for the expect template, returns absolute keystore path. - geth.SetTemplateFunc("keypath", func(file string) string { - abs, _ := filepath.Abs(filepath.Join(store, file)) - return abs - }) - geth.Expect(` -Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 -!! Unsupported terminal, password will be echoed. -Password: {{.InputLine "wrong"}} -Multiple key files exist for address f466859ead1932d743d622cb74fc058882e8648a: - keystore://{{keypath "1"}} - keystore://{{keypath "2"}} -Testing your password against all of them... -Fatal: None of the listed files could be unlocked. -`) - geth.ExpectExit() -} diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 10d4052737..1527e180fb 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -23,11 +23,9 @@ import ( "slices" "sort" "strconv" - "strings" "time" "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/console/prompt" @@ -353,14 +351,14 @@ func geth(ctx *cli.Context) error { } // startNode boots up the system node and all registered protocols, after which -// it unlocks any requested accounts, and starts the RPC/IPC interfaces and the -// miner. +// it starts the RPC/IPC interfaces and the miner. func startNode(ctx *cli.Context, stack *node.Node, isConsole bool) { // Start up the node itself utils.StartNode(ctx, stack, isConsole) - // Unlock any account specifically requested - unlockAccounts(ctx, stack) + if ctx.IsSet(utils.UnlockedAccountFlag.Name) { + log.Warn(`The "unlock" flag has been deprecated and has no effect`) + } // Register wallet event handlers to open and auto-derive wallets events := make(chan accounts.WalletEvent, 16) @@ -427,33 +425,3 @@ func startNode(ctx *cli.Context, stack *node.Node, isConsole bool) { }() } } - -// unlockAccounts unlocks any account specifically requested. -func unlockAccounts(ctx *cli.Context, stack *node.Node) { - var unlocks []string - inputs := strings.Split(ctx.String(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!") - } - backends := stack.AccountManager().Backends(keystore.KeyStoreType) - if len(backends) == 0 { - log.Warn("Failed to unlock accounts, keystore is not available") - return - } - ks := backends[0].(*keystore.KeyStore) - passwords := utils.MakePasswordList(ctx) - for i, account := range unlocks { - unlockAccount(ks, account, i, passwords) - } -} diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 4eef66ebc6..e635dd89c3 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -499,12 +499,6 @@ var ( } // Account settings - UnlockedAccountFlag = &cli.StringFlag{ - Name: "unlock", - Usage: "Comma separated list of accounts to unlock", - Value: "", - Category: flags.AccountCategory, - } PasswordFileFlag = &cli.PathFlag{ Name: "password", Usage: "Password file to use for non-interactive password input", @@ -517,12 +511,6 @@ var ( Value: "", Category: flags.AccountCategory, } - InsecureUnlockAllowedFlag = &cli.BoolFlag{ - Name: "allow-insecure-unlock", - Usage: "Allow insecure account unlocking when account-related RPCs are exposed by http", - Category: flags.AccountCategory, - } - // EVM settings VMEnableDebugFlag = &cli.BoolFlag{ Name: "vmdebug", @@ -1268,31 +1256,6 @@ func MakeDatabaseHandles(max int) int { return int(raised / 2) // Leave half for networking and other stuff } -// MakeAddress converts an account specified directly as a hex encoded string or -// a key index in the key store to an internal account representation. -func MakeAddress(ks *keystore.KeyStore, account string) (accounts.Account, error) { - // If the specified account is a valid address, return it - if common.IsHexAddress(account) { - return accounts.Account{Address: common.HexToAddress(account)}, nil - } - // Otherwise try to interpret the account as a keystore index - index, err := strconv.Atoi(account) - if err != nil || index < 0 { - return accounts.Account{}, fmt.Errorf("invalid account address or index %q", account) - } - log.Warn("-------------------------------------------------------------------") - log.Warn("Referring to accounts by order in the keystore folder is dangerous!") - log.Warn("This functionality is deprecated and will be removed in the future!") - log.Warn("Please use explicit addresses! (can search via `geth account list`)") - log.Warn("-------------------------------------------------------------------") - - accs := ks.Accounts() - if len(accs) <= index { - return accounts.Account{}, fmt.Errorf("index %d higher than number of accounts %d", index, len(accs)) - } - return accs[index], nil -} - // setEtherbase retrieves the etherbase from the directly specified command line flags. func setEtherbase(ctx *cli.Context, cfg *ethconfig.Config) { if ctx.IsSet(MinerEtherbaseFlag.Name) { @@ -1313,24 +1276,6 @@ func setEtherbase(ctx *cli.Context, cfg *ethconfig.Config) { cfg.Miner.PendingFeeRecipient = common.BytesToAddress(b) } -// MakePasswordList reads password lines from the file specified by the global --password flag. -func MakePasswordList(ctx *cli.Context) []string { - path := ctx.Path(PasswordFileFlag.Name) - if path == "" { - return nil - } - text, err := os.ReadFile(path) - if err != nil { - Fatalf("Failed to read password file: %v", err) - } - lines := strings.Split(string(text), "\n") - // Sanitise DOS line endings. - for i := range lines { - lines[i] = strings.TrimRight(lines[i], "\r") - } - return lines -} - func SetP2PConfig(ctx *cli.Context, cfg *p2p.Config) { setNodeKey(ctx, cfg) setNAT(ctx, cfg) @@ -1412,7 +1357,7 @@ func SetNodeConfig(ctx *cli.Context, cfg *node.Config) { cfg.USB = ctx.Bool(USBFlag.Name) } if ctx.IsSet(InsecureUnlockAllowedFlag.Name) { - cfg.InsecureUnlockAllowed = ctx.Bool(InsecureUnlockAllowedFlag.Name) + log.Warn(fmt.Sprintf("Option %q is deprecated and has no effect", InsecureUnlockAllowedFlag.Name)) } if ctx.IsSet(DBEngineFlag.Name) { dbEngine := ctx.String(DBEngineFlag.Name) @@ -1805,13 +1750,15 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { passphrase string err error ) - if list := MakePasswordList(ctx); len(list) > 0 { - // Just take the first value. Although the function returns a possible multiple values and - // some usages iterate through them as attempts, that doesn't make sense in this setting, - // when we're definitely concerned with only one account. - passphrase = list[0] + if path := ctx.Path(PasswordFileFlag.Name); path != "" { + if text, err := os.ReadFile(path); err != nil { + Fatalf("Failed to read password file: %v", err) + } else { + if lines := strings.Split(string(text), "\n"); len(lines) > 0 { + passphrase = strings.TrimRight(lines[0], "\r") // Sanitise DOS line endings. + } + } } - // Unlock the developer account by local keystore. var ks *keystore.KeyStore if keystores := stack.AccountManager().Backends(keystore.KeyStoreType); len(keystores) > 0 { diff --git a/cmd/utils/flags_legacy.go b/cmd/utils/flags_legacy.go index 6209516e05..ff63dd5685 100644 --- a/cmd/utils/flags_legacy.go +++ b/cmd/utils/flags_legacy.go @@ -159,6 +159,17 @@ var ( Usage: "This used to enable the 'personal' namespace.", Category: flags.DeprecatedCategory, } + UnlockedAccountFlag = &cli.StringFlag{ + Name: "unlock", + Usage: "Comma separated list of accounts to unlock (deprecated)", + Value: "", + Category: flags.DeprecatedCategory, + } + InsecureUnlockAllowedFlag = &cli.BoolFlag{ + Name: "allow-insecure-unlock", + Usage: "Allow insecure account unlocking when account-related RPCs are exposed by http (deprecated)", + Category: flags.DeprecatedCategory, + } ) // showDeprecated displays deprecated flags that will be soon removed from the codebase. diff --git a/cmd/utils/prompt.go b/cmd/utils/prompt.go index f513e38188..9419b77010 100644 --- a/cmd/utils/prompt.go +++ b/cmd/utils/prompt.go @@ -45,18 +45,3 @@ func GetPassPhrase(text string, confirmation bool) string { } return password } - -// GetPassPhraseWithList retrieves the password associated with an account, either fetched -// from a list of preloaded passphrases, or requested interactively from the user. -func GetPassPhraseWithList(text string, confirmation bool, index int, passwords []string) string { - // If a list of passwords was supplied, retrieve from them - if len(passwords) > 0 { - if index < len(passwords) { - return passwords[index] - } - return passwords[len(passwords)-1] - } - // Otherwise prompt the user for the password - password := GetPassPhrase(text, confirmation) - return password -} diff --git a/cmd/utils/prompt_test.go b/cmd/utils/prompt_test.go deleted file mode 100644 index 236353a7cc..0000000000 --- a/cmd/utils/prompt_test.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2020 The go-ethereum Authors -// This file is part of go-ethereum. -// -// go-ethereum is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// go-ethereum is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with go-ethereum. If not, see . - -// Package utils contains internal helper functions for go-ethereum commands. -package utils - -import ( - "testing" -) - -func TestGetPassPhraseWithList(t *testing.T) { - t.Parallel() - type args struct { - text string - confirmation bool - index int - passwords []string - } - tests := []struct { - name string - args args - want string - }{ - { - "test1", - args{ - "text1", - false, - 0, - []string{"zero", "one", "two"}, - }, - "zero", - }, - { - "test2", - args{ - "text2", - false, - 5, - []string{"zero", "one", "two"}, - }, - "two", - }, - { - "test3", - args{ - "text3", - true, - 1, - []string{"zero", "one", "two"}, - }, - "one", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - if got := GetPassPhraseWithList(tt.args.text, tt.args.confirmation, tt.args.index, tt.args.passwords); got != tt.want { - t.Errorf("GetPassPhraseWithList() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 758638cb16..f570c5dc4c 100644 --- a/internal/ethapi/api_test.go +++ b/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") ) diff --git a/node/config.go b/node/config.go index 949db887e4..dc436876cc 100644 --- a/node/config.go +++ b/node/config.go @@ -83,7 +83,7 @@ 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 is a deprecated option to allow users to accounts in unsafe http environment. InsecureUnlockAllowed bool `toml:",omitempty"` // NoUSB disables hardware wallet monitoring and connectivity. diff --git a/node/node.go b/node/node.go index 92c0c35607..ec7382e725 100644 --- a/node/node.go +++ b/node/node.go @@ -130,7 +130,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) // Initialize the p2p server. This creates the node key and discovery databases. node.server.Config.PrivateKey = node.config.NodeKey() diff --git a/signer/core/api.go b/signer/core/api.go index 23ddcd0a20..def2d6041f 100644 --- a/signer/core/api.go +++ b/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