cmd/ethkey: fix formatting, review nits (ethereum#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.
cslarson · Jan 16, 2018 · f08cd94 · f08cd94
1 parent 216e584
commit f08cd94
Show file tree

Hide file tree

Showing 7 changed files with 231 additions and 95 deletions.
diff --git 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 <http://www.gnu.org/licenses/>.
+
 package main
 
 import (
 	"crypto/ecdsa"
-	"crypto/rand"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -26,16 +41,16 @@ var commandGenerate = cli.Command{
 	ArgsUsage: "[ <keyfile> ]",
 	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
@@ -23,6 +23,7 @@ var commandInspect = cli.Command{
 	ArgsUsage: "<keyfile>",
 	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
@@ -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
@@ -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 <http://www.gnu.org/licenses/>.
+
 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: "<keyfile> <message/file>",
+	ArgsUsage: "<keyfile> <message>",
 	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: "<address> <signature> <message/file>",
+	ArgsUsage: "<address> <signature> <message>",
 	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
+}