accounts/usbwallet: fix Ledger hidapi/libusb protocol violation

pull/3801/head
Péter Szilágyi 8 years ago
parent 11e7a712f4
commit 26da6daaa9
No known key found for this signature in database
GPG Key ID: E9AE538CEDF8293D
  1. 43
      accounts/usbwallet/ledger_hub.go
  2. 6
      accounts/usbwallet/ledger_wallet.go

@ -22,6 +22,7 @@ package usbwallet
import ( import (
"errors" "errors"
"runtime"
"sync" "sync"
"time" "time"
@ -55,7 +56,9 @@ type LedgerHub struct {
updating bool // Whether the event notification loop is running updating bool // Whether the event notification loop is running
quit chan chan error quit chan chan error
lock sync.RWMutex
stateLock sync.RWMutex // Protects the internals of the hub from racey access
commsLock sync.RWMutex // Allows wallets to lock enumeration (TODO(karalabe): remove if hotplug lands on Windows)
} }
// NewLedgerHub creates a new hardware wallet manager for Ledger devices. // NewLedgerHub creates a new hardware wallet manager for Ledger devices.
@ -76,8 +79,8 @@ func (hub *LedgerHub) Wallets() []accounts.Wallet {
// Make sure the list of wallets is up to date // Make sure the list of wallets is up to date
hub.refreshWallets() hub.refreshWallets()
hub.lock.RLock() hub.stateLock.RLock()
defer hub.lock.RUnlock() defer hub.stateLock.RUnlock()
cpy := make([]accounts.Wallet, len(hub.wallets)) cpy := make([]accounts.Wallet, len(hub.wallets))
copy(cpy, hub.wallets) copy(cpy, hub.wallets)
@ -88,15 +91,25 @@ func (hub *LedgerHub) Wallets() []accounts.Wallet {
// list of wallets based on the found devices. // list of wallets based on the found devices.
func (hub *LedgerHub) refreshWallets() { func (hub *LedgerHub) refreshWallets() {
// Don't scan the USB like crazy it the user fetches wallets in a loop // Don't scan the USB like crazy it the user fetches wallets in a loop
hub.lock.RLock() hub.stateLock.RLock()
elapsed := time.Since(hub.refreshed) elapsed := time.Since(hub.refreshed)
hub.lock.RUnlock() hub.stateLock.RUnlock()
if elapsed < ledgerRefreshThrottling { if elapsed < ledgerRefreshThrottling {
return return
} }
// Retrieve the current list of Ledger devices // Retrieve the current list of Ledger devices
var ledgers []hid.DeviceInfo var ledgers []hid.DeviceInfo
if runtime.GOOS == "linux" {
// hidapi on Linux opens the device during enumeration to retrieve some infos,
// breaking the Ledger protocol if that is waiting for user confirmation. This
// is a bug acknowledged at Ledger, but it won't be fixed on old devices so we
// need to prevent concurrent comms ourselves. The more elegant solution would
// be to ditch enumeration in favor of hutplug events, but that don't work yet
// on Windows so if we need to hack it anyway, this is more elegant for now.
hub.commsLock.Lock()
}
for _, info := range hid.Enumerate(0, 0) { // Can't enumerate directly, one valid ID is the 0 wildcard for _, info := range hid.Enumerate(0, 0) { // Can't enumerate directly, one valid ID is the 0 wildcard
for _, id := range ledgerDeviceIDs { for _, id := range ledgerDeviceIDs {
if info.VendorID == id.Vendor && info.ProductID == id.Product { if info.VendorID == id.Vendor && info.ProductID == id.Product {
@ -105,8 +118,12 @@ func (hub *LedgerHub) refreshWallets() {
} }
} }
} }
if runtime.GOOS == "linux" {
// See rationale before the enumeration why this is needed and only on Linux.
hub.commsLock.Unlock()
}
// Transform the current list of wallets into the new one // Transform the current list of wallets into the new one
hub.lock.Lock() hub.stateLock.Lock()
wallets := make([]accounts.Wallet, 0, len(ledgers)) wallets := make([]accounts.Wallet, 0, len(ledgers))
events := []accounts.WalletEvent{} events := []accounts.WalletEvent{}
@ -121,7 +138,7 @@ func (hub *LedgerHub) refreshWallets() {
} }
// If there are no more wallets or the device is before the next, wrap new wallet // If there are no more wallets or the device is before the next, wrap new wallet
if len(hub.wallets) == 0 || hub.wallets[0].URL().Cmp(url) > 0 { if len(hub.wallets) == 0 || hub.wallets[0].URL().Cmp(url) > 0 {
wallet := &ledgerWallet{url: &url, info: ledger, log: log.New("url", url)} wallet := &ledgerWallet{hub: hub, url: &url, info: ledger, log: log.New("url", url)}
events = append(events, accounts.WalletEvent{Wallet: wallet, Arrive: true}) events = append(events, accounts.WalletEvent{Wallet: wallet, Arrive: true})
wallets = append(wallets, wallet) wallets = append(wallets, wallet)
@ -140,7 +157,7 @@ func (hub *LedgerHub) refreshWallets() {
} }
hub.refreshed = time.Now() hub.refreshed = time.Now()
hub.wallets = wallets hub.wallets = wallets
hub.lock.Unlock() hub.stateLock.Unlock()
// Fire all wallet events and return // Fire all wallet events and return
for _, event := range events { for _, event := range events {
@ -152,8 +169,8 @@ func (hub *LedgerHub) refreshWallets() {
// receive notifications on the addition or removal of Ledger wallets. // receive notifications on the addition or removal of Ledger wallets.
func (hub *LedgerHub) Subscribe(sink chan<- accounts.WalletEvent) event.Subscription { func (hub *LedgerHub) Subscribe(sink chan<- accounts.WalletEvent) event.Subscription {
// We need the mutex to reliably start/stop the update loop // We need the mutex to reliably start/stop the update loop
hub.lock.Lock() hub.stateLock.Lock()
defer hub.lock.Unlock() defer hub.stateLock.Unlock()
// Subscribe the caller and track the subscriber count // Subscribe the caller and track the subscriber count
sub := hub.updateScope.Track(hub.updateFeed.Subscribe(sink)) sub := hub.updateScope.Track(hub.updateFeed.Subscribe(sink))
@ -182,12 +199,12 @@ func (hub *LedgerHub) updater() {
hub.refreshWallets() hub.refreshWallets()
// If all our subscribers left, stop the updater // If all our subscribers left, stop the updater
hub.lock.Lock() hub.stateLock.Lock()
if hub.updateScope.Count() == 0 { if hub.updateScope.Count() == 0 {
hub.updating = false hub.updating = false
hub.lock.Unlock() hub.stateLock.Unlock()
return return
} }
hub.lock.Unlock() hub.stateLock.Unlock()
} }
} }

@ -83,6 +83,7 @@ var errInvalidVersionReply = errors.New("invalid version reply")
// ledgerWallet represents a live USB Ledger hardware wallet. // ledgerWallet represents a live USB Ledger hardware wallet.
type ledgerWallet struct { type ledgerWallet struct {
hub *LedgerHub // USB hub the device originates from (TODO(karalabe): remove if hotplug lands on Windows)
url *accounts.URL // Textual URL uniquely identifying this wallet url *accounts.URL // Textual URL uniquely identifying this wallet
info hid.DeviceInfo // Known USB device infos about the wallet info hid.DeviceInfo // Known USB device infos about the wallet
@ -576,6 +577,11 @@ func (w *ledgerWallet) SignTx(account accounts.Account, tx *types.Transaction, c
<-w.commsLock <-w.commsLock
defer func() { w.commsLock <- struct{}{} }() defer func() { w.commsLock <- struct{}{} }()
// Ensure the device isn't screwed with while user confirmation is pending
// TODO(karalabe): remove if hotplug lands on Windows
w.hub.commsLock.RLock()
defer w.hub.commsLock.RUnlock()
return w.ledgerSign(path, account.Address, tx, chainID) return w.ledgerSign(path, account.Address, tx, chainID)
} }

Loading…
Cancel
Save