Hardware-wallet/usb-subscribe-refactor (#7860)

* Hardware-wallet fix

* More fine-grained initilization of callbacks by vendorID, productID and usb class
* Each device manufacturer gets a seperate handle thread each
* Replaced "dummy for loop" with a delay to wait for the device to boot-up properly
* Haven't been very carefully with checking dependencies cycles etc
* Inline comments explaining where shortcuts have been taken
* Need to test this on Windows machine and with Ledger (both models)

Signed-off-by: niklasad1 <niklasadolfsson1@gmail.com>

* Validate product_id of detected ledger devices

* closed_device => unlocked_device

* address comments

* add target in debug

* Address feedback

* Remove thread joining in HardwareWalletManager
* Remove thread handlers in HardwareWalletManager because this makes them unused
This commit is contained in:
Niklas Adolfsson 2018-02-27 16:45:16 +01:00 committed by Marek Kotewicz
parent eff5cabb3f
commit 6445f12c2d
3 changed files with 180 additions and 97 deletions

View File

@ -20,18 +20,23 @@
use std::cmp::min; use std::cmp::min;
use std::fmt; use std::fmt;
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::{Arc, Weak};
use std::time::Duration; use std::time::Duration;
use std::thread;
use ethereum_types::{H256, Address}; use ethereum_types::{H256, Address};
use ethkey::Signature; use ethkey::Signature;
use hidapi; use hidapi;
use libusb;
use parking_lot::{Mutex, RwLock}; use parking_lot::{Mutex, RwLock};
use super::{WalletInfo, KeyPath}; use super::{WalletInfo, KeyPath};
const LEDGER_VID: u16 = 0x2c97; /// Ledger vendor ID
const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001]; // Nano S and Blue pub const LEDGER_VID: u16 = 0x2c97;
/// Legder product IDs: [Nano S and Blue]
pub const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001];
const ETH_DERIVATION_PATH_BE: [u8; 17] = [4, 0x80, 0, 0, 44, 0x80, 0, 0, 60, 0x80, 0, 0, 0, 0, 0, 0, 0]; // 44'/60'/0'/0 const ETH_DERIVATION_PATH_BE: [u8; 17] = [4, 0x80, 0, 0, 44, 0x80, 0, 0, 60, 0x80, 0, 0, 0, 0, 0, 0, 0]; // 44'/60'/0'/0
const ETC_DERIVATION_PATH_BE: [u8; 21] = [5, 0x80, 0, 0, 44, 0x80, 0, 0, 60, 0x80, 0x02, 0x73, 0xd0, 0x80, 0, 0, 0, 0, 0, 0, 0]; // 44'/60'/160720'/0'/0 const ETC_DERIVATION_PATH_BE: [u8; 21] = [5, 0x80, 0, 0, 44, 0x80, 0, 0, 60, 0x80, 0x02, 0x73, 0xd0, 0x80, 0, 0, 0, 0, 0, 0, 0]; // 44'/60'/160720'/0'/0
@ -54,10 +59,14 @@ pub enum Error {
Protocol(&'static str), Protocol(&'static str),
/// Hidapi error. /// Hidapi error.
Usb(hidapi::HidError), Usb(hidapi::HidError),
/// Libusb error
LibUsb(libusb::Error),
/// Device with request key is not available. /// Device with request key is not available.
KeyNotFound, KeyNotFound,
/// Signing has been cancelled by user. /// Signing has been cancelled by user.
UserCancel, UserCancel,
/// Invalid Device
InvalidDevice,
} }
impl fmt::Display for Error { impl fmt::Display for Error {
@ -65,8 +74,10 @@ impl fmt::Display for Error {
match *self { match *self {
Error::Protocol(ref s) => write!(f, "Ledger protocol error: {}", s), Error::Protocol(ref s) => write!(f, "Ledger protocol error: {}", s),
Error::Usb(ref e) => write!(f, "USB communication error: {}", e), Error::Usb(ref e) => write!(f, "USB communication error: {}", e),
Error::LibUsb(ref e) => write!(f, "LibUSB communication error: {}", e),
Error::KeyNotFound => write!(f, "Key not found"), Error::KeyNotFound => write!(f, "Key not found"),
Error::UserCancel => write!(f, "Operation has been cancelled"), Error::UserCancel => write!(f, "Operation has been cancelled"),
Error::InvalidDevice => write!(f, "Unsupported product was entered"),
} }
} }
} }
@ -77,6 +88,12 @@ impl From<hidapi::HidError> for Error {
} }
} }
impl From<libusb::Error> for Error {
fn from(err: libusb::Error) -> Error {
Error::LibUsb(err)
}
}
/// Ledger device manager. /// Ledger device manager.
pub struct Manager { pub struct Manager {
usb: Arc<Mutex<hidapi::HidApi>>, usb: Arc<Mutex<hidapi::HidApi>>,
@ -234,16 +251,7 @@ impl Manager {
fn open_path<R, F>(&self, f: F) -> Result<R, Error> fn open_path<R, F>(&self, f: F) -> Result<R, Error>
where F: Fn() -> Result<R, &'static str> where F: Fn() -> Result<R, &'static str>
{ {
let mut err = Error::KeyNotFound; f().map_err(Into::into)
// Try to open device a few times.
for _ in 0..10 {
match f() {
Ok(handle) => return Ok(handle),
Err(e) => err = From::from(e),
}
::std::thread::sleep(Duration::from_millis(200));
}
Err(err)
} }
fn send_apdu(handle: &hidapi::HidDevice, command: u8, p1: u8, p2: u8, data: &[u8]) -> Result<Vec<u8>, Error> { fn send_apdu(handle: &hidapi::HidDevice, command: u8, p1: u8, p2: u8, data: &[u8]) -> Result<Vec<u8>, Error> {
@ -333,6 +341,54 @@ impl Manager {
message.truncate(new_len); message.truncate(new_len);
Ok(message) Ok(message)
} }
fn is_valid_ledger(device: &libusb::Device) -> Result<(), Error> {
let desc = device.device_descriptor()?;
let vendor_id = desc.vendor_id();
let product_id = desc.product_id();
if vendor_id == LEDGER_VID && LEDGER_PIDS.contains(&product_id) {
Ok(())
} else {
Err(Error::InvalidDevice)
}
}
}
/// Ledger event handler
/// A seperate thread is handling incoming events
pub struct EventHandler {
ledger: Weak<Manager>,
}
impl EventHandler {
/// Ledger event handler constructor
pub fn new(ledger: Weak<Manager>) -> Self {
Self { ledger: ledger }
}
}
impl libusb::Hotplug for EventHandler {
fn device_arrived(&mut self, device: libusb::Device) {
if let (Some(ledger), Ok(_)) = (self.ledger.upgrade(), Manager::is_valid_ledger(&device)) {
debug!(target: "hw", "Ledger arrived");
// Wait for the device to boot up
thread::sleep(Duration::from_millis(1000));
if let Err(e) = ledger.update_devices() {
debug!(target: "hw", "Ledger connect error: {:?}", e);
}
}
}
fn device_left(&mut self, device: libusb::Device) {
if let (Some(ledger), Ok(_)) = (self.ledger.upgrade(), Manager::is_valid_ledger(&device)) {
debug!(target: "hw", "Ledger left");
if let Err(e) = ledger.update_devices() {
debug!(target: "hw", "Ledger disconnect error: {:?}", e);
}
}
}
} }
#[test] #[test]

View File

@ -33,13 +33,15 @@ use ethkey::{Address, Signature};
use parking_lot::Mutex; use parking_lot::Mutex;
use std::fmt; use std::fmt;
use std::sync::{Arc, Weak}; use std::sync::Arc;
use std::sync::atomic; use std::sync::atomic;
use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicBool;
use std::thread; use std::thread;
use std::time::Duration; use std::time::Duration;
use ethereum_types::U256; use ethereum_types::U256;
const USB_DEVICE_CLASS_DEVICE: u8 = 0;
/// Hardware wallet error. /// Hardware wallet error.
#[derive(Debug)] #[derive(Debug)]
pub enum Error { pub enum Error {
@ -128,84 +130,78 @@ impl From<libusb::Error> for Error {
/// Hardware wallet management interface. /// Hardware wallet management interface.
pub struct HardwareWalletManager { pub struct HardwareWalletManager {
update_thread: Option<thread::JoinHandle<()>>,
exiting: Arc<AtomicBool>, exiting: Arc<AtomicBool>,
ledger: Arc<ledger::Manager>, ledger: Arc<ledger::Manager>,
trezor: Arc<trezor::Manager>, trezor: Arc<trezor::Manager>,
} }
struct EventHandler {
ledger: Weak<ledger::Manager>,
trezor: Weak<trezor::Manager>,
}
impl libusb::Hotplug for EventHandler {
fn device_arrived(&mut self, _device: libusb::Device) {
debug!("USB Device arrived");
if let (Some(l), Some(t)) = (self.ledger.upgrade(), self.trezor.upgrade()) {
for _ in 0..10 {
let l_devices = l.update_devices().unwrap_or_else(|e| {
debug!("Error enumerating Ledger devices: {}", e);
0
});
let t_devices = t.update_devices().unwrap_or_else(|e| {
debug!("Error enumerating Trezor devices: {}", e);
0
});
if l_devices + t_devices > 0 {
break;
}
thread::sleep(Duration::from_millis(200));
}
}
}
fn device_left(&mut self, _device: libusb::Device) {
debug!("USB Device lost");
if let (Some(l), Some(t)) = (self.ledger.upgrade(), self.trezor.upgrade()) {
l.update_devices().unwrap_or_else(|e| {debug!("Error enumerating Ledger devices: {}", e); 0});
t.update_devices().unwrap_or_else(|e| {debug!("Error enumerating Trezor devices: {}", e); 0});
}
}
}
impl HardwareWalletManager { impl HardwareWalletManager {
/// Hardware wallet constructor
pub fn new() -> Result<HardwareWalletManager, Error> { pub fn new() -> Result<HardwareWalletManager, Error> {
let usb_context = Arc::new(libusb::Context::new()?); let usb_context_trezor = Arc::new(libusb::Context::new()?);
let usb_context_ledger = Arc::new(libusb::Context::new()?);
let hidapi = Arc::new(Mutex::new(hidapi::HidApi::new().map_err(|e| Error::Hid(e.to_string().clone()))?)); let hidapi = Arc::new(Mutex::new(hidapi::HidApi::new().map_err(|e| Error::Hid(e.to_string().clone()))?));
let ledger = Arc::new(ledger::Manager::new(hidapi.clone())); let ledger = Arc::new(ledger::Manager::new(hidapi.clone()));
let trezor = Arc::new(trezor::Manager::new(hidapi.clone())); let trezor = Arc::new(trezor::Manager::new(hidapi.clone()));
usb_context.register_callback(
None, None, None, // Subscribe to TREZOR V1
Box::new(EventHandler { // Note, this support only TREZOR V1 becasue TREZOR V2 has another vendorID for some reason
ledger: Arc::downgrade(&ledger), // Also, we now only support one product as the second argument specifies
trezor: Arc::downgrade(&trezor), usb_context_trezor.register_callback(
}), Some(trezor::TREZOR_VID), Some(trezor::TREZOR_PIDS[0]), Some(USB_DEVICE_CLASS_DEVICE),
)?; Box::new(trezor::EventHandler::new(Arc::downgrade(&trezor))))?;
// Subscribe to all Ledger Devices
// This means that we need to check that the given productID is supported
// None => LIBUSB_HOTPLUG_MATCH_ANY, in other words that all are subscribed to
// More info can be found: http://libusb.sourceforge.net/api-1.0/group__hotplug.html#gae6c5f1add6cc754005549c7259dc35ea
usb_context_ledger.register_callback(
Some(ledger::LEDGER_VID), None, Some(USB_DEVICE_CLASS_DEVICE),
Box::new(ledger::EventHandler::new(Arc::downgrade(&ledger))))?;
let exiting = Arc::new(AtomicBool::new(false)); let exiting = Arc::new(AtomicBool::new(false));
let thread_exiting = exiting.clone(); let thread_exiting_ledger = exiting.clone();
let thread_exiting_trezor = exiting.clone();
let l = ledger.clone(); let l = ledger.clone();
let t = trezor.clone(); let t = trezor.clone();
let thread = thread::Builder::new()
.name("hw_wallet".to_string()) // Ledger event thread
thread::Builder::new()
.name("hw_wallet_ledger".to_string())
.spawn(move || { .spawn(move || {
if let Err(e) = l.update_devices() { if let Err(e) = l.update_devices() {
debug!("Error updating ledger devices: {}", e); debug!(target: "hw", "Ledger couldn't connect at startup, error: {}", e);
} //debug!("Ledger could not connect at startup, error: {}", e);
if let Err(e) = t.update_devices() {
debug!("Error updating trezor devices: {}", e);
} }
loop { loop {
usb_context.handle_events(Some(Duration::from_millis(500))) usb_context_ledger.handle_events(Some(Duration::from_millis(500)))
.unwrap_or_else(|e| debug!("Error processing USB events: {}", e)); .unwrap_or_else(|e| debug!(target: "hw", "Ledger event handler error: {}", e));
if thread_exiting.load(atomic::Ordering::Acquire) { if thread_exiting_ledger.load(atomic::Ordering::Acquire) {
break; break;
} }
} }
}) })
.ok(); .ok();
// Trezor event thread
thread::Builder::new()
.name("hw_wallet_trezor".to_string())
.spawn(move || {
if let Err(e) = t.update_devices() {
debug!(target: "hw", "Trezor couldn't connect at startup, error: {}", e);
}
loop {
usb_context_trezor.handle_events(Some(Duration::from_millis(500)))
.unwrap_or_else(|e| debug!(target: "hw", "Trezor event handler error: {}", e));
if thread_exiting_trezor.load(atomic::Ordering::Acquire) {
break;
}
}
})
.ok();
Ok(HardwareWalletManager { Ok(HardwareWalletManager {
update_thread: thread,
exiting: exiting, exiting: exiting,
ledger: ledger, ledger: ledger,
trezor: trezor, trezor: trezor,
@ -259,10 +255,10 @@ impl HardwareWalletManager {
impl Drop for HardwareWalletManager { impl Drop for HardwareWalletManager {
fn drop(&mut self) { fn drop(&mut self) {
// Indicate to the USB Hotplug handlers that they
// shall terminate but don't wait for them to terminate.
// If they don't terminate for some reason USB Hotplug events will be handled
// even if the HardwareWalletManger has been dropped
self.exiting.store(true, atomic::Ordering::Release); self.exiting.store(true, atomic::Ordering::Release);
if let Some(thread) = self.update_thread.take() {
thread.thread().unpark();
thread.join().ok();
}
} }
} }

View File

@ -23,25 +23,29 @@ use super::{WalletInfo, TransactionInfo, KeyPath};
use std::cmp::{min, max}; use std::cmp::{min, max};
use std::fmt; use std::fmt;
use std::sync::Arc; use std::sync::{Arc, Weak};
use std::time::Duration; use std::time::Duration;
use std::thread;
use ethereum_types::{U256, H256, Address}; use ethereum_types::{U256, H256, Address};
use ethkey::Signature; use ethkey::Signature;
use hidapi; use hidapi;
use libusb;
use parking_lot::{Mutex, RwLock}; use parking_lot::{Mutex, RwLock};
use protobuf; use protobuf;
use protobuf::{Message, ProtobufEnum}; use protobuf::{Message, ProtobufEnum};
use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck}; use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck};
const TREZOR_VID: u16 = 0x534c; /// Trezor v1 vendor ID
const TREZOR_PIDS: [u16; 1] = [0x0001]; // Trezor v1, keeping this as an array to leave room for Trezor v2 which is in progress pub const TREZOR_VID: u16 = 0x534c;
/// Trezor product IDs
pub const TREZOR_PIDS: [u16; 1] = [0x0001];
const ETH_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003C, 0x80000000, 0, 0]; // m/44'/60'/0'/0/0 const ETH_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003C, 0x80000000, 0, 0]; // m/44'/60'/0'/0/0
const ETC_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003D, 0x80000000, 0, 0]; // m/44'/61'/0'/0/0 const ETC_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003D, 0x80000000, 0, 0]; // m/44'/61'/0'/0/0
/// Hardware wallet error. /// Hardware wallet error.
#[derive(Debug)] #[derive(Debug)]
pub enum Error { pub enum Error {
@ -56,7 +60,7 @@ pub enum Error {
/// The Message Type given in the trezor RPC call is not something we recognize /// The Message Type given in the trezor RPC call is not something we recognize
BadMessageType, BadMessageType,
/// Trying to read from a closed device at the given path /// Trying to read from a closed device at the given path
ClosedDevice(String), LockedDevice(String),
} }
impl fmt::Display for Error { impl fmt::Display for Error {
@ -67,7 +71,7 @@ impl fmt::Display for Error {
Error::KeyNotFound => write!(f, "Key not found"), Error::KeyNotFound => write!(f, "Key not found"),
Error::UserCancel => write!(f, "Operation has been cancelled"), Error::UserCancel => write!(f, "Operation has been cancelled"),
Error::BadMessageType => write!(f, "Bad Message Type in RPC call"), Error::BadMessageType => write!(f, "Bad Message Type in RPC call"),
Error::ClosedDevice(ref s) => write!(f, "Device is closed, needs PIN to perform operations: {}", s), Error::LockedDevice(ref s) => write!(f, "Device is locked, needs PIN to perform operations: {}", s),
} }
} }
} }
@ -84,11 +88,11 @@ impl From<protobuf::ProtobufError> for Error {
} }
} }
/// Ledger device manager. /// Ledger device manager
pub struct Manager { pub struct Manager {
usb: Arc<Mutex<hidapi::HidApi>>, usb: Arc<Mutex<hidapi::HidApi>>,
devices: RwLock<Vec<Device>>, devices: RwLock<Vec<Device>>,
closed_devices: RwLock<Vec<String>>, locked_devices: RwLock<Vec<String>>,
key_path: RwLock<KeyPath>, key_path: RwLock<KeyPath>,
} }
@ -110,7 +114,7 @@ impl Manager {
Manager { Manager {
usb: hidapi, usb: hidapi,
devices: RwLock::new(Vec::new()), devices: RwLock::new(Vec::new()),
closed_devices: RwLock::new(Vec::new()), locked_devices: RwLock::new(Vec::new()),
key_path: RwLock::new(KeyPath::Ethereum), key_path: RwLock::new(KeyPath::Ethereum),
} }
} }
@ -121,7 +125,7 @@ impl Manager {
usb.refresh_devices(); usb.refresh_devices();
let devices = usb.devices(); let devices = usb.devices();
let mut new_devices = Vec::new(); let mut new_devices = Vec::new();
let mut closed_devices = Vec::new(); let mut locked_devices = Vec::new();
let mut error = None; let mut error = None;
for usb_device in devices { for usb_device in devices {
let is_trezor = usb_device.vendor_id == TREZOR_VID; let is_trezor = usb_device.vendor_id == TREZOR_VID;
@ -140,7 +144,7 @@ impl Manager {
} }
match self.read_device_info(&usb, &usb_device) { match self.read_device_info(&usb, &usb_device) {
Ok(device) => new_devices.push(device), Ok(device) => new_devices.push(device),
Err(Error::ClosedDevice(path)) => closed_devices.push(path.to_string()), Err(Error::LockedDevice(path)) => locked_devices.push(path.to_string()),
Err(e) => { Err(e) => {
warn!("Error reading device: {:?}", e); warn!("Error reading device: {:?}", e);
error = Some(e); error = Some(e);
@ -148,9 +152,9 @@ impl Manager {
} }
} }
let count = new_devices.len(); let count = new_devices.len();
trace!("Got devices: {:?}, closed: {:?}", new_devices, closed_devices); trace!("Got devices: {:?}, closed: {:?}", new_devices, locked_devices);
*self.devices.write() = new_devices; *self.devices.write() = new_devices;
*self.closed_devices.write() = closed_devices; *self.locked_devices.write() = locked_devices;
match error { match error {
Some(e) => Err(e), Some(e) => Err(e),
None => Ok(count), None => Ok(count),
@ -174,7 +178,7 @@ impl Manager {
}, },
}) })
} }
Ok(None) => Err(Error::ClosedDevice(dev_info.path.clone())), Ok(None) => Err(Error::LockedDevice(dev_info.path.clone())),
Err(e) => Err(e), Err(e) => Err(e),
} }
} }
@ -190,7 +194,7 @@ impl Manager {
} }
pub fn list_locked_devices(&self) -> Vec<String> { pub fn list_locked_devices(&self) -> Vec<String> {
(*self.closed_devices.read()).clone() (*self.locked_devices.read()).clone()
} }
/// Get wallet info. /// Get wallet info.
@ -201,16 +205,7 @@ impl Manager {
fn open_path<R, F>(&self, f: F) -> Result<R, Error> fn open_path<R, F>(&self, f: F) -> Result<R, Error>
where F: Fn() -> Result<R, &'static str> where F: Fn() -> Result<R, &'static str>
{ {
let mut err = Error::KeyNotFound; f().map_err(Into::into)
// Try to open device a few times.
for _ in 0..10 {
match f() {
Ok(handle) => return Ok(handle),
Err(e) => err = From::from(e),
}
::std::thread::sleep(Duration::from_millis(200));
}
Err(err)
} }
pub fn pin_matrix_ack(&self, device_path: &str, pin: &str) -> Result<bool, Error> { pub fn pin_matrix_ack(&self, device_path: &str, pin: &str) -> Result<bool, Error> {
@ -407,6 +402,42 @@ impl Manager {
} }
} }
/// Trezor event handler
/// A separate thread is handeling incoming events
pub struct EventHandler {
trezor: Weak<Manager>,
}
impl EventHandler {
// Trezor event handler constructor
pub fn new(trezor: Weak<Manager>) -> Self {
Self { trezor: trezor }
}
}
impl libusb::Hotplug for EventHandler {
fn device_arrived(&mut self, _device: libusb::Device) {
debug!(target: "hw", "Trezor V1 arrived");
if let Some(trezor) = self.trezor.upgrade() {
// Wait for the device to boot up
thread::sleep(Duration::from_millis(1000));
if let Err(e) = trezor.update_devices() {
debug!(target: "hw", "Trezor V1 connect error: {:?}", e);
}
}
}
fn device_left(&mut self, _device: libusb::Device) {
debug!(target: "hw", "Trezor V1 left");
if let Some(trezor) = self.trezor.upgrade() {
if let Err(e) = trezor.update_devices() {
debug!(target: "hw", "Trezor V1 disconnect error: {:?}", e);
}
}
}
}
#[test] #[test]
#[ignore] #[ignore]
/// This test can't be run without an actual trezor device connected /// This test can't be run without an actual trezor device connected