cmd/signer: made possible for UI to modify tx parameters

cmd/signer/README.md

@@ -136,14 +136,17 @@ None
    Signs a transactions and responds with the signed transaction in RLP encoded form.
 
 #### Arguments
-  - from [address]: account to send the transaction from
-  - Transaction object:
-     - transaction.to [address]: receiver account
-     - gas [number]: maximum amount of gas to burn
-     - gasPrice [number]: gas price
-     - value [number:optional]: amount of Wei to send with the transaction
-     - data [data:optional]:  input data
-     - transaction.nonce [number]: account nonce
+  1. from [address]: account to send the transaction from
+  2. transaction object:
+     - `to` [address]: receiver account
+     - `gas` [number]: maximum amount of gas to burn
+     - `gasPrice` [number]: gas price
+     - `value` [number:optional]: amount of Wei to send with the transaction
+     - `data` [data:optional]:  input data
+     - `nonce` [number]: account nonce
+  3. method signature [string:optional]
+     - The method signature, if present, is to aid decoding the calldata. Should consist of `methodname(paramtype,...)`, e.g. `transfer(uint256,address)`. The signer may use this data to parse the supplied calldata, and show the user. The data, however, is considered totally untrusted, and reliability is not expected.
+
 
 #### Result
   - signed transaction in RLP encoded form [data]

cmd/signer/abihelper.go

@@ -133,6 +133,6 @@ func (db *abiDb) LookupABI(id []byte) (string, error) {
 	}
 	return "", fmt.Errorf("Signature %v not found", sig)
 }
