clef: bidirectional communication with UI (#19018)

* clef: initial implementation of bidirectional RPC communication for the UI

* signer: fix tests to pass + formatting

* clef: fix unused import + formatting

* signer: gosimple nitpicks

cmd/clef/extapi_changelog.md

 ### Changelog for external API
 
+### 6.0.0
+
+* `New` was changed to deliver only an address, not the full `Account` data
+* `Export` was moved from External API to the UI Server API
+
 #### 5.0.0
 
 * The external `account_EcRecover`-method was reimplemented.

cmd/clef/intapi_changelog.md

 ### Changelog for internal API (ui-api)
 
+### 4.0.0
+
+* Bidirectional communication implemented, so the UI can query `clef` via the stdin/stdout RPC channel. Methods implemented are:
+  - `clef_listWallets` 
+  - `clef_listAccounts`
+  - `clef_listWallets`
+  - `clef_deriveAccount`
+  - `clef_importRawKey`
+  - `clef_openWallet`
+  - `clef_chainId`
+  - `clef_setChainId`
+  - `clef_export`
+  - `clef_import`
+ 
+* The type `Account` was modified (the json-field `type` was removed), to consist of 
+
+```golang
+type Account struct {
+	Address common.Address `json:"address"` // Ethereum account address derived from the key
+	URL     URL            `json:"url"`     // Optional resource locator within a backend
+}
+```
+
+
 ### 3.2.0
 
 * Make `ShowError`, `OnApprovedTx`, `OnSignerStartup` be json-rpc [notifications](https://www.jsonrpc.org/specification#notification):

cmd/clef/main.go

@@ -344,7 +344,7 @@ func signer(c *cli.Context) error {
 		return err
 	}
 	var (
-		ui core.SignerUI
+		ui core.UIClientAPI
 	)
 	if c.GlobalBool(stdiouiFlag.Name) {
 		log.Info("Using stdin/stdout as UI-channel")
@@ -408,18 +408,21 @@ func signer(c *cli.Context) error {
 			}
 		}
 	}
-	log.Info("Starting signer", "chainid", c.GlobalInt64(chainIdFlag.Name),
-		"keystore", c.GlobalString(keystoreFlag.Name),
-		"light-kdf", c.GlobalBool(utils.LightKDFFlag.Name),
-		"advanced", c.GlobalBool(advancedMode.Name))
-
-	apiImpl := core.NewSignerAPI(
-		c.GlobalInt64(chainIdFlag.Name),
-		c.GlobalString(keystoreFlag.Name),
-		c.GlobalBool(utils.NoUSBFlag.Name),
-		ui, db,
-		c.GlobalBool(utils.LightKDFFlag.Name),
-		c.GlobalBool(advancedMode.Name))
+	var (
+		chainId  = c.GlobalInt64(chainIdFlag.Name)
+		ksLoc    = c.GlobalString(keystoreFlag.Name)
+		lightKdf = c.GlobalBool(utils.LightKDFFlag.Name)
+		advanced = c.GlobalBool(advancedMode.Name)
+		nousb    = c.GlobalBool(utils.NoUSBFlag.Name)
+	)
+	log.Info("Starting signer", "chainid", chainId, "keystore", ksLoc,
+		"light-kdf", lightKdf, "advanced", advanced)
+	am := core.StartClefAccountManager(ksLoc, nousb, lightKdf)
+	apiImpl := core.NewSignerAPI(am, chainId, nousb, ui, db, advanced)
+
+	// Establish the bidirectional communication, by creating a new UI backend and registering
+	// it with the UI.
+	ui.RegisterUIServer(core.NewUIServerAPI(apiImpl))
 	api = apiImpl
 	// Audit logging
 	if logfile := c.GlobalString(auditLogFlag.Name); logfile != "" {
@@ -539,7 +542,7 @@ func homeDir() string {
 	}
 	return ""
 }
-func readMasterKey(ctx *cli.Context, ui core.SignerUI) ([]byte, error) {
+func readMasterKey(ctx *cli.Context, ui core.UIClientAPI) ([]byte, error) {
 	var (
 		file      string
 		configDir = ctx.GlobalString(configdirFlag.Name)
@@ -674,10 +677,6 @@ func testExternalUI(api *core.SignerAPI) {
 	checkErr("List", err)
 	_, err = api.New(ctx)
 	checkErr("New", err)
-	_, err = api.Export(ctx, common.Address{})
-	checkErr("Export", err)
-	_, err = api.Import(ctx, json.RawMessage{})
-	checkErr("Import", err)
 
 	api.UI.ShowInfo("Tests completed")
 

signer/core/api_test.go

@@ -28,6 +28,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -47,6 +48,8 @@ func (ui *HeadlessUI) OnInputRequired(info UserInputRequest) (UserInputResponse,
 
 func (ui *HeadlessUI) OnSignerStartup(info StartupInfo) {
 }
+func (ui *HeadlessUI) RegisterUIServer(api *UIServerAPI) {
+}
 
 func (ui *HeadlessUI) OnApprovedTx(tx ethapi.SignTransactionResult) {
 	fmt.Printf("OnApproved()\n")
@@ -91,7 +94,7 @@ func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error)
 	case "A":
 		return ListResponse{request.Accounts}, nil
 	case "1":
-		l := make([]Account, 1)
+		l := make([]accounts.Account, 1)
 		l[0] = request.Accounts[1]
 		return ListResponse{l}, nil
 	default:
@@ -138,13 +141,8 @@ func setup(t *testing.T) (*SignerAPI, chan string) {
 	}
 	var (
 		ui  = &HeadlessUI{controller}
-		api = NewSignerAPI(
-			1,
-			tmpDirName(t),
-			true,
-			ui,
-			db,
-			true, true)
+		am  = StartClefAccountManager(tmpDirName(t), true, true)
+		api = NewSignerAPI(am, 1337, true, ui, db, true)
 	)
 	return api, controller
 }
@@ -169,22 +167,22 @@ func failCreateAccountWithPassword(control chan string, api *SignerAPI, password
 	control <- "Y"
 	control <- password
 
-	acc, err := api.New(context.Background())
+	addr, err := api.New(context.Background())
 	if err == nil {
 		t.Fatal("Should have returned an error")
 	}
-	if acc.Address != (common.Address{}) {
+	if addr != (common.Address{}) {
 		t.Fatal("Empty address should be returned")
 	}
 }
 
 func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
 	control <- "N"
-	acc, err := api.New(context.Background())
+	addr, err := api.New(context.Background())
 	if err != ErrRequestDenied {
 		t.Fatal(err)
 	}
-	if acc.Address != (common.Address{}) {
+	if addr != (common.Address{}) {
 		t.Fatal("Empty address should be returned")
 	}
 }

signer/core/auditlog.go

@@ -18,9 +18,7 @@ package core
 
 import (
 	"context"
-	"encoding/json"
 
-	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/internal/ethapi"
@@ -40,7 +38,7 @@ func (l *AuditLogger) List(ctx context.Context) ([]common.Address, error) {
 	return res, e
 }
 
-func (l *AuditLogger) New(ctx context.Context) (accounts.Account, error) {
+func (l *AuditLogger) New(ctx context.Context) (common.Address, error) {
 	return l.api.New(ctx)
 }
 
@@ -86,15 +84,6 @@ func (l *AuditLogger) EcRecover(ctx context.Context, data hexutil.Bytes, sig hex
 	return b, e
 }
 
-func (l *AuditLogger) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) {
-	l.log.Info("Export", "type", "request", "metadata", MetadataFromContext(ctx).String(),
-		"addr", addr.Hex())
-	j, e := l.api.Export(ctx, addr)
-	// In this case, we don't actually log the json-response, which may be extra sensitive
-	l.log.Info("Export", "type", "response", "json response size", len(j), "error", e)
-	return j, e
-}
-
 func (l *AuditLogger) Version(ctx context.Context) (string, error) {
 	l.log.Info("Version", "type", "request", "metadata", MetadataFromContext(ctx).String())
 	data, err := l.api.Version(ctx)

signer/core/cliui.go

@@ -39,6 +39,10 @@ func NewCommandlineUI() *CommandlineUI {
 	return &CommandlineUI{in: bufio.NewReader(os.Stdin)}
 }
 
+func (ui *CommandlineUI) RegisterUIServer(api *UIServerAPI) {
+	// noop
+}
+
 // readString reads a single line from stdin, trimming if from spaces, enforcing
 // non-emptyness.
 func (ui *CommandlineUI) readString() string {
@@ -223,7 +227,6 @@ func (ui *CommandlineUI) ApproveListing(request *ListRequest) (ListResponse, err
 	for _, account := range request.Accounts {
 		fmt.Printf("  [x] %v\n", account.Address.Hex())
 		fmt.Printf("    URL: %v\n", account.URL)
-		fmt.Printf("    Type: %v\n", account.Typ)
 	}
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(request.Meta)

signer/core/stdioui.go

@@ -32,12 +32,16 @@ type StdIOUI struct {
 }
 
 func NewStdIOUI() *StdIOUI {
-	log.Info("NewStdIOUI")
 	client, err := rpc.DialContext(context.Background(), "stdio://")
 	if err != nil {
 		log.Crit("Could not create stdio client", "err", err)
 	}
-	return &StdIOUI{client: *client}
+	ui := &StdIOUI{client: *client}
+	return ui
+}
+
+func (ui *StdIOUI) RegisterUIServer(api *UIServerAPI) {
+	ui.client.RegisterName("clef", api)
 }
 
 // dispatch sends a request over the stdio

signer/core/types.go

@@ -22,36 +22,11 @@ import (
 	"math/big"
 	"strings"
 
-	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
 )
 
-type Accounts []Account
-
-func (as Accounts) String() string {
-	var output []string
-	for _, a := range as {
-		output = append(output, a.String())
-	}
-	return strings.Join(output, "\n")
-}
-
-type Account struct {
-	Typ     string         `json:"type"`
-	URL     accounts.URL   `json:"url"`
-	Address common.Address `json:"address"`
-}
-
-func (a Account) String() string {
-	s, err := json.Marshal(a)
-	if err == nil {
-		return string(s)
-	}
-	return err.Error()
-}
-
 type ValidationInfo struct {
 	Typ     string `json:"type"`
 	Message string `json:"message"`

signer/core/uiapi.go

+// Copyright 2019 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 core
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/accounts"
+	"github.com/ethereum/go-ethereum/accounts/keystore"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/math"
+	"github.com/ethereum/go-ethereum/crypto"
+)
+
+// SignerUIAPI implements methods Clef provides for a UI to query, in the bidirectional communication
+// channel.
+// This API is considered secure, since a request can only
+// ever arrive from the UI -- and the UI is capable of approving any action, thus we can consider these
+// requests pre-approved.
+// NB: It's very important that these methods are not ever exposed on the external service
+// registry.
+type UIServerAPI struct {
+	extApi *SignerAPI
+	am     *accounts.Manager
+}
+
+// NewUIServerAPI creates a new UIServerAPI
+func NewUIServerAPI(extapi *SignerAPI) *UIServerAPI {
+	return &UIServerAPI{extapi, extapi.am}
+}
+
+// List available accounts. As opposed to the external API definition, this method delivers
+// the full Account object and not only Address.
+// Example call
+// {"jsonrpc":"2.0","method":"clef_listAccounts","params":[], "id":4}
+func (s *UIServerAPI) ListAccounts(ctx context.Context) ([]accounts.Account, error) {
+	var accs []accounts.Account
+	for _, wallet := range s.am.Wallets() {
+		accs = append(accs, wallet.Accounts()...)
+	}
+	return accs, nil
+}
+
+// rawWallet is a JSON representation of an accounts.Wallet interface, with its
+// data contents extracted into plain fields.
+type rawWallet struct {
+	URL      string             `json:"url"`
+	Status   string             `json:"status"`
+	Failure  string             `json:"failure,omitempty"`
+	Accounts []accounts.Account `json:"accounts,omitempty"`
+}
+
+// ListWallets will return a list of wallets that clef manages
+// Example call
+// {"jsonrpc":"2.0","method":"clef_listWallets","params":[], "id":5}
+func (s *UIServerAPI) ListWallets() []rawWallet {
+	wallets := make([]rawWallet, 0) // return [] instead of nil if empty
+	for _, wallet := range s.am.Wallets() {
+		status, failure := wallet.Status()
+
+		raw := rawWallet{
+			URL:      wallet.URL().String(),
+			Status:   status,
+			Accounts: wallet.Accounts(),
+		}
+		if failure != nil {
+			raw.Failure = failure.Error()
+		}
+		wallets = append(wallets, raw)
+	}
+	return wallets
+}
+
+// DeriveAccount requests a HD wallet to derive a new account, optionally pinning
+// it for later reuse.
+// Example call
+// {"jsonrpc":"2.0","method":"clef_deriveAccount","params":["ledger://","m/44'/60'/0'", false], "id":6}
+func (s *UIServerAPI) DeriveAccount(url string, path string, pin *bool) (accounts.Account, error) {
+	wallet, err := s.am.Wallet(url)
+	if err != nil {
+		return accounts.Account{}, err
+	}
+	derivPath, err := accounts.ParseDerivationPath(path)
+	if err != nil {
+		return accounts.Account{}, err
+	}
+	if pin == nil {
+		pin = new(bool)
+	}
+	return wallet.Derive(derivPath, *pin)
+}
+
+// fetchKeystore retrives the encrypted keystore from the account manager.
+func fetchKeystore(am *accounts.Manager) *keystore.KeyStore {
+	return am.Backends(keystore.KeyStoreType)[0].(*keystore.KeyStore)
+}
+
+// ImportRawKey stores the given hex encoded ECDSA key into the key directory,
+// encrypting it with the passphrase.
+// Example call (should fail on password too short)
+// {"jsonrpc":"2.0","method":"clef_importRawKey","params":["1111111111111111111111111111111111111111111111111111111111111111","test"], "id":6}
+func (s *UIServerAPI) ImportRawKey(privkey string, password string) (accounts.Account, error) {
+	key, err := crypto.HexToECDSA(privkey)
+	if err != nil {
+		return accounts.Account{}, err
+	}
+	if err := ValidatePasswordFormat(password); err != nil {
+		return accounts.Account{}, fmt.Errorf("password requirements not met: %v", err)
+	}
+	// No error
+	return fetchKeystore(s.am).ImportECDSA(key, password)
+}
+
+// OpenWallet initiates a hardware wallet opening procedure, establishing a USB
+// connection and attempting to authenticate via the provided passphrase. Note,
+// the method may return an extra challenge requiring a second open (e.g. the
+// Trezor PIN matrix challenge).
+// Example
+// {"jsonrpc":"2.0","method":"clef_openWallet","params":["ledger://",""], "id":6}
+func (s *UIServerAPI) OpenWallet(url string, passphrase *string) error {
+	wallet, err := s.am.Wallet(url)
+	if err != nil {
+		return err
+	}
+	pass := ""
+	if passphrase != nil {
+		pass = *passphrase
+	}
+	return wallet.Open(pass)
+}
+
+// ChainId returns the chainid in use for Eip-155 replay protection
+// Example call
+// {"jsonrpc":"2.0","method":"clef_chainId","params":[], "id":8}
+func (s *UIServerAPI) ChainId() math.HexOrDecimal64 {
+	return (math.HexOrDecimal64)(s.extApi.chainID.Uint64())
+}
+
+// SetChainId sets the chain id to use when signing transactions.
+// Example call to set Ropsten:
+// {"jsonrpc":"2.0","method":"clef_setChainId","params":["3"], "id":8}
+func (s *UIServerAPI) SetChainId(id math.HexOrDecimal64) math.HexOrDecimal64 {
+	s.extApi.chainID = new(big.Int).SetUint64(uint64(id))
+	return s.ChainId()
+}
+
+// Export returns encrypted private key associated with the given address in web3 keystore format.
+// Example
+// {"jsonrpc":"2.0","method":"clef_export","params":["0x19e7e376e7c213b7e7e7e46cc70a5dd086daff2a"], "id":4}
+func (s *UIServerAPI) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) {
+	// Look up the wallet containing the requested signer
+	wallet, err := s.am.Find(accounts.Account{Address: addr})
+	if err != nil {
+		return nil, err
+	}
+	if wallet.URL().Scheme != keystore.KeyStoreScheme {
+		return nil, fmt.Errorf("Account is not a keystore-account")
+	}
+	return ioutil.ReadFile(wallet.URL().Path)
+}
+
+// Import 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.
+// Example (the address in question has privkey `11...11`):
+// {"jsonrpc":"2.0","method":"clef_import","params":[{"address":"19e7e376e7c213b7e7e7e46cc70a5dd086daff2a","crypto":{"cipher":"aes-128-ctr","ciphertext":"33e4cd3756091d037862bb7295e9552424a391a6e003272180a455ca2a9fb332","cipherparams":{"iv":"b54b263e8f89c42bb219b6279fba5cce"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e4ca94644fd30569c1b1afbbc851729953c92637b7fe4bb9840bbb31ffbc64a5"},"mac":"f4092a445c2b21c0ef34f17c9cd0d873702b2869ec5df4439a0c2505823217e7"},"id":"216c7eac-e8c1-49af-a215-fa0036f29141","version":3},"test","yaddayadda"], "id":4}
+func (api *UIServerAPI) Import(ctx context.Context, keyJSON json.RawMessage, oldPassphrase, newPassphrase string) (accounts.Account, error) {
+	be := api.am.Backends(keystore.KeyStoreType)
+
+	if len(be) == 0 {
+		return accounts.Account{}, errors.New("password based accounts not supported")
+	}
+	if err := ValidatePasswordFormat(newPassphrase); err != nil {
+		return accounts.Account{}, fmt.Errorf("password requirements not met: %v", err)
+	}
+	return be[0].(*keystore.KeyStore).Import(keyJSON, oldPassphrase, newPassphrase)
+}
+
+// Other methods to be added, not yet implemented are:
+// - Ruleset interaction: add rules, attest rulefiles
+// - Store metadata about accounts, e.g. naming of accounts

signer/rules/rules.go

@@ -46,16 +46,16 @@ func consoleOutput(call otto.FunctionCall) otto.Value {
 	return otto.Value{}
 }
 
-// rulesetUI provides an implementation of SignerUI that evaluates a javascript
+// rulesetUI provides an implementation of UIClientAPI that evaluates a javascript
 // file for each defined UI-method
 type rulesetUI struct {
-	next        core.SignerUI // The next handler, for manual processing
+	next        core.UIClientAPI // The next handler, for manual processing
 	storage     storage.Storage
 	credentials storage.Storage
 	jsRules     string // The rules to use
 }
 
-func NewRuleEvaluator(next core.SignerUI, jsbackend, credentialsBackend storage.Storage) (*rulesetUI, error) {
+func NewRuleEvaluator(next core.UIClientAPI, jsbackend, credentialsBackend storage.Storage) (*rulesetUI, error) {
 	c := &rulesetUI{
 		next:        next,
 		storage:     jsbackend,
@@ -65,6 +65,9 @@ func NewRuleEvaluator(next core.SignerUI, jsbackend, credentialsBackend storage.
 
 	return c, nil
 }
+func (r *rulesetUI) RegisterUIServer(api *core.UIServerAPI) {
+	// TODO, make it possible to query from js
+}
 
 func (r *rulesetUI) Init(javascriptRules string) error {
 	r.jsRules = javascriptRules

signer/rules/rules_test.go

@@ -77,6 +77,8 @@ type alwaysDenyUI struct{}
 func (alwaysDenyUI) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
 	return core.UserInputResponse{}, nil
 }
+func (alwaysDenyUI) RegisterUIServer(api *core.UIServerAPI) {
+}
 
 func (alwaysDenyUI) OnSignerStartup(info core.StartupInfo) {
 }
@@ -133,11 +135,11 @@ func initRuleEngine(js string) (*rulesetUI, error) {
 }
 
 func TestListRequest(t *testing.T) {
-	accs := make([]core.Account, 5)
+	accs := make([]accounts.Account, 5)
 
 	for i := range accs {
 		addr := fmt.Sprintf("000000000000000000000000000000000000000%x", i)
-		acc := core.Account{
+		acc := accounts.Account{
 			Address: common.BytesToAddress(common.Hex2Bytes(addr)),
 			URL:     accounts.URL{Scheme: "test", Path: fmt.Sprintf("acc-%d", i)},
 		}
@@ -208,6 +210,10 @@ type dummyUI struct {
 	calls []string
 }
 
+func (d *dummyUI) RegisterUIServer(api *core.UIServerAPI) {
+	panic("implement me")
+}
+
 func (d *dummyUI) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
 	d.calls = append(d.calls, "OnInputRequired")
 	return core.UserInputResponse{}, nil
@@ -531,6 +537,8 @@ func (d *dontCallMe) OnInputRequired(info core.UserInputRequest) (core.UserInput
 	d.t.Fatalf("Did not expect next-handler to be called")
 	return core.UserInputResponse{}, nil
 }
+func (d *dontCallMe) RegisterUIServer(api *core.UIServerAPI) {
+}
 
 func (d *dontCallMe) OnSignerStartup(info core.StartupInfo) {
 }