From 39d43a74b10a5c38818db451890e574f841403e4 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 30 Nov 2017 14:59:06 +0100 Subject: [PATCH] cmd/signer: minor changes --- cmd/signer/README.md | 37 +++++++++++++++++++++---------------- cmd/signer/api.go | 16 ++++++++-------- cmd/signer/auditlog.go | 23 ----------------------- cmd/signer/cliui.go | 34 +++++++++++++++++++++++++++------- cmd/signer/main.go | 3 +-- 5 files changed, 57 insertions(+), 56 deletions(-) delete mode 100644 cmd/signer/auditlog.go diff --git a/cmd/signer/README.md b/cmd/signer/README.md index bd43f8acd1..d3694c25aa 100644 --- a/cmd/signer/README.md +++ b/cmd/signer/README.md @@ -11,13 +11,23 @@ synchronised with the chain or a particular Ethereum node that has no build in, In its current form the signer is very limited and designed to work with Mist. It hasn't got a connection to an Ethereum node. This restriction imposed many limitations such as the lack of ability to keep track of nonces, balances -or fetching additional information that can help the user to make a decision to sign a transaction or data. Currently -the signer only supports password protected accounts. Support for hardware tokens such as Trezor and Legder is planned. +or fetching additional information that can help the user to make a decision to sign a transaction or data. ## Command line flags The signer accepts the following command line options: -- keystore, the directory where the password protected keystore stores keyfiles. The default directory is within geth's datadir. It is OS dependand, use `signer -h` to see where the location is on your system. -- chainid, the chain identifier. Default value is the Ethereum mainnet. See of a list of chain identifiers that are used https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md. +``` + --chainid value chain identifier (default: 1) + --loglevel value log level to emit to the screen (default: 4) + --keystore value Directory for the keystore (default: "/home/martin/.ethereum/keystore") + --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 + --rpcaddr value HTTP-RPC server listening interface (default: "localhost") + --rpcport value HTTP-RPC server listening port (default: 8550) + --4bytedb value File containing 4byte-identifiers (default: "./4byte.json") + --help, -h show help +``` + Example: ``` @@ -25,7 +35,8 @@ signer -keystore /my/keystore -chainid 4 ``` ## Communicating -The signer listens on stdin for incoming requests and sends responses on stdout. Messages are expected to follow the + +The signer listens to HTTP requests on `rpcaddr`:`rpcport`. The messages are 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 @@ -50,7 +61,8 @@ The client is responsible for creating a backup of the keystore. If the keystore lost accounts. #### Arguments - - passphrase [string]: passphrase that is used to protect the private key that is stored within the keystore + +None #### Result - address [string]: account address that is derived from the generated key @@ -83,7 +95,8 @@ lost accounts. List all accounts that this signer currently manages #### Arguments - none + +None #### Result - array with account records: @@ -120,11 +133,10 @@ lost accounts. ### account_signTransaction #### Sign transactions - Signs a transactions and respons with the signed transaction in RLP encoded form. + Signs a transactions and responds with the signed transaction in RLP encoded form. #### Arguments - from [address]: account to send the transaction from - - passphrase [string]: passphrase to unlock the from account - Transaction object: - transaction.to [address]: receiver account - gas [number]: maximum amount of gas to burn @@ -144,7 +156,6 @@ lost accounts. "method": "account_signTransaction", "params": [ "0x1923f626bb8dc025849e00f99c25fe2b2f7fb0db", - "my password", { "gas": "0x55555", "gasPrice": "0x1234", @@ -170,7 +181,6 @@ lost accounts. #### Arguments - account [address]: account to sign with - - passphrase [string]: passphrase to unlock the account - data [data]: data to sign #### Result @@ -184,7 +194,6 @@ lost accounts. "method": "account_sign", "params": [ "0x1923f626bb8dc025849e00f99c25fe2b2f7fb0db", - "my password", "0xaabbccdd" ] } @@ -236,8 +245,6 @@ lost accounts. #### Arguments - account [object]: key in [web3 keystore format](https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition) (retrieved with account_export) - - passphrase [string]: password to decrypt the given account - - newPassphrase [string]: password to encrypt the imported key in the keystore with #### Result - imported key [object]: @@ -273,8 +280,6 @@ lost accounts. "id": "09bccb61-b8d3-4e93-bf4f-205a8194f0b9", "version": 3 }, - "my password", - "my password" ] } { diff --git a/cmd/signer/api.go b/cmd/signer/api.go index 814cb48e8e..bac435606e 100644 --- a/cmd/signer/api.go +++ b/cmd/signer/api.go @@ -41,7 +41,6 @@ type SignerAPI struct { am *accounts.Manager ui SignerUI abidb abiDb - audit *auditlogger } // Metadata about the request @@ -173,7 +172,7 @@ func (ui *HeadlessUI) ShowInfo(message string) { // key that is generated when a new account is created. // noUSB disables USB support that is required to support hardware devices such as // ledger and trezor. -func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abidb *abiDb, auditlog string) *SignerAPI { +func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abidb *abiDb) *SignerAPI { var backends []accounts.Backend // support password based accounts @@ -197,11 +196,7 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi log.Debug("Trezor support enabled") } } - var al *auditlogger - if auditlog != ""{ - al = &auditlogger{auditlog} - } - return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, *abidb, al} + return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, *abidb} } func metaData(ctx context.Context) Metadata { @@ -300,6 +295,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address, } signedTx, err := wallet.SignTxWithPassphrase(acc, result.pw, tx, api.chainID) if err != nil { + api.ui.ShowError(err.Error()) return nil, err } return rlp.EncodeToBytes(signedTx) @@ -337,6 +333,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.Address, data hexuti // Assemble sign the data with the wallet signature, err := wallet.SignHashWithPassphrase(account, res.pw, sighash) if err != nil { + api.ui.ShowError(err.Error()) return nil, err } signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper @@ -414,7 +411,8 @@ func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.Raw // Imports tries to import the given keyJSON in the local keystore. The keyJSON data is expected to be // in web3 keystore format. It will decrypt the keyJSON with the given passphrase and on successful // decryption it will encrypt the key with the given newPassphrase and store it in the keystore. -func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) { +func (api *SignerAPI) Import(ctx context.Context, string, keyJSON json.RawMessage) (Account, error) { + be := api.am.Backends(keystore.KeyStoreType) if len(be) == 0 { @@ -424,10 +422,12 @@ func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Acco ch := make(chan ImportResponse, 1) api.ui.ApproveImport(&ImportRequest{}, metaData(ctx), ch) + if resp := <-ch; resp.approved { acc, err := be[0].(*keystore.KeyStore).Import(keyJSON, resp.oldPassword, resp.newPassword) if err != nil { + api.ui.ShowError(err.Error()) return Account{}, err } diff --git a/cmd/signer/auditlog.go b/cmd/signer/auditlog.go deleted file mode 100644 index c084313cfc..0000000000 --- a/cmd/signer/auditlog.go +++ /dev/null @@ -1,23 +0,0 @@ -package main - -import ( - "os" - "github.com/ethereum/go-ethereum/log" -) - -type auditlogger struct{ - filename string -} -func (al auditlogger) append(text string) error{ - f, err := os.OpenFile(al.filename, os.O_APPEND|os.O_WRONLY, 0600) - if err != nil { - log.Crit("Failed to open audit log","err", err) - return err - } - defer f.Close() - if _, err = f.WriteString(text); err != nil { - log.Crit("Failed to write to audit log","err", err) - return err - } -} - diff --git a/cmd/signer/cliui.go b/cmd/signer/cliui.go index efdee0b85b..6bb8ccca3d 100644 --- a/cmd/signer/cliui.go +++ b/cmd/signer/cliui.go @@ -54,21 +54,37 @@ func (ui *CommandlineUI) readString() string { // readPassword reads a single line from stdin, trimming it from the trailing new // line and returns it. The input will not be echoed. func (ui *CommandlineUI) readPassword() string { + fmt.Printf("Enter password to approve:\n") fmt.Printf("> ") text, err := terminal.ReadPassword(int(os.Stdin.Fd())) if err != nil { log.Crit("Failed to read password", "err", err) } fmt.Println() + fmt.Println("-----------------------") + return string(text) +} + +// readPassword reads a single line from stdin, trimming it from the trailing new +// line and returns it. The input will not be echoed. +func (ui *CommandlineUI) readPasswordText(inputstring string) string { + fmt.Printf("Enter %s:\n", inputstring) + fmt.Printf("> ") + text, err := terminal.ReadPassword(int(os.Stdin.Fd())) + if err != nil { + log.Crit("Failed to read password", "err", err) + } + fmt.Println("-----------------------") return string(text) } // confirm returns true if user enters 'Yes', otherwise false func (ui *CommandlineUI) confirm() bool { - fmt.Printf("Type 'Yes' to approve\n") + fmt.Printf("Type 'Yes' to approve:\n") if ui.readString() == "Yes" { return true } + fmt.Println("-----------------------") return false } @@ -98,7 +114,7 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch showMetadata(metadata) fmt.Printf("-------------------------------------------\n") - ch <- SignTxResponse{request.transaction.Hash(), ui.confirm(), ""} + ch <- SignTxResponse{request.transaction.Hash(), true ,ui.readPassword()} } // ApproveSignData prompt the user for confirmation to request to sign data @@ -113,7 +129,7 @@ func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest, metadata Meta fmt.Printf("message hash: %v\n", request.hash) fmt.Printf("-------------------------------------------\n") showMetadata(metadata) - ch <- SignDataResponse{ui.confirm(), ""} + ch <- SignDataResponse{true, ui.readPassword()} } // ApproveExport prompt the user for confirmation to export encrypted account json @@ -141,7 +157,11 @@ func (ui *CommandlineUI) ApproveImport(request *ImportRequest, metadata Metadata fmt.Printf("A request has been made to import an encrypted keyfile\n") fmt.Printf("-------------------------------------------\n") showMetadata(metadata) - ch <- ImportResponse{ui.confirm(), "", ""} + if ui.confirm(){ + ch <- ImportResponse{true, ui.readPasswordText("Old password"), ui.readPasswordText("New password")} + }else{ + ch <- ImportResponse{false, "", ""} + } } // ApproveListing prompt the user for confirmation to list accounts @@ -177,17 +197,17 @@ func (ui *CommandlineUI) ApproveNewAccount(requst *NewAccountRequest, metadata M fmt.Printf("Approving this operation means that a new account is created,\n") fmt.Printf("and the address show to the caller\n") showMetadata(metadata) - ch <- NewAccountResponse{ui.confirm(), ""} + ch <- NewAccountResponse{ui.confirm(), ui.readPassword()} } // ShowError displays error message to user func (ui *CommandlineUI) ShowError(message string) { - fmt.Printf("ERROR: %v", message) + fmt.Printf("ERROR: %v\n", message) } // ShowInfo displays info message to user func (ui *CommandlineUI) ShowInfo(message string) { - fmt.Printf("Info: %v", message) + fmt.Printf("Info: %v\n", message) } diff --git a/cmd/signer/main.go b/cmd/signer/main.go index d81f2955cd..2c75f473d4 100644 --- a/cmd/signer/main.go +++ b/cmd/signer/main.go @@ -91,8 +91,7 @@ func main() { c.Int64(utils.NetworkIdFlag.Name), c.String("keystore"), c.Bool(utils.NoUSBFlag.Name), - NewCommandlineUI(), db, - c.String("auditlog")) + NewCommandlineUI(), db) listener net.Listener //err error )