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

@@ -1,5 +1,10 @@
 ### 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

@@ -1,5 +1,29 @@
 ### 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.go

@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"math/big"
 	"reflect"
 
@@ -39,17 +38,17 @@ const (
 	// numberOfAccountsToDerive For hardware wallets, the number of accounts to derive
 	numberOfAccountsToDerive = 10
 	// ExternalAPIVersion -- see extapi_changelog.md
-	ExternalAPIVersion = "5.0.0"
+	ExternalAPIVersion = "6.0.0"
 	// InternalAPIVersion -- see intapi_changelog.md
-	InternalAPIVersion = "3.2.0"
+	InternalAPIVersion = "4.0.0"
 )
 
 // ExternalAPI defines the external API through which signing requests are made.
 type ExternalAPI interface {
 	// List available accounts
 	List(ctx context.Context) ([]common.Address, error)
 	// New request to create a new account
-	New(ctx context.Context) (accounts.Account, error)
+	New(ctx context.Context) (common.Address, error)
 	// SignTransaction request to sign the specified transaction
 	SignTransaction(ctx context.Context, args SendTxArgs, methodSelector *string) (*ethapi.SignTransactionResult, error)
 	// SignData - request to sign the given data (plus prefix)
@@ -58,17 +57,13 @@ type ExternalAPI interface {
 	SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error)
 	// EcRecover - recover public key from given message and signature
 	EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error)
-	// Export - request to export an account
-	Export(ctx context.Context, addr common.Address) (json.RawMessage, error)
-	// Import - request to import an account
-	// Should be moved to Internal API, in next phase when we have
-	// bi-directional communication
-	//Import(ctx context.Context, keyJSON json.RawMessage) (Account, error)
+	// Version info about the APIs
 	Version(ctx context.Context) (string, error)
 }
 
