Clef: USB hw wallet support (#17756)

* signer: implement USB interaction with hw wallets

* signer: fix failing testcases

7 files changed, 169 insertions, 3 deletions

diff --git a/cmd/clef/intapi_changelog.md b/cmd/clef/intapi_changelog.md
index 7d2a897ea..9e13f67d0 100644
--- a/cmd/clef/intapi_changelog.md
+++ b/cmd/clef/intapi_changelog.md
@@ -1,5 +1,21 @@
 ### Changelog for internal API (ui-api)
 
+### 2.1.0
+
+* Add `OnInputRequired(info UserInputRequest)` to internal API. This method is used when Clef needs user input, e.g. passwords.
+
+The following structures are used:
+```golang
+       UserInputRequest struct {
+               Prompt     string `json:"prompt"`
+               Title      string `json:"title"`
+               IsPassword bool   `json:"isPassword"`
+       }
+       UserInputResponse struct {
+               Text string `json:"text"`
+       }
+```
+
 ### 2.0.0
 
 * Modify how `call_info` on a transaction is conveyed. New format:
diff --git a/signer/core/api.go b/signer/core/api.go
index 1da86991e..c380fe977 100644
--- a/signer/core/api.go
+++ b/signer/core/api.go
@@ -36,6 +36,9 @@ import (
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
+// numberOfAccountsToDerive For hardware wallets, the number of accounts to derive
+const numberOfAccountsToDerive = 10
+
 // ExternalAPI defines the external API through which signing requests are made.
 type ExternalAPI interface {
 	// List available accounts
@@ -79,6 +82,9 @@ type SignerUI interface {
 	// OnSignerStartup is invoked when the signer boots, and tells the UI info about external API location and version
 	// information
 	OnSignerStartup(info StartupInfo)
+	// OnInputRequried is invoked when clef requires user input, for example master password or
+	// pin-code for unlocking hardware wallets
+	OnInputRequired(info UserInputRequest) (UserInputResponse, error)
 }
 
 // SignerAPI defines the actual implementation of ExternalAPI
@@ -194,6 +200,14 @@ type (
 	StartupInfo struct {
 		Info map[string]interface{} `json:"info"`
 	}
+	UserInputRequest struct {
+		Prompt     string `json:"prompt"`
+		Title      string `json:"title"`
+		IsPassword bool   `json:"isPassword"`
+	}
+	UserInputResponse struct {
+		Text string `json:"text"`
+	}
 )
 
 var ErrRequestDenied = errors.New("Request denied")
@@ -215,6 +229,9 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi
 	if len(ksLocation) > 0 {
 		backends = append(backends, keystore.NewKeyStore(ksLocation, n, p))
 	}
+	if advancedMode {
+		log.Info("Clef is in advanced mode: will warn instead of reject")
+	}
 	if !noUSB {
 		// Start a USB hub for Ledger hardware wallets
 		if ledgerhub, err := usbwallet.NewLedgerHub(); err != nil {
@@ -231,10 +248,94 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi
 			log.Debug("Trezor support enabled")
 		}
 	}
-	if advancedMode {
-		log.Info("Clef is in advanced mode: will warn instead of reject")
+	signer := &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb), !advancedMode}
+	if !noUSB {
+		signer.startUSBListener()
+	}
+	return signer
+}
+func (api *SignerAPI) openTrezor(url accounts.URL) {
+	resp, err := api.UI.OnInputRequired(UserInputRequest{
+		Prompt: "Pin required to open Trezor wallet\n" +
+			"Look at the device for number positions\n\n" +
+			"7 | 8 | 9\n" +
+			"--+---+--\n" +
+			"4 | 5 | 6\n" +
+			"--+---+--\n" +
+			"1 | 2 | 3\n\n",
+		IsPassword: true,
+		Title:      "Trezor unlock",
+	})
+	if err != nil {
+		log.Warn("failed getting trezor pin", "err", err)
+		return
+	}
+	// We're using the URL instead of the pointer to the
+	// Wallet -- perhaps it is not actually present anymore
+	w, err := api.am.Wallet(url.String())
+	if err != nil {
+		log.Warn("wallet unavailable", "url", url)
+		return
+	}
+	err = w.Open(resp.Text)
+	if err != nil {
+		log.Warn("failed to open wallet", "wallet", url, "err", err)
+		return
 	}
-	return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb), !advancedMode}
+
+}
+
+// startUSBListener starts a listener for USB events, for hardware wallet interaction
+func (api *SignerAPI) startUSBListener() {
+	events := make(chan accounts.WalletEvent, 16)
+	am := api.am
+	am.Subscribe(events)
+	go func() {
+
+		// Open any wallets already attached
+		for _, wallet := range am.Wallets() {
+			if err := wallet.Open(""); err != nil {
+				log.Warn("Failed to open wallet", "url", wallet.URL(), "err", err)
+				if err == usbwallet.ErrTrezorPINNeeded {
+					go api.openTrezor(wallet.URL())
+				}
+			}
+		}
+		// Listen for wallet event till termination
+		for event := range events {
+			switch event.Kind {
+			case accounts.WalletArrived:
+				if err := event.Wallet.Open(""); err != nil {
+					log.Warn("New wallet appeared, failed to open", "url", event.Wallet.URL(), "err", err)
+					if err == usbwallet.ErrTrezorPINNeeded {
+						go api.openTrezor(event.Wallet.URL())
+					}
+				}
+			case accounts.WalletOpened:
+				status, _ := event.Wallet.Status()
+				log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status)
+
+				derivationPath := accounts.DefaultBaseDerivationPath
+				if event.Wallet.URL().Scheme == "ledger" {
+					derivationPath = accounts.DefaultLedgerBaseDerivationPath
+				}
+				var nextPath = derivationPath
+				// Derive first N accounts, hardcoded for now
+				for i := 0; i < numberOfAccountsToDerive; i++ {
+					acc, err := event.Wallet.Derive(nextPath, true)
+					if err != nil {
+						log.Warn("account derivation failed", "error", err)
+					} else {
+						log.Info("derived account", "address", acc.Address)
+					}
+					nextPath[len(nextPath)-1]++
+				}
+			case accounts.WalletDropped:
+				log.Info("Old wallet dropped", "url", event.Wallet.URL())
+				event.Wallet.Close()
+			}
+		}
+	}()
 }
 
 // List returns the set of wallet this signer manages. Each wallet can contain
