signer: review concerns+ baptize Clef

cmd/clef/README.md

@@ -1,20 +1,20 @@
-**Signer API**
+Clef
 ----
-The signer utility can be used to sign transactions and data and is meant as a replacement for geth's account management.
+Clef can be used to sign transactions and data and is meant as a replacement for geth's account management.
 This allows DApps not to depend on geth's account management. When a DApp wants to sign data it can send the data to
-the signer, the signer will than provide the user with context and asks the user for permission to sign the data. If
+the signer, the signer will then provide the user with context and asks the user for permission to sign the data. If
 the users grants the signing request the signer will send the signature back to the DApp.
   
 This setup allows a DApp to connect to a remote Ethereum node and send transactions that are locally signed. This can
 help in situations when a DApp is connected to a remote node because a local Ethereum node is not available, not
-synchronised with the chain or a particular Ethereum node that has no build in, or limited account management.
+synchronised with the chain or a particular Ethereum node that has no built-in (or limited) account management.
   
-The signer can run as a daemon on the same machine, or off a usb-stick like [usb armory](https://inversepath.com/usbarmory),
+Clef can run as a daemon on the same machine, or off a usb-stick like [usb armory](https://inversepath.com/usbarmory),
 or a separate VM in a [QubesOS](https://www.qubes-os.org/) type os setup.
 
 
 ## Command line flags
-The signer accepts the following command line options:
+Clef accepts the following command line options:
 ```
 COMMANDS:
    init    Initialize the signer, generate secret storage
@@ -24,8 +24,8 @@ COMMANDS:
 
 GLOBAL OPTIONS:
    --loglevel value        log level to emit to the screen (default: 4)
-   --keystore value        Directory for the keystore (default: "/home/martin/.ethereum/keystore")
+   --keystore value        Directory for the keystore (default: "$HOME/.ethereum/keystore")
-   --configdir value       Directory for signer configuration (default: "/home/martin/.signer")
+   --configdir value       Directory for clef configuration (default: "$HOME/.clef")
    --networkid value       Network identifier (integer, 1=Frontier, 2=Morden (disused), 3=Ropsten, 4=Rinkeby) (default: 1)
    --lightkdf              Reduce key-derivation RAM & CPU usage at some expense of KDF strength
    --nousb                 Disables monitoring for and managing USB hardware wallets
@@ -55,7 +55,7 @@ Check out the [tutorial](tutorial.md) for some concrete examples on how the sign
 
 The security model of the signer is as follows:
 
-* One critical component (the signer binary / daemon) is responsible for handling cryptographic operations; signing, private keys, encryption/decryption of keystore files.
+* One critical component (the signer binary / daemon) is responsible for handling cryptographic operations: signing, private keys, encryption/decryption of keystore files.
 * The signer binary has a well-defined 'external' API.
 * The 'external' API is considered UNTRUSTED.
 * The signer binary also communicates with whatever process that invoked the binary, via stdin/stdout.
@@ -70,25 +70,24 @@ In this case, `geth` would be started with `--externalsigner=http://localhost:85
 
 Some snags and todos
 
-* The signer should take a startup param "--no-change", for UI:s that do not contain the capability
+* [ ] The signer should take a startup param "--no-change", for UIs that do not contain the capability
    to perform changes to things, only approve/deny. Such a UI should be able to start the signer in
    a more secure mode by telling it that it only wants approve/deny capabilities.
 
-* [x] DONE: It would be nice if the signer could collect new 4byte-id:s/method selectors, and have a
+* [x] It would be nice if the signer could collect new 4byte-id:s/method selectors, and have a
 secondary database for those (`4byte_custom.json`). Users could then (optionally) submit their collections for
 inclusion upstream.
 
 * It should be possible to configure the signer to check if an account is indeed known to it, before
 passing on to the UI. The reason it currently does not, is that it would make it possible to enumerate
 accounts if it immediately returned "unknown account".
-  * [x] DONE: Similarly, it should be possible to configure the signer to auto-allow listing (certain) accounts, instead of asking every time.
+* [x] It should be possible to configure the signer to auto-allow listing (certain) accounts, instead of asking every time.
-
 * [x] Done Upon startup, the signer should spit out some info to the caller (particularly important when executed in `stdio-ui`-mode),
 invoking methods with the following info:
   * [x] Version info about the signer
   * [x] Address of API (http/ipc)
   * [ ] List of known accounts
-
+* [ ] Have a default timeout on signing operations, so that if the user has not answered withing e.g. 60 seconds, the request is rejected.
 * [ ] `account_signRawTransaction`
 * [ ] `account_bulkSignTransactions([] transactions)` should
    * only exist if enabled via config/flag
@@ -98,17 +97,15 @@ invoking methods with the following info:
       * the total amount
       * the number of unique recipients
 
-
 * Geth todos
     - The signer should pass the `Origin` header as call-info to the UI. As of right now, the way that info about the request is
 put together is a bit of a hack into the http server. This could probably be greatly improved
     - Relay: Geth should be started in `geth --external_signer localhost:8550`.
-    - Currently, the Geth API:s use `common.Address` in the arguments to transaction submission (e.g `to` field). This
+    - Currently, the Geth APIs use `common.Address` in the arguments to transaction submission (e.g `to` field). This
   type is 20 `bytes`, and is incapable of carrying checksum information. The signer uses `common.MixedcaseAddress`, which
   retains the original input.
     - The Geth api should switch to use the same type, and relay `to`-account verbatim to the external api.
 
-
 * [x] Storage
     * [x] An encrypted key-value storage should be implemented
     * See [rules.md](rules.md) for more info about this.
@@ -128,7 +125,7 @@ The signer listens to HTTP requests on `rpcaddr`:`rpcport`, with the same JSONRP
 expected to be JSON [jsonrpc 2.0 standard](http://www.jsonrpc.org/specification).
 
 Some of these call can require user interaction. Clients must be aware that responses
-may be deplayed significanlty or may never be received if a users decideds to ignore the confirmation request.
+may be delayed significanlty or may never be received if a users decides to ignore the confirmation request.
 
 The External API is **untrusted** : it does not accept credentials over this api, nor does it expect
 that requests have any authority.
@@ -578,8 +575,7 @@ curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","me
         "data": "0x4401a6e40000000000000000000000000000000000000000000000000000000000000012",
         "input": null
       },
-      "call_info": {
+      "call_info": [
-        "Messages": [
           {
             "type": "WARNING",
             "message": "Invalid checksum on to-address"
@@ -588,8 +584,7 @@ curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","me
             "type": "Info",
             "message": "safeSend(address: 0x0000000000000000000000000000000000000012)"
           }
-        ]
+        ],
-      },
       "meta": {
         "remote": "127.0.0.1:48486",
         "local": "localhost:8550",
@@ -625,8 +620,7 @@ curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","me
         "data": "0x4401a6e40000000000000002000000000000000000000000000000000000000000000012",
         "input": null
       },
-      "call_info": {
+      "call_info": [
-        "Messages": [
           {
             "type": "WARNING",
             "message": "Invalid checksum on to-address"
@@ -635,8 +629,7 @@ curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","me
             "type": "WARNING",
             "message": "Transaction data did not match ABI-interface: WARNING: Supplied data is stuffed with extra data. \nWant 0000000000000002000000000000000000000000000000000000000000000012\nHave 0000000000000000000000000000000000000000000000000000000000000012\nfor method safeSend(address)"
           }
-        ]
+        ],
-      },
       "meta": {
         "remote": "127.0.0.1:48492",
         "local": "localhost:8550",
@@ -670,14 +663,12 @@ One which has missing `to`, but with no `data`:
         "data": null,
         "input": null
       },
-      "call_info": {
+      "call_info": [
-        "Messages": [
           {
             "type": "CRITICAL",
             "message": "Tx will create contract with empty code!"
           }
-        ]
+        ],
-      },
       "meta": {
         "remote": "signer binary",
         "local": "main",

cmd/clef/main.go

@@ -34,29 +34,29 @@ import (
 	"strings"
 
 	"encoding/hex"
-	"github.com/ethereum/go-ethereum/cmd/signer/core"
-	"github.com/ethereum/go-ethereum/cmd/signer/rules"
-	"github.com/ethereum/go-ethereum/cmd/signer/storage"
 	"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/ethereum/go-ethereum/node"
 	"github.com/ethereum/go-ethereum/rpc"
+	"github.com/ethereum/go-ethereum/signer/core"
+	"github.com/ethereum/go-ethereum/signer/rules"
+	"github.com/ethereum/go-ethereum/signer/storage"
 	"gopkg.in/urfave/cli.v1"
 	"os/signal"
 )
 
-// EXT_API_VERSION -- see extapi_changelog.md
+// ExternalApiVersion -- see extapi_changelog.md
-const EXT_API_VERSION = "2.0.0"
+const ExternalApiVersion = "2.0.0"
 
-// INT_API_VERSION -- see intapi_changelog.md
+// InternalApiVersion -- see intapi_changelog.md
-const INT_API_VERSION = "2.0.0"
+const InternalApiVersion = "2.0.0"
 
 const legal_warning = `
 WARNING! 
 
-The signer is alpha software, and not yet publically released. This software has _not_ been audited, and there
+Clef is alpha software, and not yet publically released. This software has _not_ been audited, and there
 are no guarantees about the workings of this software. It may contain severe flaws. You should not use this software
 unless you agree to take full responsibility for doing so, and know what you are doing. 
 
@@ -78,7 +78,7 @@ var (
 	configdirFlag = cli.StringFlag{
 		Name:  "configdir",
 		Value: DefaultConfigDir(),
-		Usage: "Directory for signer configuration",
+		Usage: "Directory for Clef configuration",
 	}
 	rpcPortFlag = cli.IntFlag{
 		Name:  "rpcport",
@@ -87,7 +87,7 @@ var (
 	}
 	signerSecretFlag = cli.StringFlag{
 		Name:  "signersecret",
-		Usage: "A file containing the password used to encrypt signer credentials, e.g. keystore credentials and ruleset hash",
+		Usage: "A file containing the password used to encrypt Clef credentials, e.g. keystore credentials and ruleset hash",
 	}
 	dBFlag = cli.StringFlag{
 		Name:  "4bytedb",
@@ -113,11 +113,11 @@ var (
 		Name: "stdio-ui",
 		Usage: "Use STDIN/STDOUT as a channel for an external UI. " +
 			"This means that an STDIN/STDOUT is used for RPC-communication with a e.g. a graphical user " +
-			"interface, and can be used when the signer is started by an external process.",
+			"interface, and can be used when Clef is started by an external process.",
 	}
 	testFlag = cli.BoolFlag{
 		Name:  "stdio-ui-test",
-		Usage: "Mechanism to test interface between signer and UI. Requires 'stdio-ui'.",
+		Usage: "Mechanism to test interface between Clef and UI. Requires 'stdio-ui'.",
 	}
 	app         = cli.NewApp()
 	initCommand = cli.Command{
@@ -130,7 +130,7 @@ var (
 			configdirFlag,
 		},
 		Description: `
-The init command generates a master seed which the signer can use to store credentials and data needed for 
+The init command generates a master seed which Clef can use to store credentials and data needed for 
 the rule-engine to work.`,
 	}
 	attestCommand = cli.Command{
@@ -148,7 +148,7 @@ The attest command stores the sha256 of the rule.js-file that you want to use fo
 incoming requests. 
 
 Whenever you make an edit to the rule file, you need to use attestation to tell 
-the signer that the file is 'safe' to execute.`,
+Clef that the file is 'safe' to execute.`,
 	}
 
 	addCredentialCommand = cli.Command{
@@ -169,7 +169,7 @@ remove any stored credential for that address (keyfile)
 )
 
 func init() {
-	app.Name = "signer"
+	app.Name = "Clef"
 	app.Usage = "Manage ethereum Account operations"
 	app.Flags = []cli.Flag{
 		logLevelFlag,
@@ -416,7 +416,7 @@ func signer(c *cli.Context) error {
 
 		// start http server
 		httpEndpoint := fmt.Sprintf("%s:%d", c.String(utils.RPCListenAddrFlag.Name), c.Int(rpcPortFlag.Name))
-		listener, _, err := node.StartHttpEndpoint(httpEndpoint, rpcApi, []string{"account"}, cors, vhosts)
+		listener, _, err := node.StartHTTPEndpoint(httpEndpoint, rpcApi, []string{"account"}, cors, vhosts)
 		if err != nil {
 			utils.Fatalf("Could not start RPC api: %v", err)
 		}
@@ -433,7 +433,7 @@ func signer(c *cli.Context) error {
 		if c.IsSet(utils.IPCPathFlag.Name) {
 			ipcApiUrl = c.String(utils.IPCPathFlag.Name)
 		} else {
-			ipcApiUrl = fmt.Sprintf("%s", filepath.Join(configDir, "signer.ipc"))
+			ipcApiUrl = fmt.Sprintf("%s", filepath.Join(configDir, "clef.ipc"))
 		}
 
 		listener, _, err := node.StartIPCEndpoint(func() bool { return true }, ipcApiUrl, rpcApi)
@@ -454,8 +454,8 @@ func signer(c *cli.Context) error {
 	}
 	ui.OnSignerStartup(core.StartupInfo{
 		Info: map[string]interface{}{
-			"extapi_version": EXT_API_VERSION,
+			"extapi_version": ExternalApiVersion,
-			"intapi_version": INT_API_VERSION,
+			"intapi_version": InternalApiVersion,
 			"extapi_http":    extapiUrl,
 			"extapi_ipc":     ipcApiUrl,
 		},
@@ -493,7 +493,7 @@ func DefaultConfigDir() string {
 		} else if runtime.GOOS == "windows" {
 			return filepath.Join(home, "AppData", "Roaming", "Signer")
 		} else {
-			return filepath.Join(home, ".signer")
+			return filepath.Join(home, ".clef")
 		}
 	}
 	// As we cannot guess a stable location, return empty and handle later
@@ -574,7 +574,7 @@ func confirm(text string) bool {
 
 func testExternalUI(api *core.SignerAPI) {
 
-	ctx := context.WithValue(context.Background(), "remote", "signer binary")
+	ctx := context.WithValue(context.Background(), "remote", "clef binary")
 	ctx = context.WithValue(ctx, "scheme", "in-proc")
 	ctx = context.WithValue(ctx, "local", "main")
 

cmd/clef/pythonsigner.py

@@ -4,9 +4,8 @@ from tinyrpc.protocols.jsonrpc import JSONRPCProtocol
 from tinyrpc.dispatch import public,RPCDispatcher
 from tinyrpc.server import RPCServer
 
-""" This is a POC example of how to write a custom UI for the signer. The UI starts the 
+""" This is a POC example of how to write a custom UI for Clef. The UI starts the
-signer process with the '--stdio-ui' option, and communicates with the signer binary
+clef process with the '--stdio-ui' option, and communicates with clef using standard input / output.
-using standard input / output.
 
 The standard input/output is a relatively secure way to communicate, as it does not require opening any ports
 or IPC files. Needless to say, it does not protect against memory inspection mechanisms where an attacker
@@ -160,7 +159,7 @@ class StdIOHandler():
 
 def main(args):
 
-    cmd = ["./signer", "--stdio-ui"]
+    cmd = ["./clef", "--stdio-ui"]
     if len(args) > 0 and args[0] == "test":
         cmd.extend(["--stdio-ui-test"])
     print("cmd: {}".format(" ".join(cmd)))

signer/core/api.go

@@ -366,7 +366,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth
 	}
 
 	rlpdata, err := rlp.EncodeToBytes(signedTx)
-	response := ethapi.SignTransactionResult{rlpdata, signedTx}
+	response := ethapi.SignTransactionResult{Raw: rlpdata, Tx: signedTx}
 
 	// Finally, send the signed tx to the UI
 	api.UI.OnApprovedTx(response)

signer/core/types.go

@@ -60,18 +60,6 @@ type ValidationMessages struct {
 	Messages []ValidationInfo
 }
 
-/*
-// TransactionArg represents a Transaction for the signer.
-type TransactionArg struct {
-	To       *common.MixedcaseAddress `json:"to"`
-	Gas      *hexutil.Big             `json:"gas"`
-	GasPrice *hexutil.Big             `json:"gasPrice"`
-	Value    *hexutil.Big             `json:"value"`
-	Data     hexutil.Bytes            `json:"data"`
-	Nonce    *hexutil.Uint64          `json:"nonce"`
-}
-*/
-
 // SendTxArgs represents the arguments to submit a transaction
 type SendTxArgs struct {
 	From     common.MixedcaseAddress  `json:"from"`

signer/rules/rules.go

@@ -22,12 +22,12 @@ import (
 	"os"
 	"strings"
 
-	"github.com/ethereum/go-ethereum/cmd/signer/core"
-	"github.com/ethereum/go-ethereum/cmd/signer/rules/deps"
-	"github.com/ethereum/go-ethereum/cmd/signer/storage"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/internal/ethapi"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/signer/core"
+	"github.com/ethereum/go-ethereum/signer/rules/deps"
+	"github.com/ethereum/go-ethereum/signer/storage"
 	"github.com/robertkrimen/otto"
 )
 

signer/rules/rules_test.go

@@ -23,8 +23,8 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/accounts"
-	"github.com/ethereum/go-ethereum/cmd/signer/core"
+	"github.com/ethereum/go-ethereum/cmd/clef/core"
-	"github.com/ethereum/go-ethereum/cmd/signer/storage"
+	"github.com/ethereum/go-ethereum/cmd/clef/storage"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"