From f08cd94fb755471cb78091af99ef7026afb392f3 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 16 Jan 2018 15:42:41 +0100 Subject: [PATCH] cmd/ethkey: fix formatting, review nits (#15807) This commit: - Adds a --msgfile option to read the message to sign from a file instead of command line argument. - Adds a unit test for signing subcommands. - Removes some weird whitespace in the code. --- cmd/ethkey/generate.go | 63 +++++++++++------------ cmd/ethkey/inspect.go | 1 + cmd/ethkey/main.go | 33 ++++++------ cmd/ethkey/message.go | 97 ++++++++++++++++++++---------------- cmd/ethkey/message_test.go | 70 ++++++++++++++++++++++++++ cmd/ethkey/run_test.go | 54 ++++++++++++++++++++ internal/cmdtest/test_cmd.go | 8 +-- 7 files changed, 231 insertions(+), 95 deletions(-) create mode 100644 cmd/ethkey/message_test.go create mode 100644 cmd/ethkey/run_test.go diff --git a/cmd/ethkey/generate.go b/cmd/ethkey/generate.go index dee0e9d70e..6d57d17fb4 100644 --- a/cmd/ethkey/generate.go +++ b/cmd/ethkey/generate.go @@ -1,8 +1,23 @@ +// Copyright 2017 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 ( "crypto/ecdsa" - "crypto/rand" "fmt" "io/ioutil" "os" @@ -26,16 +41,16 @@ var commandGenerate = cli.Command{ ArgsUsage: "[ ]", Description: ` Generate a new keyfile. -If you want to use an existing private key to use in the keyfile, it can be -specified by setting --privatekey with the location of the file containing the -private key.`, + +If you want to encrypt an existing private key, it can be specified by setting +--privatekey with the location of the file containing the private key. +`, Flags: []cli.Flag{ passphraseFlag, jsonFlag, cli.StringFlag{ - Name: "privatekey", - Usage: "the file from where to read the private key to " + - "generate a keyfile for", + Name: "privatekey", + Usage: "file containing a raw private key to encrypt", }, }, Action: func(ctx *cli.Context) error { @@ -51,32 +66,19 @@ private key.`, } var privateKey *ecdsa.PrivateKey - - // First check if a private key file is provided. - privateKeyFile := ctx.String("privatekey") - if privateKeyFile != "" { - privateKeyBytes, err := ioutil.ReadFile(privateKeyFile) + var err error + if file := ctx.String("privatekey"); file != "" { + // Load private key from file. + privateKey, err = crypto.LoadECDSA(file) if err != nil { - utils.Fatalf("Failed to read the private key file '%s': %v", - privateKeyFile, err) + utils.Fatalf("Can't load private key: %v", err) } - - pk, err := crypto.HexToECDSA(string(privateKeyBytes)) - if err != nil { - utils.Fatalf( - "Could not construct ECDSA private key from file content: %v", - err) - } - privateKey = pk - } - - // If not loaded, generate random. - if privateKey == nil { - pk, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + } else { + // If not loaded, generate random. + privateKey, err = crypto.GenerateKey() if err != nil { utils.Fatalf("Failed to generate random private key: %v", err) } - privateKey = pk } // Create the keyfile object with a random UUID. @@ -89,8 +91,7 @@ private key.`, // Encrypt key with passphrase. passphrase := getPassPhrase(ctx, true) - keyjson, err := keystore.EncryptKey(key, passphrase, - keystore.StandardScryptN, keystore.StandardScryptP) + keyjson, err := keystore.EncryptKey(key, passphrase, keystore.StandardScryptN, keystore.StandardScryptP) if err != nil { utils.Fatalf("Error encrypting key: %v", err) } @@ -110,7 +111,7 @@ private key.`, if ctx.Bool(jsonFlag.Name) { mustPrintJSON(out) } else { - fmt.Println("Address: ", out.Address) + fmt.Println("Address:", out.Address) } return nil }, diff --git a/cmd/ethkey/inspect.go b/cmd/ethkey/inspect.go index 8a7aeef848..219a5460b8 100644 --- a/cmd/ethkey/inspect.go +++ b/cmd/ethkey/inspect.go @@ -23,6 +23,7 @@ var commandInspect = cli.Command{ ArgsUsage: "", Description: ` Print various information about the keyfile. + Private key information can be printed by using the --private flag; make sure to use this feature with great caution!`, Flags: []cli.Flag{ diff --git a/cmd/ethkey/main.go b/cmd/ethkey/main.go index b9b7a18e05..2a9e5ee483 100644 --- a/cmd/ethkey/main.go +++ b/cmd/ethkey/main.go @@ -28,40 +28,37 @@ const ( defaultKeyfileName = "keyfile.json" ) -var ( - gitCommit = "" // Git SHA1 commit hash of the release (set via linker flags) +// Git SHA1 commit hash of the release (set via linker flags) +var gitCommit = "" - app *cli.App // the main app instance -) +var app *cli.App -var ( // Commonly used command line flags. +func init() { + app = utils.NewApp(gitCommit, "an Ethereum key manager") + app.Commands = []cli.Command{ + commandGenerate, + commandInspect, + commandSignMessage, + commandVerifyMessage, + } +} + +// Commonly used command line flags. +var ( passphraseFlag = cli.StringFlag{ Name: "passwordfile", Usage: "the file that contains the passphrase for the keyfile", } - jsonFlag = cli.BoolFlag{ Name: "json", Usage: "output JSON instead of human-readable format", } - messageFlag = cli.StringFlag{ Name: "message", Usage: "the file that contains the message to sign/verify", } ) -// Configure the app instance. -func init() { - app = utils.NewApp(gitCommit, "an Ethereum key manager") - app.Commands = []cli.Command{ - commandGenerate, - commandInspect, - commandSignMessage, - commandVerifyMessage, - } -} - func main() { if err := app.Run(os.Args); err != nil { fmt.Fprintln(os.Stderr, err) diff --git a/cmd/ethkey/message.go b/cmd/ethkey/message.go index ae6b6552d3..531a931c82 100644 --- a/cmd/ethkey/message.go +++ b/cmd/ethkey/message.go @@ -1,11 +1,25 @@ +// Copyright 2017 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 ( "encoding/hex" "fmt" "io/ioutil" - "os" - "strings" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/cmd/utils" @@ -18,26 +32,33 @@ type outputSign struct { Signature string } +var msgfileFlag = cli.StringFlag{ + Name: "msgfile", + Usage: "file containing the message to sign/verify", +} + var commandSignMessage = cli.Command{ Name: "signmessage", Usage: "sign a message", - ArgsUsage: " ", + ArgsUsage: " ", Description: ` Sign the message with a keyfile. -It is possible to refer to a file containing the message.`, + +To sign a message contained in a file, use the --msgfile flag. +`, Flags: []cli.Flag{ passphraseFlag, jsonFlag, + msgfileFlag, }, Action: func(ctx *cli.Context) error { - keyfilepath := ctx.Args().First() - message := []byte(ctx.Args().Get(1)) + message := getMessage(ctx, 1) // Load the keyfile. + keyfilepath := ctx.Args().First() keyjson, err := ioutil.ReadFile(keyfilepath) if err != nil { - utils.Fatalf("Failed to read the keyfile at '%s': %v", - keyfilepath, err) + utils.Fatalf("Failed to read the keyfile at '%s': %v", keyfilepath, err) } // Decrypt key with passphrase. @@ -47,29 +68,15 @@ It is possible to refer to a file containing the message.`, utils.Fatalf("Error decrypting key: %v", err) } - if len(message) == 0 { - utils.Fatalf("A message must be provided") - } - // Read message if file. - if _, err := os.Stat(string(message)); err == nil { - message, err = ioutil.ReadFile(string(message)) - if err != nil { - utils.Fatalf("Failed to read the message file: %v", err) - } - } - signature, err := crypto.Sign(signHash(message), key.PrivateKey) if err != nil { utils.Fatalf("Failed to sign message: %v", err) } - - out := outputSign{ - Signature: hex.EncodeToString(signature), - } + out := outputSign{Signature: hex.EncodeToString(signature)} if ctx.Bool(jsonFlag.Name) { mustPrintJSON(out) } else { - fmt.Println("Signature: ", out.Signature) + fmt.Println("Signature:", out.Signature) } return nil }, @@ -84,53 +91,40 @@ type outputVerify struct { var commandVerifyMessage = cli.Command{ Name: "verifymessage", Usage: "verify the signature of a signed message", - ArgsUsage: "
", + ArgsUsage: "
", Description: ` Verify the signature of the message. It is possible to refer to a file containing the message.`, Flags: []cli.Flag{ jsonFlag, + msgfileFlag, }, Action: func(ctx *cli.Context) error { addressStr := ctx.Args().First() signatureHex := ctx.Args().Get(1) - message := []byte(ctx.Args().Get(2)) + message := getMessage(ctx, 2) - // Determine whether it is a keyfile, public key or address. if !common.IsHexAddress(addressStr) { utils.Fatalf("Invalid address: %s", addressStr) } address := common.HexToAddress(addressStr) - signature, err := hex.DecodeString(signatureHex) if err != nil { utils.Fatalf("Signature encoding is not hexadecimal: %v", err) } - if len(message) == 0 { - utils.Fatalf("A message must be provided") - } - // Read message if file. - if _, err := os.Stat(string(message)); err == nil { - message, err = ioutil.ReadFile(string(message)) - if err != nil { - utils.Fatalf("Failed to read the message file: %v", err) - } - } - recoveredPubkey, err := crypto.SigToPub(signHash(message), signature) if err != nil || recoveredPubkey == nil { utils.Fatalf("Signature verification failed: %v", err) } recoveredPubkeyBytes := crypto.FromECDSAPub(recoveredPubkey) recoveredAddress := crypto.PubkeyToAddress(*recoveredPubkey) - success := address == recoveredAddress out := outputVerify{ Success: success, RecoveredPublicKey: hex.EncodeToString(recoveredPubkeyBytes), - RecoveredAddress: strings.ToLower(recoveredAddress.Hex()), + RecoveredAddress: recoveredAddress.Hex(), } if ctx.Bool(jsonFlag.Name) { mustPrintJSON(out) @@ -140,9 +134,26 @@ It is possible to refer to a file containing the message.`, } else { fmt.Println("Signature verification failed!") } - fmt.Println("Recovered public key: ", out.RecoveredPublicKey) - fmt.Println("Recovered address: ", out.RecoveredAddress) + fmt.Println("Recovered public key:", out.RecoveredPublicKey) + fmt.Println("Recovered address:", out.RecoveredAddress) } return nil }, } + +func getMessage(ctx *cli.Context, msgarg int) []byte { + if file := ctx.String("msgfile"); file != "" { + if len(ctx.Args()) > msgarg { + utils.Fatalf("Can't use --msgfile and message argument at the same time.") + } + msg, err := ioutil.ReadFile(file) + if err != nil { + utils.Fatalf("Can't read message file: %v", err) + } + return msg + } else if len(ctx.Args()) == msgarg+1 { + return []byte(ctx.Args().Get(msgarg)) + } + utils.Fatalf("Invalid number of arguments: want %d, got %d", msgarg+1, len(ctx.Args())) + return nil +} diff --git a/cmd/ethkey/message_test.go b/cmd/ethkey/message_test.go new file mode 100644 index 0000000000..fb16f03d02 --- /dev/null +++ b/cmd/ethkey/message_test.go @@ -0,0 +1,70 @@ +// Copyright 2017 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 ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +func TestMessageSignVerify(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "ethkey-test") + if err != nil { + t.Fatal("Can't create temporary directory:", err) + } + defer os.RemoveAll(tmpdir) + + keyfile := filepath.Join(tmpdir, "the-keyfile") + message := "test message" + + // Create the key. + generate := runEthkey(t, "generate", keyfile) + generate.Expect(` +!! Unsupported terminal, password will be echoed. +Passphrase: {{.InputLine "foobar"}} +Repeat passphrase: {{.InputLine "foobar"}} +`) + _, matches := generate.ExpectRegexp(`Address: (0x[0-9a-fA-F]{40})\n`) + address := matches[1] + generate.ExpectExit() + + // Sign a message. + sign := runEthkey(t, "signmessage", keyfile, message) + sign.Expect(` +!! Unsupported terminal, password will be echoed. +Passphrase: {{.InputLine "foobar"}} +`) + _, matches = sign.ExpectRegexp(`Signature: ([0-9a-f]+)\n`) + signature := matches[1] + sign.ExpectExit() + + // Verify the message. + verify := runEthkey(t, "verifymessage", address, signature, message) + _, matches = verify.ExpectRegexp(` +Signature verification successful! +Recovered public key: [0-9a-f]+ +Recovered address: (0x[0-9a-fA-F]{40}) +`) + recovered := matches[1] + verify.ExpectExit() + + if recovered != address { + t.Error("recovered address doesn't match generated key") + } +} diff --git a/cmd/ethkey/run_test.go b/cmd/ethkey/run_test.go new file mode 100644 index 0000000000..8ce4fe5cde --- /dev/null +++ b/cmd/ethkey/run_test.go @@ -0,0 +1,54 @@ +// Copyright 2017 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" +) + +type testEthkey struct { + *cmdtest.TestCmd +} + +// spawns ethkey with the given command line args. +func runEthkey(t *testing.T, args ...string) *testEthkey { + tt := new(testEthkey) + tt.TestCmd = cmdtest.NewTestCmd(t, tt) + tt.Run("ethkey-test", args...) + return tt +} + +func TestMain(m *testing.M) { + // Run the app if we've been exec'd as "ethkey-test" in runEthkey. + reexec.Register("ethkey-test", func() { + if err := app.Run(os.Args); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) + }) + // check if we have been reexec'd + if reexec.Init() { + return + } + os.Exit(m.Run()) +} diff --git a/internal/cmdtest/test_cmd.go b/internal/cmdtest/test_cmd.go index 541e51c4c0..fae61cfe32 100644 --- a/internal/cmdtest/test_cmd.go +++ b/internal/cmdtest/test_cmd.go @@ -25,6 +25,7 @@ import ( "os" "os/exec" "regexp" + "strings" "sync" "testing" "text/template" @@ -141,9 +142,10 @@ func (tt *TestCmd) matchExactOutput(want []byte) error { // Note that an arbitrary amount of output may be consumed by the // regular expression. This usually means that expect cannot be used // after ExpectRegexp. -func (tt *TestCmd) ExpectRegexp(resource string) (*regexp.Regexp, []string) { +func (tt *TestCmd) ExpectRegexp(regex string) (*regexp.Regexp, []string) { + regex = strings.TrimPrefix(regex, "\n") var ( - re = regexp.MustCompile(resource) + re = regexp.MustCompile(regex) rtee = &runeTee{in: tt.stdout} matches []int ) @@ -151,7 +153,7 @@ func (tt *TestCmd) ExpectRegexp(resource string) (*regexp.Regexp, []string) { output := rtee.buf.Bytes() if matches == nil { tt.Fatalf("Output did not match:\n---------------- (stdout text)\n%s\n---------------- (regular expression)\n%s", - output, resource) + output, regex) return re, nil } tt.Logf("Matched stdout text:\n%s", output)