diff --git a/signer/core/api_test.go b/signer/core/api_test.go
index 70aa9aa94..a8aa23896 100644
--- a/signer/core/api_test.go
+++ b/signer/core/api_test.go
@@ -19,6 +19,7 @@ package core
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"math/big"
@@ -41,6 +42,10 @@ type HeadlessUI struct {
 	controller chan string
 }
 
+func (ui *HeadlessUI) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
+	return UserInputResponse{}, errors.New("not implemented")
+}
+
 func (ui *HeadlessUI) OnSignerStartup(info StartupInfo) {
 }
 
diff --git a/signer/core/cliui.go b/signer/core/cliui.go
index cc237612e..940f1f43a 100644
--- a/signer/core/cliui.go
+++ b/signer/core/cliui.go
@@ -83,6 +83,22 @@ func (ui *CommandlineUI) readPasswordText(inputstring string) string {
 	return string(text)
 }
 
+func (ui *CommandlineUI) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
+	fmt.Println(info.Title)
+	fmt.Println(info.Prompt)
+	if info.IsPassword {
+		text, err := terminal.ReadPassword(int(os.Stdin.Fd()))
+		if err != nil {
+			log.Error("Failed to read password", "err", err)
+		}
+		fmt.Println("-----------------------")
+		return UserInputResponse{string(text)}, err
+	}
+	text := ui.readString()
+	fmt.Println("-----------------------")
+	return UserInputResponse{text}, nil
+}
+
 // confirm returns true if user enters 'Yes', otherwise false
 func (ui *CommandlineUI) confirm() bool {
 	fmt.Printf("Approve? [y/N]:\n")
diff --git a/signer/core/stdioui.go b/signer/core/stdioui.go
index 5640ed03b..64032386f 100644
--- a/signer/core/stdioui.go
+++ b/signer/core/stdioui.go
@@ -111,3 +111,11 @@ func (ui *StdIOUI) OnSignerStartup(info StartupInfo) {
 		log.Info("Error calling 'OnSignerStartup'", "exc", err.Error(), "info", info)
 	}
 }
+func (ui *StdIOUI) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
+	var result UserInputResponse
+	err := ui.dispatch("OnInputRequired", info, &result)
+	if err != nil {
+		log.Info("Error calling 'OnInputRequired'", "exc", err.Error(), "info", info)
+	}
+	return result, err
+}
diff --git a/signer/rules/rules.go b/signer/rules/rules.go
index 711e2ddde..07c34db22 100644
--- a/signer/rules/rules.go
+++ b/signer/rules/rules.go
@@ -194,6 +194,11 @@ func (r *rulesetUI) ApproveImport(request *core.ImportRequest) (core.ImportRespo
 	return r.next.ApproveImport(request)
 }
 
+// OnInputRequired not handled by rules
+func (r *rulesetUI) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
+	return r.next.OnInputRequired(info)
+}
+
 func (r *rulesetUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
 	jsonreq, err := json.Marshal(request)
 	approved, err := r.checkApproval("ApproveListing", jsonreq, err)
@@ -222,6 +227,7 @@ func (r *rulesetUI) ShowInfo(message string) {
 	log.Info(message)
 	r.next.ShowInfo(message)
 }
+
 func (r *rulesetUI) OnSignerStartup(info core.StartupInfo) {
 	jsonInfo, err := json.Marshal(info)
 	if err != nil {
diff --git a/signer/rules/rules_test.go b/signer/rules/rules_test.go
index b6060eba7..c2f92d51f 100644
--- a/signer/rules/rules_test.go
+++ b/signer/rules/rules_test.go
@@ -74,6 +74,10 @@ func mixAddr(a string) (*common.MixedcaseAddress, error) {
 
 type alwaysDenyUI struct{}
 
+func (alwaysDenyUI) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
+	return core.UserInputResponse{}, nil
+}
+
 func (alwaysDenyUI) OnSignerStartup(info core.StartupInfo) {
 }
 
@@ -200,6 +204,11 @@ type dummyUI struct {
 	calls []string
 }
 
+func (d *dummyUI) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
+	d.calls = append(d.calls, "OnInputRequired")
+	return core.UserInputResponse{}, nil
+}
+
 func (d *dummyUI) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) {
 	d.calls = append(d.calls, "ApproveTx")
 	return core.SignTxResponse{}, core.ErrRequestDenied
@@ -509,6 +518,11 @@ type dontCallMe struct {
 	t *testing.T
 }
 
+func (d *dontCallMe) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
+	d.t.Fatalf("Did not expect next-handler to be called")
+	return core.UserInputResponse{}, nil
+}
+
 func (d *dontCallMe) OnSignerStartup(info core.StartupInfo) {
 }