From a3228ddf2ffaa4e8a71cfd2dbb621ba10c05fc7b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 28 Oct 2022 14:05:12 +0200 Subject: [PATCH 1/3] cmd/clef: add importraw feature to clef --- cmd/clef/main.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/cmd/clef/main.go b/cmd/clef/main.go index 188a1150000..e1e3ed1a252 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -23,8 +23,10 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" + "io/ioutil" "math/big" "os" "os/signal" @@ -273,6 +275,7 @@ func init() { setCredentialCommand, delCredentialCommand, newAccountCommand, + importRawCommand, gendocCommand, listAccountsCommand, listWalletsCommand, @@ -452,6 +455,79 @@ func removeCredential(ctx *cli.Context) error { return nil } +// accountImport imports a raw hexadecimal private key via CLI. +func accountImport(c *cli.Context) error { + if err := initialize(c); err != nil { + return err + } + // The importraw is meant for users using the CLI, since 'real' external + // UIs can use the UI-api instead. So we'll just use the native CLI UI here. + var ( + ui = core.NewCommandlineUI() + pwStorage storage.Storage = &storage.NoStorage{} + ksLoc = c.String(keystoreFlag.Name) + lightKdf = c.Bool(utils.LightKDFFlag.Name) + ) + if c.Args().Len() != 1 { + return errors.New(" must be given as first argument.") + } + var hexkey string + if data, err := ioutil.ReadFile(c.Args().First()); err != nil { + return err + } else { + hexkey = strings.TrimSpace(string(data)) + } + // This will be properly validated during import, but we do a brief sanity check + // prior to prompting for password + if b, err := hex.DecodeString(hexkey); err != nil { + return err + } else if len(b) != 32 { + return fmt.Errorf("Wrong-sized key, %d bits, need 256 bits", len(b)*8) + } + log.Info("Starting clef", "keystore", ksLoc, "light-kdf", lightKdf) + am := core.StartClefAccountManager(ksLoc, true, lightKdf, "") + // This gives is us access to the external API + apiImpl := core.NewSignerAPI(am, 0, true, ui, nil, false, pwStorage) + // This gives us access to the internal API + internalApi := core.NewUIServerAPI(apiImpl) + readPw := func(prompt string) (string, error) { + resp, err := ui.OnInputRequired(core.UserInputRequest{ + Title: "Password", + Prompt: prompt, + IsPassword: true, + }) + if err != nil { + return "", err + } + return resp.Text, nil + } + first, err := readPw("Please enter a password for the imported account") + if err != nil { + return err + } + second, err := readPw("Please repeat the password you just entered") + if err != nil { + return err + } + if first != second { + return errors.New("Passwords do not match") + } + acc, err := internalApi.ImportRawKey(hexkey, first) + if err != nil { + return err + } + ui.ShowInfo(fmt.Sprintf(`Key imported: + Address %v + Keystore file: %v + +The key is now encrypted; losing the password will result in permanently losing +access to the key and all associated funds! + +Make sure to backup keystore and passwords in a safe location.`, + acc.Address, acc.URL.Path)) + return nil +} + func initialize(c *cli.Context) error { // Set up the logger to print everything logOutput := os.Stdout From fb96ae795e4a80ccab0bba1c5d32310241960eed Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 28 Oct 2022 15:17:43 +0200 Subject: [PATCH 2/3] cmd/clef: lower default verbosity, fix lint nit --- cmd/clef/main.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/clef/main.go b/cmd/clef/main.go index e1e3ed1a252..3f41412231a 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -26,7 +26,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "math/big" "os" "os/signal" @@ -76,7 +75,7 @@ PURPOSE. See the GNU General Public License for more details. var ( logLevelFlag = &cli.IntFlag{ Name: "loglevel", - Value: 4, + Value: 3, Usage: "log level to emit to the screen", } advancedMode = &cli.BoolFlag{ @@ -472,7 +471,7 @@ func accountImport(c *cli.Context) error { return errors.New(" must be given as first argument.") } var hexkey string - if data, err := ioutil.ReadFile(c.Args().First()); err != nil { + if data, err := os.ReadFile(c.Args().First()); err != nil { return err } else { hexkey = strings.TrimSpace(string(data)) From 855774920dae461ad6270845a26a5c8d5368f0f3 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 2 Nov 2022 10:29:33 +0100 Subject: [PATCH 3/3] cmd/clef: address review concerns + testcases for clef binary --- cmd/clef/consolecmd_test.go | 117 +++++++++++++++++++++++++++ cmd/clef/main.go | 153 +++++++++++++++++------------------- cmd/clef/run_test.go | 109 +++++++++++++++++++++++++ 3 files changed, 300 insertions(+), 79 deletions(-) create mode 100644 cmd/clef/consolecmd_test.go create mode 100644 cmd/clef/run_test.go diff --git a/cmd/clef/consolecmd_test.go b/cmd/clef/consolecmd_test.go new file mode 100644 index 00000000000..283d7e8def3 --- /dev/null +++ b/cmd/clef/consolecmd_test.go @@ -0,0 +1,117 @@ +// Copyright 2022 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 main + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestImportRaw tests clef --importraw +func TestImportRaw(t *testing.T) { + keyPath := filepath.Join(os.TempDir(), fmt.Sprintf("%v-tempkey.test", t.Name())) + os.WriteFile(keyPath, []byte("0102030405060708090a0102030405060708090a0102030405060708090a0102"), 0777) + t.Cleanup(func() { os.Remove(keyPath) }) + + t.Parallel() + t.Run("happy-path", func(t *testing.T) { + // Run clef importraw + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "importraw", keyPath) + clef.input("myverylongpassword").input("myverylongpassword") + if out := string(clef.Output()); !strings.Contains(out, + "Key imported:\n Address 0x9160DC9105f7De5dC5E7f3d97ef11DA47269BdA6") { + t.Logf("Output\n%v", out) + t.Error("Failure") + } + }) + // tests clef --importraw with mismatched passwords. + t.Run("pw-mismatch", func(t *testing.T) { + // Run clef importraw + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "importraw", keyPath) + clef.input("myverylongpassword1").input("myverylongpassword2").WaitExit() + if have, want := clef.StderrText(), "Passwords do not match\n"; have != want { + t.Errorf("have %q, want %q", have, want) + } + }) + // tests clef --importraw with a too short password. + t.Run("short-pw", func(t *testing.T) { + // Run clef importraw + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "importraw", keyPath) + clef.input("shorty").input("shorty").WaitExit() + if have, want := clef.StderrText(), + "password requirements not met: password too short (<10 characters)\n"; have != want { + t.Errorf("have %q, want %q", have, want) + } + }) +} + +// TestListAccounts tests clef --list-accounts +func TestListAccounts(t *testing.T) { + keyPath := filepath.Join(os.TempDir(), fmt.Sprintf("%v-tempkey.test", t.Name())) + os.WriteFile(keyPath, []byte("0102030405060708090a0102030405060708090a0102030405060708090a0102"), 0777) + t.Cleanup(func() { os.Remove(keyPath) }) + + t.Parallel() + t.Run("no-accounts", func(t *testing.T) { + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "list-accounts") + if out := string(clef.Output()); !strings.Contains(out, "The keystore is empty.") { + t.Logf("Output\n%v", out) + t.Error("Failure") + } + }) + t.Run("one-account", func(t *testing.T) { + // First, we need to import + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "importraw", keyPath) + clef.input("myverylongpassword").input("myverylongpassword").WaitExit() + // Secondly, do a listing, using the same datadir + clef = runWithKeystore(t, clef.Datadir, "--suppress-bootwarn", "--lightkdf", "list-accounts") + if out := string(clef.Output()); !strings.Contains(out, "0x9160DC9105f7De5dC5E7f3d97ef11DA47269BdA6 (keystore:") { + t.Logf("Output\n%v", out) + t.Error("Failure") + } + }) +} + +// TestListWallets tests clef --list-wallets +func TestListWallets(t *testing.T) { + keyPath := filepath.Join(os.TempDir(), fmt.Sprintf("%v-tempkey.test", t.Name())) + os.WriteFile(keyPath, []byte("0102030405060708090a0102030405060708090a0102030405060708090a0102"), 0777) + t.Cleanup(func() { os.Remove(keyPath) }) + + t.Parallel() + t.Run("no-accounts", func(t *testing.T) { + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "list-wallets") + if out := string(clef.Output()); !strings.Contains(out, "There are no wallets.") { + t.Logf("Output\n%v", out) + t.Error("Failure") + } + }) + t.Run("one-account", func(t *testing.T) { + // First, we need to import + clef := runClef(t, "--suppress-bootwarn", "--lightkdf", "importraw", keyPath) + clef.input("myverylongpassword").input("myverylongpassword").WaitExit() + // Secondly, do a listing, using the same datadir + clef = runWithKeystore(t, clef.Datadir, "--suppress-bootwarn", "--lightkdf", "list-wallets") + if out := string(clef.Output()); !strings.Contains(out, "Account 0: 0x9160DC9105f7De5dC5E7f3d97ef11DA47269BdA6") { + t.Logf("Output\n%v", out) + t.Error("Failure") + } + }) +} diff --git a/cmd/clef/main.go b/cmd/clef/main.go index 3f41412231a..be7089ce449 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -239,6 +239,23 @@ The gendoc generates example structures of the json-rpc communication types. Description: ` Lists the wallets known to Clef. `} + importRawCommand = &cli.Command{ + Action: accountImport, + Name: "importraw", + Usage: "Import a hex-encoded private key.", + ArgsUsage: "", + Flags: []cli.Flag{ + logLevelFlag, + keystoreFlag, + utils.LightKDFFlag, + acceptFlag, + }, + Description: ` +Imports an unencrypted private key from and creates a new account. +Prints the address. +The keyfile is assumed to contain an unencrypted private key in hexadecimal format. +The account is saved in encrypted format, you are prompted for a password. +`} ) var app = flags.NewApp("Manage Ethereum account operations") @@ -380,9 +397,9 @@ func attestFile(ctx *cli.Context) error { return nil } -func initInternalApi(c *cli.Context) (*core.UIServerAPI, error) { +func initInternalApi(c *cli.Context) (*core.UIServerAPI, core.UIClientAPI, error) { if err := initialize(c); err != nil { - return nil, err + return nil, nil, err } var ( ui = core.NewCommandlineUI() @@ -393,7 +410,7 @@ func initInternalApi(c *cli.Context) (*core.UIServerAPI, error) { am := core.StartClefAccountManager(ksLoc, true, lightKdf, "") api := core.NewSignerAPI(am, 0, true, ui, nil, false, pwStorage) internalApi := core.NewUIServerAPI(api) - return internalApi, nil + return internalApi, ui, nil } func setCredential(ctx *cli.Context) error { @@ -454,79 +471,6 @@ func removeCredential(ctx *cli.Context) error { return nil } -// accountImport imports a raw hexadecimal private key via CLI. -func accountImport(c *cli.Context) error { - if err := initialize(c); err != nil { - return err - } - // The importraw is meant for users using the CLI, since 'real' external - // UIs can use the UI-api instead. So we'll just use the native CLI UI here. - var ( - ui = core.NewCommandlineUI() - pwStorage storage.Storage = &storage.NoStorage{} - ksLoc = c.String(keystoreFlag.Name) - lightKdf = c.Bool(utils.LightKDFFlag.Name) - ) - if c.Args().Len() != 1 { - return errors.New(" must be given as first argument.") - } - var hexkey string - if data, err := os.ReadFile(c.Args().First()); err != nil { - return err - } else { - hexkey = strings.TrimSpace(string(data)) - } - // This will be properly validated during import, but we do a brief sanity check - // prior to prompting for password - if b, err := hex.DecodeString(hexkey); err != nil { - return err - } else if len(b) != 32 { - return fmt.Errorf("Wrong-sized key, %d bits, need 256 bits", len(b)*8) - } - log.Info("Starting clef", "keystore", ksLoc, "light-kdf", lightKdf) - am := core.StartClefAccountManager(ksLoc, true, lightKdf, "") - // This gives is us access to the external API - apiImpl := core.NewSignerAPI(am, 0, true, ui, nil, false, pwStorage) - // This gives us access to the internal API - internalApi := core.NewUIServerAPI(apiImpl) - readPw := func(prompt string) (string, error) { - resp, err := ui.OnInputRequired(core.UserInputRequest{ - Title: "Password", - Prompt: prompt, - IsPassword: true, - }) - if err != nil { - return "", err - } - return resp.Text, nil - } - first, err := readPw("Please enter a password for the imported account") - if err != nil { - return err - } - second, err := readPw("Please repeat the password you just entered") - if err != nil { - return err - } - if first != second { - return errors.New("Passwords do not match") - } - acc, err := internalApi.ImportRawKey(hexkey, first) - if err != nil { - return err - } - ui.ShowInfo(fmt.Sprintf(`Key imported: - Address %v - Keystore file: %v - -The key is now encrypted; losing the password will result in permanently losing -access to the key and all associated funds! - -Make sure to backup keystore and passwords in a safe location.`, - acc.Address, acc.URL.Path)) - return nil -} - func initialize(c *cli.Context) error { // Set up the logger to print everything logOutput := os.Stdout @@ -553,7 +497,7 @@ func initialize(c *cli.Context) error { } func newAccount(c *cli.Context) error { - internalApi, err := initInternalApi(c) + internalApi, _, err := initInternalApi(c) if err != nil { return err } @@ -565,7 +509,7 @@ func newAccount(c *cli.Context) error { } func listAccounts(c *cli.Context) error { - internalApi, err := initInternalApi(c) + internalApi, _, err := initInternalApi(c) if err != nil { return err } @@ -584,7 +528,7 @@ func listAccounts(c *cli.Context) error { } func listWallets(c *cli.Context) error { - internalApi, err := initInternalApi(c) + internalApi, _, err := initInternalApi(c) if err != nil { return err } @@ -603,6 +547,57 @@ func listWallets(c *cli.Context) error { return nil } +// accountImport imports a raw hexadecimal private key via CLI. +func accountImport(c *cli.Context) error { + if c.Args().Len() != 1 { + return errors.New(" must be given as first argument.") + } + internalApi, ui, err := initInternalApi(c) + if err != nil { + return err + } + pKey, err := crypto.LoadECDSA(c.Args().First()) + if err != nil { + return err + } + readPw := func(prompt string) (string, error) { + resp, err := ui.OnInputRequired(core.UserInputRequest{ + Title: "Password", + Prompt: prompt, + IsPassword: true, + }) + if err != nil { + return "", err + } + return resp.Text, nil + } + first, err := readPw("Please enter a password for the imported account") + if err != nil { + return err + } + second, err := readPw("Please repeat the password you just entered") + if err != nil { + return err + } + if first != second { + return errors.New("Passwords do not match") + } + acc, err := internalApi.ImportRawKey(hex.EncodeToString(crypto.FromECDSA(pKey)), first) + if err != nil { + return err + } + ui.ShowInfo(fmt.Sprintf(`Key imported: + Address %v + Keystore file: %v + +The key is now encrypted; losing the password will result in permanently losing +access to the key and all associated funds! + +Make sure to backup keystore and passwords in a safe location.`, + acc.Address, acc.URL.Path)) + return nil +} + // ipcEndpoint resolves an IPC endpoint based on a configured value, taking into // account the set data folders as well as the designated platform we're currently // running on. diff --git a/cmd/clef/run_test.go b/cmd/clef/run_test.go new file mode 100644 index 00000000000..fc3145b1e0c --- /dev/null +++ b/cmd/clef/run_test.go @@ -0,0 +1,109 @@ +// Copyright 2022 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 main + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/docker/pkg/reexec" + "github.com/ethereum/go-ethereum/internal/cmdtest" +) + +const registeredName = "clef-test" + +type testproc struct { + *cmdtest.TestCmd + + // template variables for expect + Datadir string + Etherbase string +} + +func init() { + reexec.Register(registeredName, func() { + if err := app.Run(os.Args); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) + }) +} + +func TestMain(m *testing.M) { + // check if we have been reexec'd + if reexec.Init() { + return + } + os.Exit(m.Run()) +} + +// runClef spawns clef with the given command line args and adds keystore arg. +// This method creates a temporary keystore folder which will be removed after +// the test exits. +func runClef(t *testing.T, args ...string) *testproc { + ddir, err := os.MkdirTemp("", "cleftest-*") + if err != nil { + return nil + } + t.Cleanup(func() { + os.RemoveAll(ddir) + }) + return runWithKeystore(t, ddir, args...) +} + +// runWithKeystore spawns clef with the given command line args and adds keystore arg. +// This method does _not_ create the keystore folder, but it _does_ add the arg +// to the args. +func runWithKeystore(t *testing.T, keystore string, args ...string) *testproc { + args = append([]string{"--keystore", keystore}, args...) + tt := &testproc{Datadir: keystore} + tt.TestCmd = cmdtest.NewTestCmd(t, tt) + // Boot "clef". This actually runs the test binary but the TestMain + // function will prevent any tests from running. + tt.Run(registeredName, args...) + return tt +} + +func (proc *testproc) input(text string) *testproc { + proc.TestCmd.InputLine(text) + return proc +} + +/* +// waitForEndpoint waits for the rpc endpoint to appear, or +// aborts after 3 seconds. +func (proc *testproc) waitForEndpoint(t *testing.T) *testproc { + t.Helper() + timeout := 3 * time.Second + ipc := filepath.Join(proc.Datadir, "clef.ipc") + + start := time.Now() + for time.Since(start) < timeout { + if _, err := os.Stat(ipc); !errors.Is(err, os.ErrNotExist) { + t.Logf("endpoint %v opened", ipc) + return proc + } + time.Sleep(200 * time.Millisecond) + } + t.Logf("stderr: \n%v", proc.StderrText()) + t.Logf("stdout: \n%v", proc.Output()) + t.Fatal("endpoint", ipc, "did not open within", timeout) + return proc +} +*/