-// SignerUI specifies what method a UI needs to implement to be able to be used as a UI for the signer
-type SignerUI interface {
+// UIClientAPI specifies what method a UI needs to implement to be able to be used as a
+// UI for the signer
+type UIClientAPI interface {
 	// ApproveTx prompt the user for confirmation to request to sign Transaction
 	ApproveTx(request *SignTxRequest) (SignTxResponse, error)
 	// ApproveSignData prompt the user for confirmation to request to sign data
@@ -95,13 +90,15 @@ type SignerUI interface {
 	// OnInputRequired is invoked when clef requires user input, for example master password or
 	// pin-code for unlocking hardware wallets
 	OnInputRequired(info UserInputRequest) (UserInputResponse, error)
+	// RegisterUIServer tells the UI to use the given UIServerAPI for ui->clef communication
+	RegisterUIServer(api *UIServerAPI)
 }
 
 // SignerAPI defines the actual implementation of ExternalAPI
 type SignerAPI struct {
 	chainID    *big.Int
 	am         *accounts.Manager
-	UI         SignerUI
+	UI         UIClientAPI
 	validator  *Validator
 	rejectMode bool
 }
@@ -115,6 +112,37 @@ type Metadata struct {
 	Origin    string `json:"Origin"`
 }
 
+func StartClefAccountManager(ksLocation string, nousb, lightKDF bool) *accounts.Manager {
+	var (
+		backends []accounts.Backend
+		n, p     = keystore.StandardScryptN, keystore.StandardScryptP
+	)
+	if lightKDF {
+		n, p = keystore.LightScryptN, keystore.LightScryptP
+	}
+	// support password based accounts
+	if len(ksLocation) > 0 {
+		backends = append(backends, keystore.NewKeyStore(ksLocation, n, p))
+	}
+	if !nousb {
+		// Start a USB hub for Ledger hardware wallets
+		if ledgerhub, err := usbwallet.NewLedgerHub(); err != nil {
+			log.Warn(fmt.Sprintf("Failed to start Ledger hub, disabling: %v", err))
+		} else {
+			backends = append(backends, ledgerhub)
+			log.Debug("Ledger support enabled")
+		}
+		// Start a USB hub for Trezor hardware wallets
+		if trezorhub, err := usbwallet.NewTrezorHub(); err != nil {
+			log.Warn(fmt.Sprintf("Failed to start Trezor hub, disabling: %v", err))
+		} else {
+			backends = append(backends, trezorhub)
+			log.Debug("Trezor support enabled")
+		}
+	}
+	return accounts.NewManager(backends...)
+}
+
 // MetadataFromContext extracts Metadata from a given context.Context
 func MetadataFromContext(ctx context.Context) Metadata {
 	m := Metadata{"NA", "NA", "NA", "", ""} // batman
@@ -199,11 +227,11 @@ type (
 		Password string `json:"password"`
 	}
 	ListRequest struct {
-		Accounts []Account `json:"accounts"`
-		Meta     Metadata  `json:"meta"`
+		Accounts []accounts.Account `json:"accounts"`
+		Meta     Metadata           `json:"meta"`
 	}
 	ListResponse struct {
-		Accounts []Account `json:"accounts"`
+		Accounts []accounts.Account `json:"accounts"`
 	}
 	Message struct {
 		Text string `json:"text"`
@@ -234,38 +262,11 @@ var ErrRequestDenied = errors.New("Request denied")
 // 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, lightKDF bool, advancedMode bool) *SignerAPI {
-	var (
-		backends []accounts.Backend
-		n, p     = keystore.StandardScryptN, keystore.StandardScryptP
-	)
-	if lightKDF {
-		n, p = keystore.LightScryptN, keystore.LightScryptP
-	}
-	// support password based accounts
-	if len(ksLocation) > 0 {
-		backends = append(backends, keystore.NewKeyStore(ksLocation, n, p))
-	}
+func NewSignerAPI(am *accounts.Manager, chainID int64, noUSB bool, ui UIClientAPI, abidb *AbiDb, advancedMode bool) *SignerAPI {
 	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 {
-			log.Warn(fmt.Sprintf("Failed to start Ledger hub, disabling: %v", err))
-		} else {
-			backends = append(backends, ledgerhub)
-			log.Debug("Ledger support enabled")
-		}
-		// Start a USB hub for Trezor hardware wallets
-		if trezorhub, err := usbwallet.NewTrezorHub(); err != nil {
-			log.Warn(fmt.Sprintf("Failed to start Trezor hub, disabling: %v", err))
-		} else {
-			backends = append(backends, trezorhub)
-			log.Debug("Trezor support enabled")
-		}
-	}
-	signer := &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb), !advancedMode}
+	signer := &SignerAPI{big.NewInt(chainID), am, ui, NewValidator(abidb), !advancedMode}
 	if !noUSB {
 		signer.startUSBListener()
 	}
@@ -358,12 +359,9 @@ func (api *SignerAPI) startUSBListener() {
 // List returns the set of wallet this signer manages. Each wallet can contain
 // multiple accounts.
 func (api *SignerAPI) List(ctx context.Context) ([]common.Address, error) {
-	var accs []Account
+	var accs []accounts.Account
 	for _, wallet := range api.am.Wallets() {
-		for _, acc := range wallet.Accounts() {
-			acc := Account{Typ: "Account", URL: wallet.URL(), Address: acc.Address}
-			accs = append(accs, acc)
-		}
+		accs = append(accs, wallet.Accounts()...)
 	}
 	result, err := api.UI.ApproveListing(&ListRequest{Accounts: accs, Meta: MetadataFromContext(ctx)})
 	if err != nil {
@@ -373,7 +371,6 @@ func (api *SignerAPI) List(ctx context.Context) ([]common.Address, error) {
 		return nil, ErrRequestDenied
 
 	}
-
 	addresses := make([]common.Address, 0)
 	for _, acc := range result.Accounts {
 		addresses = append(addresses, acc.Address)
@@ -385,10 +382,10 @@ func (api *SignerAPI) List(ctx context.Context) ([]common.Address, error) {
 // New creates a new password protected Account. The private key is protected with
 // the given password. Users are responsible to backup the private key that is stored
 // in the keystore location thas was specified when this API was created.
-func (api *SignerAPI) New(ctx context.Context) (accounts.Account, error) {
+func (api *SignerAPI) New(ctx context.Context) (common.Address, error) {
 	be := api.am.Backends(keystore.KeyStoreType)
 	if len(be) == 0 {
-		return accounts.Account{}, errors.New("password based accounts not supported")
+		return common.Address{}, errors.New("password based accounts not supported")
 	}
 	var (
 		resp NewAccountResponse
@@ -398,20 +395,21 @@ func (api *SignerAPI) New(ctx context.Context) (accounts.Account, error) {
 	for i := 0; i < 3; i++ {
 		resp, err = api.UI.ApproveNewAccount(&NewAccountRequest{MetadataFromContext(ctx)})
 		if err != nil {
-			return accounts.Account{}, err
+			return common.Address{}, err
 		}
 		if !resp.Approved {
-			return accounts.Account{}, ErrRequestDenied
+			return common.Address{}, ErrRequestDenied
 		}
 		if pwErr := ValidatePasswordFormat(resp.Password); pwErr != nil {
 			api.UI.ShowError(fmt.Sprintf("Account creation attempt #%d failed due to password requirements: %v", (i + 1), pwErr))
 		} else {
 			// No error
-			return be[0].(*keystore.KeyStore).NewAccount(resp.Password)
+			acc, err := be[0].(*keystore.KeyStore).NewAccount(resp.Password)
+			return acc.Address, err
 		}
 	}
 	// Otherwise fail, with generic error message
-	return accounts.Account{}, errors.New("account creation failed")
+	return common.Address{}, errors.New("account creation failed")
 }
 
 // logDiff logs the difference between the incoming (original) transaction and the one returned from the signer.
@@ -521,57 +519,6 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth
 
 }
 
-// Export returns encrypted private key associated with the given address in web3 keystore format.
-func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) {
-	res, err := api.UI.ApproveExport(&ExportRequest{Address: addr, Meta: MetadataFromContext(ctx)})
-
-	if err != nil {
-		return nil, err
-	}
-	if !res.Approved {
-		return nil, ErrRequestDenied
-	}
-	// Look up the wallet containing the requested signer
-	wallet, err := api.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.
-// OBS! This method is removed from the public API. It should not be exposed on the external API
-// for a couple of reasons:
-// 1. Even though it is encrypted, it should still be seen as sensitive data
-// 2. It can be used to DoS clef, by using malicious data with e.g. extreme large
-// values for the kdfparams.
-func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) {
-	be := api.am.Backends(keystore.KeyStoreType)
-
-	if len(be) == 0 {
-		return Account{}, errors.New("password based accounts not supported")
-	}
-	res, err := api.UI.ApproveImport(&ImportRequest{Meta: MetadataFromContext(ctx)})
-
-	if err != nil {
-		return Account{}, err
-	}
-	if !res.Approved {
-		return Account{}, ErrRequestDenied
-	}
-	acc, err := be[0].(*keystore.KeyStore).Import(keyJSON, res.OldPassword, res.NewPassword)
-	if err != nil {
-		api.UI.ShowError(err.Error())
-		return Account{}, err
-	}
-	return Account{Typ: "Account", URL: acc.URL, Address: acc.Address}, nil
-}
-
 // Returns the external api version. This method does not require user acceptance. Available methods are
 // available via enumeration anyway, and this info does not contain user-specific data
 func (api *SignerAPI) Version(ctx context.Context) (string, error) {