From b64086521fbcbdba830699dd2b00b46f1ffb2dde Mon Sep 17 00:00:00 2001 From: Philip Fan Date: Mon, 1 Aug 2022 16:35:42 +0800 Subject: [PATCH] sync-eth: accounts, keystore wallet, upgrade to github.com/karalabe/usb v0.0.2 --- accounts/accounts.go | 91 +++++++-- accounts/hd.go | 59 +++++- accounts/hd_test.go | 41 +++- .../{keystore_wallet.go => wallet.go} | 71 ++++--- accounts/usbwallet/hub.go | 61 +++++- accounts/usbwallet/wallet.go | 181 +++++++++++------- cmd/efsn/config.go | 2 +- cmd/efsn/main.go | 10 +- consensus/datong/consensus.go | 18 +- eth/backend.go | 2 +- go.mod | 2 +- go.sum | 4 +- internal/ethapi/api.go | 18 +- signer/core/api.go | 4 +- 14 files changed, 390 insertions(+), 174 deletions(-) rename accounts/keystore/{keystore_wallet.go => wallet.go} (72%) diff --git a/accounts/accounts.go b/accounts/accounts.go index 917d777e..c0c4cb91 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -18,12 +18,14 @@ package accounts import ( + "fmt" "math/big" ethereum "github.com/FusionFoundation/efsn/v4" "github.com/FusionFoundation/efsn/v4/common" "github.com/FusionFoundation/efsn/v4/core/types" "github.com/FusionFoundation/efsn/v4/event" + "golang.org/x/crypto/sha3" ) // Account represents an Ethereum account located at a specific location defined @@ -33,11 +35,18 @@ type Account struct { URL URL `json:"url"` // Optional resource locator within a backend } +const ( + MimetypeDataWithValidator = "data/validator" + MimetypeTypedData = "data/typed" + MimetypeClique = "application/x-clique-header" + MimetypeTextPlain = "text/plain" +) + // Wallet represents a software or hardware wallet that might contain one or more // accounts (derived from the same seed). type Wallet interface { // URL retrieves the canonical path under which this wallet is reachable. It is - // user by upper layers to define a sorting order over all wallets from multiple + // used by upper layers to define a sorting order over all wallets from multiple // backends. URL() URL @@ -79,26 +88,53 @@ type Wallet interface { // to discover non zero accounts and automatically add them to list of tracked // accounts. // - // Note, self derivaton will increment the last component of the specified path - // opposed to decending into a child path to allow discovering accounts starting + // Note, self derivation will increment the last component of the specified path + // opposed to descending into a child path to allow discovering accounts starting // from non zero components. // + // Some hardware wallets switched derivation paths through their evolution, so + // this method supports providing multiple bases to discover old user accounts + // too. Only the last base will be used to derive the next empty account. + // // You can disable automatic account discovery by calling SelfDerive with a nil // chain state reader. - SelfDerive(base DerivationPath, chain ethereum.ChainStateReader) + SelfDerive(bases []DerivationPath, chain ethereum.ChainStateReader) - // SignHash requests the wallet to sign the given hash. + // SignData requests the wallet to sign the hash of the given data + // It looks up the account specified either solely via its address contained within, + // or optionally with the aid of any location metadata from the embedded URL field. // + // If the wallet requires additional authentication to sign the request (e.g. + // a password to decrypt the account, or a PIN code to verify the transaction), + // an AuthNeededError instance will be returned, containing infos for the user + // about which fields or actions are needed. The user may retry by providing + // the needed details via SignDataWithPassphrase, or by other means (e.g. unlock + // the account in a keystore). + SignData(account Account, mimeType string, data []byte) ([]byte, error) + + // SignDataWithPassphrase is identical to SignData, but also takes a password + // NOTE: there's a chance that an erroneous call might mistake the two strings, and + // supply password in the mimetype field, or vice versa. Thus, an implementation + // should never echo the mimetype or return the mimetype in the error-response + SignDataWithPassphrase(account Account, passphrase, mimeType string, data []byte) ([]byte, error) + + // SignText requests the wallet to sign the hash of a given piece of data, prefixed + // by the Ethereum prefix scheme // It looks up the account specified either solely via its address contained within, // or optionally with the aid of any location metadata from the embedded URL field. // // If the wallet requires additional authentication to sign the request (e.g. - // a password to decrypt the account, or a PIN code o verify the transaction), + // a password to decrypt the account, or a PIN code to verify the transaction), // an AuthNeededError instance will be returned, containing infos for the user // about which fields or actions are needed. The user may retry by providing - // the needed details via SignHashWithPassphrase, or by other means (e.g. unlock + // the needed details via SignTextWithPassphrase, or by other means (e.g. unlock // the account in a keystore). - SignHash(account Account, hash []byte) ([]byte, error) + // + // This method should return the signature in 'canonical' format, with v 0 or 1. + SignText(account Account, text []byte) ([]byte, error) + + // SignTextWithPassphrase is identical to Signtext, but also takes a password + SignTextWithPassphrase(account Account, passphrase string, hash []byte) ([]byte, error) // SignTx requests the wallet to sign the given transaction. // @@ -113,18 +149,7 @@ type Wallet interface { // the account in a keystore). SignTx(account Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) - // SignHashWithPassphrase requests the wallet to sign the given hash with the - // given passphrase as extra authentication information. - // - // It looks up the account specified either solely via its address contained within, - // or optionally with the aid of any location metadata from the embedded URL field. - SignHashWithPassphrase(account Account, passphrase string, hash []byte) ([]byte, error) - - // SignTxWithPassphrase requests the wallet to sign the given transaction, with the - // given passphrase as extra authentication information. - // - // It looks up the account specified either solely via its address contained within, - // or optionally with the aid of any location metadata from the embedded URL field. + // SignTxWithPassphrase is identical to SignTx, but also takes a password SignTxWithPassphrase(account Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) } @@ -148,6 +173,32 @@ type Backend interface { Subscribe(sink chan<- WalletEvent) event.Subscription } +// TextHash is a helper function that calculates a hash for the given message that can be +// safely used to calculate a signature from. +// +// The hash is calculated as +// keccak256("\x19Ethereum Signed Message:\n"${message length}${message}). +// +// This gives context to the signed message and prevents signing of transactions. +func TextHash(data []byte) []byte { + hash, _ := TextAndHash(data) + return hash +} + +// TextAndHash is a helper function that calculates a hash for the given message that can be +// safely used to calculate a signature from. +// +// The hash is calculated as +// keccak256("\x19Ethereum Signed Message:\n"${message length}${message}). +// +// This gives context to the signed message and prevents signing of transactions. +func TextAndHash(data []byte) ([]byte, string) { + msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), string(data)) + hasher := sha3.NewLegacyKeccak256() + hasher.Write([]byte(msg)) + return hasher.Sum(nil), msg +} + // WalletEventType represents the different event types that can be fired by // the wallet subscription subsystem. type WalletEventType int diff --git a/accounts/hd.go b/accounts/hd.go index 277f688e..3009f19b 100644 --- a/accounts/hd.go +++ b/accounts/hd.go @@ -17,6 +17,7 @@ package accounts import ( + "encoding/json" "errors" "fmt" "math" @@ -30,17 +31,17 @@ import ( var DefaultRootDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0} // DefaultBaseDerivationPath is the base path from which custom derivation endpoints -// are incremented. As such, the first account will be at m/44'/60'/0'/0, the second -// at m/44'/60'/0'/1, etc. +// are incremented. As such, the first account will be at m/44'/60'/0'/0/0, the second +// at m/44'/60'/0'/0/1, etc. var DefaultBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0, 0} -// DefaultLedgerBaseDerivationPath is the base path from which custom derivation endpoints -// are incremented. As such, the first account will be at m/44'/60'/0'/0, the second -// at m/44'/60'/0'/1, etc. -var DefaultLedgerBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0} +// LegacyLedgerBaseDerivationPath is the legacy base path from which custom derivation +// endpoints are incremented. As such, the first account will be at m/44'/60'/0'/0, the +// second at m/44'/60'/0'/1, etc. +var LegacyLedgerBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0} // DerivationPath represents the computer friendly version of a hierarchical -// deterministic wallet account derivaion path. +// deterministic wallet account derivation path. // // The BIP-32 spec https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki // defines derivation paths to be of the form: @@ -133,3 +134,47 @@ func (path DerivationPath) String() string { } return result } + +// MarshalJSON turns a derivation path into its json-serialized string +func (path DerivationPath) MarshalJSON() ([]byte, error) { + return json.Marshal(path.String()) +} + +// UnmarshalJSON a json-serialized string back into a derivation path +func (path *DerivationPath) UnmarshalJSON(b []byte) error { + var dp string + var err error + if err = json.Unmarshal(b, &dp); err != nil { + return err + } + *path, err = ParseDerivationPath(dp) + return err +} + +// DefaultIterator creates a BIP-32 path iterator, which progresses by increasing the last component: +// i.e. m/44'/60'/0'/0/0, m/44'/60'/0'/0/1, m/44'/60'/0'/0/2, ... m/44'/60'/0'/0/N. +func DefaultIterator(base DerivationPath) func() DerivationPath { + path := make(DerivationPath, len(base)) + copy(path[:], base[:]) + // Set it back by one, so the first call gives the first result + path[len(path)-1]-- + return func() DerivationPath { + path[len(path)-1]++ + return path + } +} + +// LedgerLiveIterator creates a bip44 path iterator for Ledger Live. +// Ledger Live increments the third component rather than the fifth component +// i.e. m/44'/60'/0'/0/0, m/44'/60'/1'/0/0, m/44'/60'/2'/0/0, ... m/44'/60'/N'/0/0. +func LedgerLiveIterator(base DerivationPath) func() DerivationPath { + path := make(DerivationPath, len(base)) + copy(path[:], base[:]) + // Set it back by one, so the first call gives the first result + path[2]-- + return func() DerivationPath { + // ledgerLivePathIterator iterates on the third component + path[2]++ + return path + } +} diff --git a/accounts/hd_test.go b/accounts/hd_test.go index b6b23230..0743bbe6 100644 --- a/accounts/hd_test.go +++ b/accounts/hd_test.go @@ -17,6 +17,7 @@ package accounts import ( + "fmt" "reflect" "testing" ) @@ -61,7 +62,7 @@ func TestHDPathParsing(t *testing.T) { // Weird inputs just to ensure they work {" m / 44 '\n/\n 60 \n\n\t' /\n0 ' /\t\t 0", DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0}}, - // Invaid derivation paths + // Invalid derivation paths {"", nil}, // Empty relative derivation path {"m", nil}, // Empty absolute derivation path {"m/", nil}, // Missing last derivation component @@ -77,3 +78,41 @@ func TestHDPathParsing(t *testing.T) { } } } + +func testDerive(t *testing.T, next func() DerivationPath, expected []string) { + t.Helper() + for i, want := range expected { + if have := next(); fmt.Sprintf("%v", have) != want { + t.Errorf("step %d, have %v, want %v", i, have, want) + } + } +} + +func TestHdPathIteration(t *testing.T) { + testDerive(t, DefaultIterator(DefaultBaseDerivationPath), + []string{ + "m/44'/60'/0'/0/0", "m/44'/60'/0'/0/1", + "m/44'/60'/0'/0/2", "m/44'/60'/0'/0/3", + "m/44'/60'/0'/0/4", "m/44'/60'/0'/0/5", + "m/44'/60'/0'/0/6", "m/44'/60'/0'/0/7", + "m/44'/60'/0'/0/8", "m/44'/60'/0'/0/9", + }) + + testDerive(t, DefaultIterator(LegacyLedgerBaseDerivationPath), + []string{ + "m/44'/60'/0'/0", "m/44'/60'/0'/1", + "m/44'/60'/0'/2", "m/44'/60'/0'/3", + "m/44'/60'/0'/4", "m/44'/60'/0'/5", + "m/44'/60'/0'/6", "m/44'/60'/0'/7", + "m/44'/60'/0'/8", "m/44'/60'/0'/9", + }) + + testDerive(t, LedgerLiveIterator(DefaultBaseDerivationPath), + []string{ + "m/44'/60'/0'/0/0", "m/44'/60'/1'/0/0", + "m/44'/60'/2'/0/0", "m/44'/60'/3'/0/0", + "m/44'/60'/4'/0/0", "m/44'/60'/5'/0/0", + "m/44'/60'/6'/0/0", "m/44'/60'/7'/0/0", + "m/44'/60'/8'/0/0", "m/44'/60'/9'/0/0", + }) +} diff --git a/accounts/keystore/keystore_wallet.go b/accounts/keystore/wallet.go similarity index 72% rename from accounts/keystore/keystore_wallet.go rename to accounts/keystore/wallet.go index 30a0d8d5..9754deac 100644 --- a/accounts/keystore/keystore_wallet.go +++ b/accounts/keystore/wallet.go @@ -22,6 +22,7 @@ import ( ethereum "github.com/FusionFoundation/efsn/v4" "github.com/FusionFoundation/efsn/v4/accounts" "github.com/FusionFoundation/efsn/v4/core/types" + "github.com/FusionFoundation/efsn/v4/crypto" ) // keystoreWallet implements the accounts.Wallet interface for the original @@ -52,12 +53,12 @@ func (w *keystoreWallet) Status() (string, error) { // is no connection or decryption step necessary to access the list of accounts. func (w *keystoreWallet) Open(passphrase string) error { return nil } -// Close implements accounts.Wallet, but is a noop for plain wallets since is no -// meaningful open operation. +// Close implements accounts.Wallet, but is a noop for plain wallets since there +// is no meaningful open operation. func (w *keystoreWallet) Close() error { return nil } // Accounts implements accounts.Wallet, returning an account list consisting of -// a single account that the plain kestore wallet contains. +// a single account that the plain keystore wallet contains. func (w *keystoreWallet) Accounts() []accounts.Account { return []accounts.Account{w.account} } @@ -76,62 +77,72 @@ func (w *keystoreWallet) Derive(path accounts.DerivationPath, pin bool) (account // SelfDerive implements accounts.Wallet, but is a noop for plain wallets since // there is no notion of hierarchical account derivation for plain keystore accounts. -func (w *keystoreWallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {} +func (w *keystoreWallet) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) { +} -// SignHash implements accounts.Wallet, attempting to sign the given hash with +// signHash attempts to sign the given hash with // the given account. If the wallet does not wrap this particular account, an // error is returned to avoid account leakage (even though in theory we may be // able to sign via our shared keystore backend). -func (w *keystoreWallet) SignHash(account accounts.Account, hash []byte) ([]byte, error) { +func (w *keystoreWallet) signHash(account accounts.Account, hash []byte) ([]byte, error) { // Make sure the requested account is contained within - if account.Address != w.account.Address { - return nil, accounts.ErrUnknownAccount - } - if account.URL != (accounts.URL{}) && account.URL != w.account.URL { + if !w.Contains(account) { return nil, accounts.ErrUnknownAccount } // Account seems valid, request the keystore to sign return w.keystore.SignHash(account, hash) } -// SignTx implements accounts.Wallet, attempting to sign the given transaction -// with the given account. If the wallet does not wrap this particular account, -// an error is returned to avoid account leakage (even though in theory we may -// be able to sign via our shared keystore backend). -func (w *keystoreWallet) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) { +// SignData signs keccak256(data). The mimetype parameter describes the type of data being signed. +func (w *keystoreWallet) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) { + return w.signHash(account, crypto.Keccak256(data)) +} + +// SignDataWithPassphrase signs keccak256(data). The mimetype parameter describes the type of data being signed. +func (w *keystoreWallet) SignDataWithPassphrase(account accounts.Account, passphrase, mimeType string, data []byte) ([]byte, error) { // Make sure the requested account is contained within - if account.Address != w.account.Address { - return nil, accounts.ErrUnknownAccount - } - if account.URL != (accounts.URL{}) && account.URL != w.account.URL { + if !w.Contains(account) { return nil, accounts.ErrUnknownAccount } // Account seems valid, request the keystore to sign - return w.keystore.SignTx(account, tx, chainID) + return w.keystore.SignHashWithPassphrase(account, passphrase, crypto.Keccak256(data)) } -// SignHashWithPassphrase implements accounts.Wallet, attempting to sign the -// given hash with the given account using passphrase as extra authentication. -func (w *keystoreWallet) SignHashWithPassphrase(account accounts.Account, passphrase string, hash []byte) ([]byte, error) { +// SignText implements accounts.Wallet, attempting to sign the hash of +// the given text with the given account. +func (w *keystoreWallet) SignText(account accounts.Account, text []byte) ([]byte, error) { + return w.signHash(account, accounts.TextHash(text)) +} + +// SignTextWithPassphrase implements accounts.Wallet, attempting to sign the +// hash of the given text with the given account using passphrase as extra authentication. +func (w *keystoreWallet) SignTextWithPassphrase(account accounts.Account, passphrase string, text []byte) ([]byte, error) { // Make sure the requested account is contained within - if account.Address != w.account.Address { + if !w.Contains(account) { return nil, accounts.ErrUnknownAccount } - if account.URL != (accounts.URL{}) && account.URL != w.account.URL { + // Account seems valid, request the keystore to sign + return w.keystore.SignHashWithPassphrase(account, passphrase, accounts.TextHash(text)) +} + +// SignTx implements accounts.Wallet, attempting to sign the given transaction +// with the given account. If the wallet does not wrap this particular account, +// an error is returned to avoid account leakage (even though in theory we may +// be able to sign via our shared keystore backend). +func (w *keystoreWallet) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) { + // Make sure the requested account is contained within + if !w.Contains(account) { return nil, accounts.ErrUnknownAccount } // Account seems valid, request the keystore to sign - return w.keystore.SignHashWithPassphrase(account, passphrase, hash) + return w.keystore.SignTx(account, tx, chainID) } // SignTxWithPassphrase implements accounts.Wallet, attempting to sign the given // transaction with the given account using passphrase as extra authentication. func (w *keystoreWallet) SignTxWithPassphrase(account accounts.Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) { // Make sure the requested account is contained within - if account.Address != w.account.Address { - return nil, accounts.ErrUnknownAccount - } - if account.URL != (accounts.URL{}) && account.URL != w.account.URL { + if !w.Contains(account) { return nil, accounts.ErrUnknownAccount } // Account seems valid, request the keystore to sign diff --git a/accounts/usbwallet/hub.go b/accounts/usbwallet/hub.go index 8ef4e38b..a801e80f 100644 --- a/accounts/usbwallet/hub.go +++ b/accounts/usbwallet/hub.go @@ -20,12 +20,13 @@ import ( "errors" "runtime" "sync" + "sync/atomic" "time" "github.com/FusionFoundation/efsn/v4/accounts" "github.com/FusionFoundation/efsn/v4/event" "github.com/FusionFoundation/efsn/v4/log" - "github.com/karalabe/hid" + "github.com/karalabe/usb" ) // LedgerScheme is the protocol scheme prefixing account and wallet URLs. @@ -64,21 +65,41 @@ type Hub struct { // TODO(karalabe): remove if hotplug lands on Windows commsPend int // Number of operations blocking enumeration commsLock sync.Mutex // Lock protecting the pending counter and enumeration + enumFails uint32 // Number of times enumeration has failed } // NewLedgerHub creates a new hardware wallet manager for Ledger devices. func NewLedgerHub() (*Hub, error) { - return newHub(LedgerScheme, 0x2c97, []uint16{0x0000 /* Ledger Blue */, 0x0001 /* Ledger Nano S */}, 0xffa0, 0, newLedgerDriver) + return newHub(LedgerScheme, 0x2c97, []uint16{ + // Original product IDs + 0x0000, /* Ledger Blue */ + 0x0001, /* Ledger Nano S */ + 0x0004, /* Ledger Nano X */ + + // Upcoming product IDs: https://www.ledger.com/2019/05/17/windows-10-update-sunsetting-u2f-tunnel-transport-for-ledger-devices/ + 0x0015, /* HID + U2F + WebUSB Ledger Blue */ + 0x1015, /* HID + U2F + WebUSB Ledger Nano S */ + 0x4015, /* HID + U2F + WebUSB Ledger Nano X */ + 0x0011, /* HID + WebUSB Ledger Blue */ + 0x1011, /* HID + WebUSB Ledger Nano S */ + 0x4011, /* HID + WebUSB Ledger Nano X */ + }, 0xffa0, 0, newLedgerDriver) } -// NewTrezorHub creates a new hardware wallet manager for Trezor devices. -func NewTrezorHub() (*Hub, error) { - return newHub(TrezorScheme, 0x534c, []uint16{0x0001 /* Trezor 1 */}, 0xff00, 0, newTrezorDriver) +// NewTrezorHubWithHID creates a new hardware wallet manager for Trezor devices. +func NewTrezorHubWithHID() (*Hub, error) { + return newHub(TrezorScheme, 0x534c, []uint16{0x0001 /* Trezor HID */}, 0xff00, 0, newTrezorDriver) +} + +// NewTrezorHubWithWebUSB creates a new hardware wallet manager for Trezor devices with +// firmware version > 1.8.0 +func NewTrezorHubWithWebUSB() (*Hub, error) { + return newHub(TrezorScheme, 0x1209, []uint16{0x53c1 /* Trezor WebUSB */}, 0xffff /* No usage id on webusb, don't match unset (0) */, 0, newTrezorDriver) } // newHub creates a new hardware wallet manager for generic USB devices. func newHub(scheme string, vendorID uint16, productIDs []uint16, usageID uint16, endpointID int, makeDriver func(log.Logger) driver) (*Hub, error) { - if !hid.Supported() { + if !usb.Supported() { return nil, errors.New("unsupported platform") } hub := &Hub{ @@ -119,8 +140,12 @@ func (hub *Hub) refreshWallets() { if elapsed < refreshThrottling { return } + // If USB enumeration is continually failing, don't keep trying indefinitely + if atomic.LoadUint32(&hub.enumFails) > 2 { + return + } // Retrieve the current list of USB wallet devices - var devices []hid.DeviceInfo + var devices []usb.DeviceInfo if runtime.GOOS == "linux" { // hidapi on Linux opens the device during enumeration to retrieve some infos, @@ -135,8 +160,22 @@ func (hub *Hub) refreshWallets() { return } } - for _, info := range hid.Enumerate(hub.vendorID, 0) { + infos, err := usb.Enumerate(hub.vendorID, 0) + if err != nil { + failcount := atomic.AddUint32(&hub.enumFails, 1) + if runtime.GOOS == "linux" { + // See rationale before the enumeration why this is needed and only on Linux. + hub.commsLock.Unlock() + } + log.Error("Failed to enumerate USB devices", "hub", hub.scheme, + "vendor", hub.vendorID, "failcount", failcount, "err", err) + return + } + atomic.StoreUint32(&hub.enumFails, 0) + + for _, info := range infos { for _, id := range hub.productIDs { + // Windows and Macos use UsageID matching, Linux uses Interface matching if info.ProductID == id && (info.UsagePage == hub.usageID || info.Interface == hub.endpointID) { devices = append(devices, info) break @@ -150,8 +189,10 @@ func (hub *Hub) refreshWallets() { // Transform the current list of wallets into the new one hub.stateLock.Lock() - wallets := make([]accounts.Wallet, 0, len(devices)) - events := []accounts.WalletEvent{} + var ( + wallets = make([]accounts.Wallet, 0, len(devices)) + events []accounts.WalletEvent + ) for _, device := range devices { url := accounts.URL{Scheme: hub.scheme, Path: device.Path} diff --git a/accounts/usbwallet/wallet.go b/accounts/usbwallet/wallet.go index 37b29be7..6a9ce573 100644 --- a/accounts/usbwallet/wallet.go +++ b/accounts/usbwallet/wallet.go @@ -29,8 +29,9 @@ import ( "github.com/FusionFoundation/efsn/v4/accounts" "github.com/FusionFoundation/efsn/v4/common" "github.com/FusionFoundation/efsn/v4/core/types" + "github.com/FusionFoundation/efsn/v4/crypto" "github.com/FusionFoundation/efsn/v4/log" - "github.com/karalabe/hid" + "github.com/karalabe/usb" ) // Maximum time between wallet health checks to detect USB unplugs. @@ -76,17 +77,17 @@ type wallet struct { driver driver // Hardware implementation of the low level device operations url *accounts.URL // Textual URL uniquely identifying this wallet - info hid.DeviceInfo // Known USB device infos about the wallet - device *hid.Device // USB device advertising itself as a hardware wallet + info usb.DeviceInfo // Known USB device infos about the wallet + device usb.Device // USB device advertising itself as a hardware wallet accounts []accounts.Account // List of derive accounts pinned on the hardware wallet paths map[common.Address]accounts.DerivationPath // Known derivation paths for signing operations - deriveNextPath accounts.DerivationPath // Next derivation path for account auto-discovery - deriveNextAddr common.Address // Next derived account address for auto-discovery - deriveChain ethereum.ChainStateReader // Blockchain state reader to discover used account with - deriveReq chan chan struct{} // Channel to request a self-derivation on - deriveQuit chan chan error // Channel to terminate the self-deriver with + deriveNextPaths []accounts.DerivationPath // Next derivation paths for account auto-discovery (multiple bases supported) + deriveNextAddrs []common.Address // Next derived account addresses for auto-discovery (multiple bases supported) + deriveChain ethereum.ChainStateReader // Blockchain state reader to discover used account with + deriveReq chan chan struct{} // Channel to request a self-derivation on + deriveQuit chan chan error // Channel to terminate the self-deriver with healthQuit chan chan error @@ -273,9 +274,7 @@ func (w *wallet) close() error { w.device = nil w.accounts, w.paths = nil, nil - w.driver.Close() - - return nil + return w.driver.Close() } // Accounts implements accounts.Wallet, returning the list of accounts pinned to @@ -340,57 +339,66 @@ func (w *wallet) selfDerive() { accs []accounts.Account paths []accounts.DerivationPath - nextAddr = w.deriveNextAddr - nextPath = w.deriveNextPath + nextPaths = append([]accounts.DerivationPath{}, w.deriveNextPaths...) + nextAddrs = append([]common.Address{}, w.deriveNextAddrs...) context = context.Background() ) - for empty := false; !empty; { - // Retrieve the next derived Ethereum account - if nextAddr == (common.Address{}) { - if nextAddr, err = w.driver.Derive(nextPath); err != nil { - w.log.Warn("USB wallet account derivation failed", "err", err) + for i := 0; i < len(nextAddrs); i++ { + for empty := false; !empty; { + // Retrieve the next derived Ethereum account + if nextAddrs[i] == (common.Address{}) { + if nextAddrs[i], err = w.driver.Derive(nextPaths[i]); err != nil { + w.log.Warn("USB wallet account derivation failed", "err", err) + break + } + } + // Check the account's status against the current chain state + var ( + balance *big.Int + nonce uint64 + ) + balance, err = w.deriveChain.BalanceAt(context, nextAddrs[i], nil) + if err != nil { + w.log.Warn("USB wallet balance retrieval failed", "err", err) break } - } - // Check the account's status against the current chain state - var ( - balance *big.Int - nonce uint64 - ) - balance, err = w.deriveChain.BalanceAt(context, nextAddr, nil) - if err != nil { - w.log.Warn("USB wallet balance retrieval failed", "err", err) - break - } - nonce, err = w.deriveChain.NonceAt(context, nextAddr, nil) - if err != nil { - w.log.Warn("USB wallet nonce retrieval failed", "err", err) - break - } - // If the next account is empty, stop self-derivation, but add it nonetheless - if balance.Sign() == 0 && nonce == 0 { - empty = true - } - // We've just self-derived a new account, start tracking it locally - path := make(accounts.DerivationPath, len(nextPath)) - copy(path[:], nextPath[:]) - paths = append(paths, path) - - account := accounts.Account{ - Address: nextAddr, - URL: accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)}, - } - accs = append(accs, account) + nonce, err = w.deriveChain.NonceAt(context, nextAddrs[i], nil) + if err != nil { + w.log.Warn("USB wallet nonce retrieval failed", "err", err) + break + } + // We've just self-derived a new account, start tracking it locally + // unless the account was empty. + path := make(accounts.DerivationPath, len(nextPaths[i])) + copy(path[:], nextPaths[i][:]) + if balance.Sign() == 0 && nonce == 0 { + empty = true + // If it indeed was empty, make a log output for it anyway. In the case + // of legacy-ledger, the first account on the legacy-path will + // be shown to the user, even if we don't actively track it + if i < len(nextAddrs)-1 { + w.log.Info("Skipping trakcking first account on legacy path, use personal.deriveAccount(,, false) to track", + "path", path, "address", nextAddrs[i]) + break + } + } + paths = append(paths, path) + account := accounts.Account{ + Address: nextAddrs[i], + URL: accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)}, + } + accs = append(accs, account) - // Display a log message to the user for new (or previously empty accounts) - if _, known := w.paths[nextAddr]; !known || (!empty && nextAddr == w.deriveNextAddr) { - w.log.Info("USB wallet discovered new account", "address", nextAddr, "path", path, "balance", balance, "nonce", nonce) - } - // Fetch the next potential account - if !empty { - nextAddr = common.Address{} - nextPath[len(nextPath)-1]++ + // Display a log message to the user for new (or previously empty accounts) + if _, known := w.paths[nextAddrs[i]]; !known || (!empty && nextAddrs[i] == w.deriveNextAddrs[i]) { + w.log.Info("USB wallet discovered new account", "address", nextAddrs[i], "path", path, "balance", balance, "nonce", nonce) + } + // Fetch the next potential account + if !empty { + nextAddrs[i] = common.Address{} + nextPaths[i][len(nextPaths[i])-1]++ + } } } // Self derivation complete, release device lock @@ -407,8 +415,8 @@ func (w *wallet) selfDerive() { } // Shift the self-derivation forward // TODO(karalabe): don't overwrite changes from wallet.SelfDerive - w.deriveNextAddr = nextAddr - w.deriveNextPath = nextPath + w.deriveNextAddrs = nextAddrs + w.deriveNextPaths = nextPaths w.stateLock.Unlock() // Notify the user of termination and loop after a bit of time (to avoid trashing) @@ -475,32 +483,61 @@ func (w *wallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Accoun if _, ok := w.paths[address]; !ok { w.accounts = append(w.accounts, account) - w.paths[address] = path + w.paths[address] = make(accounts.DerivationPath, len(path)) + copy(w.paths[address], path) } return account, nil } -// SelfDerive implements accounts.Wallet, trying to discover accounts that the -// user used previously (based on the chain state), but ones that he/she did not -// explicitly pin to the wallet manually. To avoid chain head monitoring, self -// derivation only runs during account listing (and even then throttled). -func (w *wallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) { +// SelfDerive sets a base account derivation path from which the wallet attempts +// to discover non zero accounts and automatically add them to list of tracked +// accounts. +// +// Note, self derivation will increment the last component of the specified path +// opposed to decending into a child path to allow discovering accounts starting +// from non zero components. +// +// Some hardware wallets switched derivation paths through their evolution, so +// this method supports providing multiple bases to discover old user accounts +// too. Only the last base will be used to derive the next empty account. +// +// You can disable automatic account discovery by calling SelfDerive with a nil +// chain state reader. +func (w *wallet) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) { w.stateLock.Lock() defer w.stateLock.Unlock() - w.deriveNextPath = make(accounts.DerivationPath, len(base)) - copy(w.deriveNextPath[:], base[:]) - - w.deriveNextAddr = common.Address{} + w.deriveNextPaths = make([]accounts.DerivationPath, len(bases)) + for i, base := range bases { + w.deriveNextPaths[i] = make(accounts.DerivationPath, len(base)) + copy(w.deriveNextPaths[i][:], base[:]) + } + w.deriveNextAddrs = make([]common.Address, len(bases)) w.deriveChain = chain } -// SignHash implements accounts.Wallet, however signing arbitrary data is not +// signHash implements accounts.Wallet, however signing arbitrary data is not // supported for hardware wallets, so this method will always return an error. -func (w *wallet) SignHash(account accounts.Account, hash []byte) ([]byte, error) { +func (w *wallet) signHash(account accounts.Account, hash []byte) ([]byte, error) { return nil, accounts.ErrNotSupported } +// SignData signs keccak256(data). The mimetype parameter describes the type of data being signed +func (w *wallet) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) { + return w.signHash(account, crypto.Keccak256(data)) +} + +// SignDataWithPassphrase implements accounts.Wallet, attempting to sign the given +// data with the given account using passphrase as extra authentication. +// Since USB wallets don't rely on passphrases, these are silently ignored. +func (w *wallet) SignDataWithPassphrase(account accounts.Account, passphrase, mimeType string, data []byte) ([]byte, error) { + return w.SignData(account, mimeType, data) +} + +func (w *wallet) SignText(account accounts.Account, text []byte) ([]byte, error) { + return w.signHash(account, accounts.TextHash(text)) +} + // SignTx implements accounts.Wallet. It sends the transaction over to the Ledger // wallet to request a confirmation from the user. It returns either the signed // transaction or a failure if the user denied the transaction. @@ -550,8 +587,8 @@ func (w *wallet) SignTx(account accounts.Account, tx *types.Transaction, chainID // SignHashWithPassphrase implements accounts.Wallet, however signing arbitrary // data is not supported for Ledger wallets, so this method will always return // an error. -func (w *wallet) SignHashWithPassphrase(account accounts.Account, passphrase string, hash []byte) ([]byte, error) { - return w.SignHash(account, hash) +func (w *wallet) SignTextWithPassphrase(account accounts.Account, passphrase string, text []byte) ([]byte, error) { + return w.SignText(account, accounts.TextHash(text)) } // SignTxWithPassphrase implements accounts.Wallet, attempting to sign the given diff --git a/cmd/efsn/config.go b/cmd/efsn/config.go index a2287ad9..b09b12c7 100644 --- a/cmd/efsn/config.go +++ b/cmd/efsn/config.go @@ -193,7 +193,7 @@ func setAccountManagerBackends(stack *node.Node) error { am.AddBackend(ledgerhub) } // Start a USB hub for Trezor hardware wallets - if trezorhub, err := usbwallet.NewTrezorHub(); err != nil { + if trezorhub, err := usbwallet.NewTrezorHubWithHID(); err != nil { log.Warn(fmt.Sprintf("Failed to start Trezor hub, disabling: %v", err)) } else { am.AddBackend(trezorhub) diff --git a/cmd/efsn/main.go b/cmd/efsn/main.go index 3d7f2eac..063d9b61 100644 --- a/cmd/efsn/main.go +++ b/cmd/efsn/main.go @@ -301,7 +301,7 @@ func startNode(ctx *cli.Context, stack *node.Node, backend ethapi.Backend) { if err != nil { utils.Fatalf("Failed to attach to self: %v", err) } - stateReader := ethclient.NewClient(rpcClient) + ethClient := ethclient.NewClient(rpcClient) // Open any wallets already attached for _, wallet := range stack.AccountManager().Wallets() { @@ -320,11 +320,13 @@ func startNode(ctx *cli.Context, stack *node.Node, backend ethapi.Backend) { status, _ := event.Wallet.Status() log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status) - derivationPath := accounts.DefaultBaseDerivationPath + var derivationPaths []accounts.DerivationPath if event.Wallet.URL().Scheme == "ledger" { - derivationPath = accounts.DefaultLedgerBaseDerivationPath + derivationPaths = append(derivationPaths, accounts.LegacyLedgerBaseDerivationPath) } - event.Wallet.SelfDerive(derivationPath, stateReader) + derivationPaths = append(derivationPaths, accounts.DefaultBaseDerivationPath) + + event.Wallet.SelfDerive(derivationPaths, ethClient) case accounts.WalletDropped: log.Info("Old wallet dropped", "url", event.Wallet.URL()) diff --git a/consensus/datong/consensus.go b/consensus/datong/consensus.go index 33902585..ae05bfd7 100644 --- a/consensus/datong/consensus.go +++ b/consensus/datong/consensus.go @@ -53,7 +53,7 @@ var ( // SignerFn is a signer callback function to request a hash to be signed by a // backing account. -type SignerFn func(accounts.Account, []byte) ([]byte, error) +type SignerFn func(accounts.Account, string, []byte) ([]byte, error) var ( extraVanity = 32 @@ -99,7 +99,7 @@ func (dt *DaTong) Author(header *types.Header) (common.Address, error) { func VerifySignature(header *types.Header) error { signature := header.Extra[len(header.Extra)-extraSeal:] - pubkey, err := crypto.Ecrecover(sigHash(header).Bytes(), signature) + pubkey, err := crypto.Ecrecover(sigHash(header), signature) if err != nil { return err } @@ -497,7 +497,7 @@ func (dt *DaTong) Seal(chain consensus.ChainReader, block *types.Block, results return errc } - sighash, err := signFn(accounts.Account{Address: header.Coinbase}, sigHash(header).Bytes()) + sighash, err := signFn(accounts.Account{Address: header.Coinbase}, "", sigRlp(header)) if err != nil { return err } @@ -746,9 +746,8 @@ func (dt *DaTong) getAllTickets(chain consensus.ChainReader, header *types.Heade return tickets, nil } -func sigHash(header *types.Header) (hash common.Hash) { - hasher := sha3.NewLegacyKeccak256() - rlp.Encode(hasher, []interface{}{ +func sigRlp(header *types.Header) (result []byte) { + result, _ = rlp.EncodeToBytes([]interface{}{ header.ParentHash, header.UncleHash, header.Coinbase, @@ -765,8 +764,11 @@ func sigHash(header *types.Header) (hash common.Hash) { header.MixDigest, header.Nonce, }) - hasher.Sum(hash[:0]) - return hash + return +} + +func sigHash(header *types.Header) (result []byte) { + return crypto.Keccak256(sigRlp(header)) } func getSnapDataByHeader(header *types.Header) []byte { diff --git a/eth/backend.go b/eth/backend.go index 1d4ab223..cb0d89cc 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -384,7 +384,7 @@ func (s *Ethereum) StartMining(threads int) error { log.Error("Etherbase account unavailable locally", "err", err) return fmt.Errorf("signer missing: %v", err) } - datong.Authorize(eb, wallet.SignHash) + datong.Authorize(eb, wallet.SignData) } // If mining is started, we can disable the transaction rejection mechanism diff --git a/go.mod b/go.mod index 2f09ec17..a63ab590 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( github.com/influxdata/influxdb v1.8.3 github.com/jackpal/go-nat-pmp v1.0.2 github.com/julienschmidt/httprouter v1.2.0 - github.com/karalabe/hid v0.0.0-20180420081245-2b4488a37358 + github.com/karalabe/usb v0.0.2 github.com/mattn/go-colorable v0.1.0 github.com/mattn/go-isatty v0.0.5-0.20180830101745-3fb116b82035 github.com/naoina/go-stringutil v0.1.0 // indirect diff --git a/go.sum b/go.sum index d2d0d14f..82f881c8 100644 --- a/go.sum +++ b/go.sum @@ -194,8 +194,8 @@ github.com/julienschmidt/httprouter v1.2.0 h1:TDTW5Yz1mjftljbcKqRcrYhd4XeOoI98t+ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/jwilder/encoding v0.0.0-20170811194829-b4e1701a28ef/go.mod h1:Ct9fl0F6iIOGgxJ5npU/IUOhOhqlVrGjyIZc8/MagT0= -github.com/karalabe/hid v0.0.0-20180420081245-2b4488a37358 h1:FVFwfCq+MMGoSohqKWiJwMy3FMZSM+vA0SrACbrFx1Y= -github.com/karalabe/hid v0.0.0-20180420081245-2b4488a37358/go.mod h1:YvbcH+3Wo6XPs9nkgTY3u19KXLauXW+J5nB7hEHuX0A= +github.com/karalabe/usb v0.0.2 h1:M6QQBNxF+CQ8OFvxrT90BA0qBOXymndZnk5q235mFc4= +github.com/karalabe/usb v0.0.2/go.mod h1:Od972xHfMJowv7NGVDiWVxk2zxnWgjLlJzE+F4F7AGU= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.4.0/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 5dce5c3b..9a71aeac 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -421,18 +421,6 @@ func (s *PrivateAccountAPI) SignTransaction(ctx context.Context, args Transactio return &SignTransactionResult{data, signed}, nil } -// signHash is a helper function that calculates a hash for the given message that can be -// safely used to calculate a signature from. -// -// The hash is calulcated as -// keccak256("\x19Ethereum Signed Message:\n"${message length}${message}). -// -// This gives context to the signed message and prevents signing of transactions. -func signHash(data []byte) []byte { - msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data) - return crypto.Keccak256([]byte(msg)) -} - // Sign calculates an Ethereum ECDSA signature for: // keccack256("\x19Ethereum Signed Message:\n" + len(message) + message)) // @@ -451,7 +439,7 @@ func (s *PrivateAccountAPI) Sign(ctx context.Context, data hexutil.Bytes, addr c return nil, err } // Assemble sign the data with the wallet - signature, err := wallet.SignHashWithPassphrase(account, passwd, signHash(data)) + signature, err := wallet.SignTextWithPassphrase(account, passwd, data) if err != nil { return nil, err } @@ -478,7 +466,7 @@ func (s *PrivateAccountAPI) EcRecover(ctx context.Context, data, sig hexutil.Byt } sig[64] -= 27 // Transform yellow paper V from 27/28 to 0/1 - rpk, err := crypto.SigToPub(signHash(data), sig) + rpk, err := crypto.SigToPub(accounts.TextHash(data), sig) if err != nil { return common.Address{}, err } @@ -1456,7 +1444,7 @@ func (s *PublicTransactionPoolAPI) Sign(addr common.Address, data hexutil.Bytes) return nil, err } // Sign the requested hash with the wallet - signature, err := wallet.SignHash(account, signHash(data)) + signature, err := wallet.SignText(account, data) if err == nil { signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper } diff --git a/signer/core/api.go b/signer/core/api.go index cb9f4cf2..487727b9 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -224,7 +224,7 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi log.Debug("Ledger support enabled") } // Start a USB hub for Trezor hardware wallets - if trezorhub, err := usbwallet.NewTrezorHub(); err != nil { + if trezorhub, err := usbwallet.NewTrezorHubWithHID(); err != nil { log.Warn(fmt.Sprintf("Failed to start Trezor hub, disabling: %v", err)) } else { backends = append(backends, trezorhub) @@ -404,7 +404,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, da return nil, err } // Assemble sign the data with the wallet - signature, err := wallet.SignHashWithPassphrase(account, res.Password, sighash) + signature, err := wallet.SignDataWithPassphrase(account, res.Password, "", data) if err != nil { api.UI.ShowError(err.Error()) return nil, err