-func (db *abiDb) Size() int{
+func (db *abiDb) Size() int {
 	return len(db.db)
 }

cmd/signer/api.go

@@ -53,13 +53,14 @@ type Metadata struct {
 type (
 	// SignTxRequest contains info about a transaction to sign
 	SignTxRequest struct {
-		transaction *types.Transaction
+		transaction types.Transaction
 		from        accounts.Account
 		callinfo    fmt.Stringer
 	}
 	// SignTxResponse result from SignTxRequest
 	SignTxResponse struct {
-		hash     common.Hash
+		//The UI may make changes to the TX
+		transaction types.Transaction
 		approved    bool
 		pw          string
 	}
@@ -244,7 +245,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address,
 		tx = types.NewTransaction(uint64(*args.Nonce), *args.To, (*big.Int)(args.Value), (*big.Int)(args.Gas), (*big.Int)(args.GasPrice), args.Data)
 	}
 
-	req := SignTxRequest{transaction: tx, from: acc}
+	req := SignTxRequest{transaction: *tx, from: acc}
 	if len(tx.Data()) > 3 {
 		// Try to make sense of the data
 		var abidata string
@@ -269,10 +270,11 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address,
 
 	if result := <-ch; result.approved {
 		//Sanity check
-		if result.hash != tx.Hash() {
-			return nil, fmt.Errorf("Transaction hash mismatch")
+		if result.transaction.Hash() != tx.Hash() {
+			api.ui.ShowInfo("Transaction modified by UI")
 		}
-		signedTx, err := wallet.SignTxWithPassphrase(acc, result.pw, tx, api.chainID)
+		// The one to sign is the one that was returned from the UI
+		signedTx, err := wallet.SignTxWithPassphrase(acc, result.pw, &result.transaction, api.chainID)
 		if err != nil {
 			api.ui.ShowError(err.Error())
 			return nil, err

cmd/signer/api_test.go

@@ -1,12 +1,14 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/core/types"
 	"io/ioutil"
 	"math/big"
 	"os"
@@ -21,10 +23,17 @@ type HeadlessUI struct {
 }
 
 func (ui *HeadlessUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch chan SignTxResponse) {
-	if "Y" == <-ui.controller {
-		ch <- SignTxResponse{request.transaction.Hash(), true, <-ui.controller}
-	} else {
-		ch <- SignTxResponse{request.transaction.Hash(), false, ""}
+
+	switch <-ui.controller {
+	case "Y":
+		ch <- SignTxResponse{request.transaction, true, <-ui.controller}
+	case "M": //Modify
+		old := request.transaction
+		newVal := big.NewInt(0).Add(old.Value(), big.NewInt(1))
+		tx := types.NewTransaction(old.Nonce(), *old.To(), newVal, old.Gas(), old.GasPrice(), old.Data())
+		ch <- SignTxResponse{*tx, true, <-ui.controller}
+	default:
+		ch <- SignTxResponse{request.transaction, false, ""}
 	}
 }
 func (ui *HeadlessUI) ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan SignDataResponse) {
@@ -250,10 +259,18 @@ func mkTestTx() TransactionArg {
 }
 
 func TestSignTx(t *testing.T) {
+
+	var (
+		list Accounts
+		h    []byte
+		h2   []byte
+		err  error
+	)
+
 	api, control := setup(t)
 	createAccount(control, api, t)
 	control <- "A"
-	list, err := api.List(context.Background())
+	list, err = api.List(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -264,7 +281,7 @@ func TestSignTx(t *testing.T) {
 
 	control <- "Y"
 	control <- "wrongpassword"
-	h, err := api.SignTransaction(context.Background(), a, tx, &methodSig)
+	h, err = api.SignTransaction(context.Background(), a, tx, &methodSig)
 	if h != nil {
 		t.Errorf("Expected nil-data, got %h", h)
 	}
@@ -291,4 +308,28 @@ func TestSignTx(t *testing.T) {
 	if h == nil || len(h) != 118 {
 		t.Errorf("Expected 181 byte rlp-data (got %d bytes)", len(h))
 	}
+	//The tx is NOT modified by the UI
+	control <- "Y"
+	control <- "apassword"
+
+	h2, err = api.SignTransaction(context.Background(), a, tx, &methodSig)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(h, h2) {
+		t.Error("Expected tx to be unmodified by UI")
+	}
+
+	//The tx is modified by the UI
+	control <- "M"
+	control <- "apassword"
+
+	h2, err = api.SignTransaction(context.Background(), a, tx, &methodSig)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if bytes.Equal(h, h2) {
+		t.Error("Expected tx to be modified by UI")
+	}
+
 }

cmd/signer/auditlog.go

@@ -0,0 +1,26 @@
+package main
+
+import (
+	"bufio"
+	"fmt"
+	"github.com/ethereum/go-ethereum/rpc"
+	"io"
+	"time"
+)
+
+type AuditLogger struct {
+	writer *bufio.Writer
+}
+
+func (l AuditLogger) Store(record *rpc.RPCInvocationRecord) {
+	l.writer.WriteString(fmt.Sprintf("%v\n%v\n", time.Now().Format(time.RFC3339), record.Method))
+	for i, arg := range record.Args {
+		l.writer.WriteString(fmt.Sprintf("\t%d: %v\n", i, arg))
+	}
+	l.writer.WriteString(fmt.Sprintf("%v\n", record.Response))
+	l.writer.Flush()
+}
+
+func NewAuditLogger(writer io.Writer) *AuditLogger {
+	return &AuditLogger{bufio.NewWriter(writer)}
+}

cmd/signer/cliui.go

@@ -56,10 +56,13 @@ func (ui *CommandlineUI) readString() string {
 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)
-	}
+	//TODO; remove this, only for debuggging within IDE
+	text := "foobar"
+	//TODO: Use this
+	//	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)
@@ -114,7 +117,7 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch
 	showMetadata(metadata)
 	fmt.Printf("-------------------------------------------\n")
 
-	ch <- SignTxResponse{request.transaction.Hash(), true ,ui.readPassword()}
+	ch <- SignTxResponse{request.transaction, true, ui.readPassword()}
 }
 
 // ApproveSignData prompt the user for confirmation to request to sign data
@@ -157,9 +160,9 @@ 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)
-	if ui.confirm(){
+	if ui.confirm() {
 		ch <- ImportResponse{true, ui.readPasswordText("Old password"), ui.readPasswordText("New password")}
-	}else{
+	} else {
 		ch <- ImportResponse{false, "", ""}
 	}
 }

cmd/signer/main.go

@@ -20,7 +20,6 @@ package main
 
 import (
 	"fmt"
-	"io"
 	"net"
 	"os"
 	"path/filepath"
@@ -70,9 +69,14 @@ func main() {
 		},
 		cli.StringFlag{
 			Name:  "auditlog",
-			Usage: "File used to emit audit logs. Set to '' to disable",
+			Usage: "File used to emit audit logs. Set to \"\" to disable",
 			Value: "audit.log",
 		},
+		cli.StringFlag{
+			Name:  "requestfile",
+			Usage: "File containing requests to handle",
+			Value: "",
+		},
 	}
 
 	app.Action = func(c *cli.Context) error {
@@ -88,6 +92,7 @@ func main() {
 
 		var (
 			server = rpc.NewServer()
+
 			api = NewSignerAPI(
 				c.Int64(utils.NetworkIdFlag.Name),
 				c.String("keystore"),
@@ -95,14 +100,26 @@ func main() {
 				NewCommandlineUI(), db,
 				c.Bool(utils.LightKDFFlag.Name))
 			listener net.Listener
-			//err      error
 		)
-
+		if logfile := c.String("auditlog"); logfile != "" {
+			f, err := os.OpenFile(logfile, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
+			if err != nil {
+				utils.Fatalf("Could not open %v for audit logging", logfile)
+			}
+			server.SetAuditLogger(NewAuditLogger(f))
+			log.Info("Writing audit logs to %v", logfile)
+		}
 		// register signer API with server
 		if err = server.RegisterName("account", api); err != nil {
 			utils.Fatalf("Could not register signer API: %v", err)
 		}
 
+		// Import from file
+		if rfile := c.String("requestfile"); rfile != "" {
+			//Each line of file represents one request
+			log.Warn("Import from file not yet implemented")
+		}
+
 		// start http server
 		endpoint := fmt.Sprintf("%s:%d", c.String(utils.RPCListenAddrFlag.Name), c.Int("rpcport"))
 		if listener, err = net.Listen("tcp", endpoint); err != nil {
@@ -112,6 +129,7 @@ func main() {
 		cors := []string{"*"}
 
 		rpc.NewHTTPServer(cors, server).Serve(listener)
+
 		return nil
 	}
 	app.Run(os.Args)
@@ -127,16 +145,11 @@ func main() {
 // Make transaction
 // safeSend(0x12)
 // 4401a6e40000000000000000000000000000000000000000000000000000000000000012
-// curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_signTransaction","params":["0x82A2A876D39022B3019932D30Cd9c97ad5616813","pw",{"gas":"0x333","gasPrice":"0x123","nonce":"0x0","to":"0x07a565b7ed7d7a678680a4c162885bedbb695fe0", "value":"0x10", "input":"0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"}],"id":67}' http://localhost:8550/
 
-type rwc struct {
-	io.Reader
-	io.Writer
-}
+/*
 
-func (r *rwc) Close() error {
-	if err := os.Stdin.Close(); err != nil {
-		return err
-	}
-	return os.Stdout.Close()
-}
+curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_signTransaction","params":["0x82A2A876D39022B3019932D30Cd9c97ad5616813",{"gas":"0x333","gasPrice":"0x123","nonce":"0x0","to":"0x07a565b7ed7d7a678680a4c162885bedbb695fe0", "value":"0x10", "input":"0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"}],"id":67}' http://localhost:8550/
+
+
+curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_signTransaction","params":["0x82A2A876D39022B3019932D30Cd9c97ad5616813",{"gas":"0x333","gasPrice":"0x123","nonce":"0x0","to":"0x07a565b7ed7d7a678680a4c162885bedbb695fe0", "value":"0x10", "input":"0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"},"test"],"id":67}' http://localhost:8550/
+*/

cmd/signer/types.go

@@ -26,9 +26,9 @@ import (
 
 type Accounts []Account
 
-func (as Accounts) String() string{
+func (as Accounts) String() string {
 	var output []string
-	for _,a := range as{
+	for _, a := range as {
 		output = append(output, a.String())
 	}
 	return strings.Join(output, "\n")