refactor (hardware wallet) : reduce the number of threads (#9644)

* refactor(hardware-wallet) - use only one thread

This changes parity to use one thread instead of two to subscribe USB
events via the hardware-wallets.

* Fix logs and tests

* fix(grumbles) : formatting and spelling nits

* Update hw/src/lib.rs

Co-Authored-By: niklasad1 <niklasadolfsson1@gmail.com>

* Update hw/src/lib.rs

Co-Authored-By: niklasad1 <niklasadolfsson1@gmail.com>

* Update hw/src/ledger.rs

Co-Authored-By: niklasad1 <niklasadolfsson1@gmail.com>

* fix(grumbles): clarify docs and use constants

* Clarify bad explaination of `run-to-completion`
* Replace magic numbers with constants
This commit is contained in:
Niklas Adolfsson 2019-01-02 16:48:45 +01:00 committed by Afri Schoedon
parent c077dc652d
commit 2bb79614f6
3 changed files with 243 additions and 257 deletions

File diff suppressed because one or more lines are too long

View File

@ -34,15 +34,18 @@ extern crate trezor_sys;
mod ledger; mod ledger;
mod trezor; mod trezor;
use std::sync::{Arc, atomic, atomic::AtomicBool}; use std::sync::{Arc, atomic, atomic::AtomicBool, Weak};
use std::{fmt, time::Duration}; use std::{fmt, time::Duration};
use std::thread;
use ethereum_types::U256; use ethereum_types::U256;
use ethkey::{Address, Signature}; use ethkey::{Address, Signature};
use parking_lot::Mutex; use parking_lot::Mutex;
const USB_DEVICE_CLASS_DEVICE: u8 = 0; const HID_GLOBAL_USAGE_PAGE: u16 = 0xFF00;
const POLLING_DURATION: Duration = Duration::from_millis(500); const HID_USB_DEVICE_CLASS: u8 = 0;
const MAX_POLLING_DURATION: Duration = Duration::from_millis(500);
const USB_EVENT_POLLING_INTERVAL: Duration = Duration::from_millis(500);
/// `HardwareWallet` device /// `HardwareWallet` device
#[derive(Debug)] #[derive(Debug)]
@ -189,7 +192,7 @@ impl From<libusb::Error> for Error {
} }
/// Specifies the direction of the `HardwareWallet` i.e, whether it arrived or left /// Specifies the direction of the `HardwareWallet` i.e, whether it arrived or left
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone, PartialEq)]
pub enum DeviceDirection { pub enum DeviceDirection {
/// Device arrived /// Device arrived
Arrived, Arrived,
@ -218,13 +221,47 @@ impl HardwareWalletManager {
pub fn new() -> Result<Self, Error> { pub fn new() -> Result<Self, Error> {
let exiting = Arc::new(AtomicBool::new(false)); let exiting = Arc::new(AtomicBool::new(false));
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 = ledger::Manager::new(hidapi.clone(), exiting.clone())?; let ledger = ledger::Manager::new(hidapi.clone());
let trezor = trezor::Manager::new(hidapi.clone(), exiting.clone())?; let trezor = trezor::Manager::new(hidapi.clone());
let usb_context = Arc::new(libusb::Context::new()?);
let l = ledger.clone();
let t = trezor.clone();
let exit = exiting.clone();
// Subscribe to all vendor IDs (VIDs) and product IDs (PIDs)
// This means that the `HardwareWalletManager` is responsible to validate the detected device
usb_context.register_callback(
None, None, Some(HID_USB_DEVICE_CLASS),
Box::new(EventHandler::new(
Arc::downgrade(&ledger),
Arc::downgrade(&trezor)
))
)?;
// Hardware event subscriber thread
thread::Builder::new()
.name("hw_wallet_manager".to_string())
.spawn(move || {
if let Err(e) = l.update_devices(DeviceDirection::Arrived) {
debug!(target: "hw", "Ledger couldn't connect at startup, error: {}", e);
}
if let Err(e) = t.update_devices(DeviceDirection::Arrived) {
debug!(target: "hw", "Trezor couldn't connect at startup, error: {}", e);
}
while !exit.load(atomic::Ordering::Acquire) {
if let Err(e) = usb_context.handle_events(Some(USB_EVENT_POLLING_INTERVAL)) {
debug!(target: "hw", "HardwareWalletManager event handler error: {}", e);
}
}
})
.ok();
Ok(Self { Ok(Self {
exiting, exiting,
ledger,
trezor, trezor,
ledger,
}) })
} }
@ -292,10 +329,74 @@ 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 // Indicate to the USB Hotplug handler that it
// shall terminate but don't wait for them to terminate. // shall terminate but don't wait for it to terminate.
// If they don't terminate for some reason USB Hotplug events will be handled // If it doesn't terminate for some reason USB Hotplug events will be handled
// even if the HardwareWalletManger has been dropped // even if the HardwareWalletManger has been dropped
self.exiting.store(true, atomic::Ordering::Release); self.exiting.store(true, atomic::Ordering::Release);
} }
} }
/// Hardware wallet event handler
///
/// Note, that this runs to completion and race-conditions can't occur but it can
/// stop other events for being processed with an infinite loop or similar
struct EventHandler {
ledger: Weak<ledger::Manager>,
trezor: Weak<trezor::Manager>,
}
impl EventHandler {
/// Trezor event handler constructor
pub fn new(ledger: Weak<ledger::Manager>, trezor: Weak<trezor::Manager>) -> Self {
Self { ledger, trezor }
}
fn extract_device_info(device: &libusb::Device) -> Result<(u16, u16), Error> {
let desc = device.device_descriptor()?;
Ok((desc.vendor_id(), desc.product_id()))
}
}
impl libusb::Hotplug for EventHandler {
fn device_arrived(&mut self, device: libusb::Device) {
// Upgrade reference to an Arc
if let (Some(ledger), Some(trezor)) = (self.ledger.upgrade(), self.trezor.upgrade()) {
// Version ID and Product ID are available
if let Ok((vid, pid)) = Self::extract_device_info(&device) {
if trezor::is_valid_trezor(vid, pid) {
if !trezor::try_connect_polling(&trezor, &MAX_POLLING_DURATION, DeviceDirection::Arrived) {
trace!(target: "hw", "Trezor device was detected but connection failed");
}
} else if ledger::is_valid_ledger(vid, pid) {
if !ledger::try_connect_polling(&ledger, &MAX_POLLING_DURATION, DeviceDirection::Arrived) {
trace!(target: "hw", "Ledger device was detected but connection failed");
}
}
}
}
}
fn device_left(&mut self, device: libusb::Device) {
// Upgrade reference to an Arc
if let (Some(ledger), Some(trezor)) = (self.ledger.upgrade(), self.trezor.upgrade()) {
// Version ID and Product ID are available
if let Ok((vid, pid)) = Self::extract_device_info(&device) {
if trezor::is_valid_trezor(vid, pid) {
if !trezor::try_connect_polling(&trezor, &MAX_POLLING_DURATION, DeviceDirection::Left) {
trace!(target: "hw", "Trezor device was detected but disconnection failed");
}
} else if ledger::is_valid_ledger(vid, pid) {
if !ledger::try_connect_polling(&ledger, &MAX_POLLING_DURATION, DeviceDirection::Left) {
trace!(target: "hw", "Ledger device was detected but disconnection failed");
}
}
}
}
}
}
/// Helper to determine if a device is a valid HID
pub fn is_valid_hid_device(usage_page: u16, interface_number: i32) -> bool {
usage_page == HID_GLOBAL_USAGE_PAGE || interface_number == HID_USB_DEVICE_CLASS as i32
}

View File

@ -20,9 +20,9 @@
//! for protocol details. //! for protocol details.
use std::cmp::{min, max}; use std::cmp::{min, max};
use std::sync::{atomic, atomic::AtomicBool, Arc, Weak}; use std::sync::Arc;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
use std::{fmt, thread}; use std::fmt;
use ethereum_types::{U256, H256, Address}; use ethereum_types::{U256, H256, Address};
use ethkey::Signature; use ethkey::Signature;
@ -30,7 +30,7 @@ use hidapi;
use libusb; use libusb;
use parking_lot::{Mutex, RwLock}; use parking_lot::{Mutex, RwLock};
use protobuf::{self, Message, ProtobufEnum}; use protobuf::{self, Message, ProtobufEnum};
use super::{DeviceDirection, WalletInfo, TransactionInfo, KeyPath, Wallet, Device, USB_DEVICE_CLASS_DEVICE, POLLING_DURATION}; use super::{DeviceDirection, WalletInfo, TransactionInfo, KeyPath, Wallet, Device, is_valid_hid_device};
use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck}; use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck};
/// Trezor v1 vendor ID /// Trezor v1 vendor ID
@ -48,6 +48,8 @@ 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.
@ -62,6 +64,8 @@ pub enum Error {
NoDeviceArrived, NoDeviceArrived,
/// No device left /// No device left
NoDeviceLeft, NoDeviceLeft,
/// Invalid PID or VID
InvalidDevice,
} }
impl fmt::Display for Error { impl fmt::Display for Error {
@ -69,13 +73,15 @@ impl fmt::Display for Error {
match *self { match *self {
Error::Protocol(ref s) => write!(f, "Trezor protocol error: {}", s), Error::Protocol(ref s) => write!(f, "Trezor 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::BadMessageType => write!(f, "Bad Message Type in RPC call"), Error::BadMessageType => write!(f, "Bad Message Type in RPC call"),
Error::LockedDevice(ref s) => write!(f, "Device is locked, needs PIN to perform operations: {}", s), Error::LockedDevice(ref s) => write!(f, "Device is locked, needs PIN to perform operations: {}", s),
Error::NoSigningMessage=> write!(f, "Signing messages are not supported by Trezor"), Error::NoSigningMessage=> write!(f, "Signing messages are not supported by Trezor"),
Error::NoDeviceArrived => write!(f, "No device arrived"), Error::NoDeviceArrived => write!(f, "No device arrived"),
Error::NoDeviceLeft=> write!(f, "No device left"), Error::NoDeviceLeft => write!(f, "No device left"),
Error::InvalidDevice => write!(f, "Device with non-supported product ID or vendor ID was detected"),
} }
} }
} }
@ -86,6 +92,12 @@ impl From<hidapi::HidError> for Error {
} }
} }
impl From<libusb::Error> for Error {
fn from(err: libusb::Error) -> Self {
Error::LibUsb(err)
}
}
impl From<protobuf::ProtobufError> for Error { impl From<protobuf::ProtobufError> for Error {
fn from(_: protobuf::ProtobufError) -> Self { fn from(_: protobuf::ProtobufError) -> Self {
Error::Protocol(&"Could not read response from Trezor Device") Error::Protocol(&"Could not read response from Trezor Device")
@ -93,7 +105,7 @@ impl From<protobuf::ProtobufError> for Error {
} }
/// Trezor device manager /// Trezor device manager
pub (crate) struct Manager { pub struct Manager {
usb: Arc<Mutex<hidapi::HidApi>>, usb: Arc<Mutex<hidapi::HidApi>>,
devices: RwLock<Vec<Device>>, devices: RwLock<Vec<Device>>,
locked_devices: RwLock<Vec<String>>, locked_devices: RwLock<Vec<String>>,
@ -108,42 +120,13 @@ enum HidVersion {
impl Manager { impl Manager {
/// Create a new instance. /// Create a new instance.
pub fn new(hidapi: Arc<Mutex<hidapi::HidApi>>, exiting: Arc<AtomicBool>) -> Result<Arc<Self>, libusb::Error> { pub fn new(usb: Arc<Mutex<hidapi::HidApi>>) -> Arc<Self> {
let manager = Arc::new(Self { Arc::new(Self {
usb: hidapi, usb,
devices: RwLock::new(Vec::new()), devices: RwLock::new(Vec::new()),
locked_devices: RwLock::new(Vec::new()), locked_devices: RwLock::new(Vec::new()),
key_path: RwLock::new(KeyPath::Ethereum), key_path: RwLock::new(KeyPath::Ethereum),
}); })
let usb_context = Arc::new(libusb::Context::new()?);
let m = manager.clone();
// Subscribe to TREZOR V1
// Note, this support only TREZOR V1 because TREZOR V2 has a different vendorID for some reason
// Also, we now only support one product as the second argument specifies
usb_context.register_callback(
Some(TREZOR_VID), Some(TREZOR_PIDS[0]), Some(USB_DEVICE_CLASS_DEVICE),
Box::new(EventHandler::new(Arc::downgrade(&manager))))?;
// Trezor event thread
thread::Builder::new()
.name("hw_wallet_trezor".to_string())
.spawn(move || {
if let Err(e) = m.update_devices(DeviceDirection::Arrived) {
debug!(target: "hw", "Trezor couldn't connect at startup, error: {}", e);
}
loop {
usb_context.handle_events(Some(Duration::from_millis(500)))
.unwrap_or_else(|e| debug!(target: "hw", "Trezor event handler error: {}", e));
if exiting.load(atomic::Ordering::Acquire) {
break;
}
}
})
.ok();
Ok(manager)
} }
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> {
@ -153,7 +136,7 @@ impl Manager {
let t = MessageType::MessageType_PinMatrixAck; let t = MessageType::MessageType_PinMatrixAck;
let mut m = PinMatrixAck::new(); let mut m = PinMatrixAck::new();
m.set_pin(pin.to_string()); m.set_pin(pin.to_string());
self.send_device_message(&device, &t, &m)?; self.send_device_message(&device, t, &m)?;
let (resp_type, _) = self.read_device_response(&device)?; let (resp_type, _) = self.read_device_response(&device)?;
match resp_type { match resp_type {
// Getting an Address back means it's unlocked, this is undocumented behavior // Getting an Address back means it's unlocked, this is undocumented behavior
@ -178,7 +161,7 @@ impl Manager {
match resp_type { match resp_type {
MessageType::MessageType_Cancel => Err(Error::UserCancel), MessageType::MessageType_Cancel => Err(Error::UserCancel),
MessageType::MessageType_ButtonRequest => { MessageType::MessageType_ButtonRequest => {
self.send_device_message(handle, &MessageType::MessageType_ButtonAck, &ButtonAck::new())?; self.send_device_message(handle, MessageType::MessageType_ButtonAck, &ButtonAck::new())?;
// Signing loop goes back to the top and reading blocks // Signing loop goes back to the top and reading blocks
// for up to 5 minutes waiting for response from the device // for up to 5 minutes waiting for response from the device
// if the user doesn't click any button within 5 minutes you // if the user doesn't click any button within 5 minutes you
@ -191,7 +174,7 @@ impl Manager {
let mut msg = EthereumTxAck::new(); let mut msg = EthereumTxAck::new();
let len = resp.get_data_length() as usize; let len = resp.get_data_length() as usize;
msg.set_data_chunk(data[..len].to_vec()); msg.set_data_chunk(data[..len].to_vec());
self.send_device_message(handle, &MessageType::MessageType_EthereumTxAck, &msg)?; self.send_device_message(handle, MessageType::MessageType_EthereumTxAck, &msg)?;
self.signing_loop(handle, chain_id, &data[len..]) self.signing_loop(handle, chain_id, &data[len..])
} else { } else {
let v = resp.get_signature_v(); let v = resp.get_signature_v();
@ -216,8 +199,8 @@ impl Manager {
} }
} }
fn send_device_message(&self, device: &hidapi::HidDevice, msg_type: &MessageType, msg: &Message) -> Result<usize, Error> { fn send_device_message(&self, device: &hidapi::HidDevice, msg_type: MessageType, msg: &Message) -> Result<usize, Error> {
let msg_id = *msg_type as u16; let msg_id = msg_type as u16;
let mut message = msg.write_to_bytes()?; let mut message = msg.write_to_bytes()?;
let msg_size = message.len(); let msg_size = message.len();
let mut data = Vec::new(); let mut data = Vec::new();
@ -284,7 +267,7 @@ impl Manager {
} }
} }
impl <'a>Wallet<'a> for Manager { impl<'a> Wallet<'a> for Manager {
type Error = Error; type Error = Error;
type Transaction = &'a TransactionInfo; type Transaction = &'a TransactionInfo;
@ -316,7 +299,7 @@ impl <'a>Wallet<'a> for Manager {
message.set_chain_id(c_id as u32); message.set_chain_id(c_id as u32);
} }
self.send_device_message(&handle, &msg_type, &message)?; self.send_device_message(&handle, msg_type, &message)?;
self.signing_loop(&handle, &t_info.chain_id, &t_info.data[first_chunk_length..]) self.signing_loop(&handle, &t_info.chain_id, &t_info.data[first_chunk_length..])
} }
@ -332,13 +315,9 @@ impl <'a>Wallet<'a> for Manager {
let num_prev_devices = self.devices.read().len(); let num_prev_devices = self.devices.read().len();
let detected_devices = devices.iter() let detected_devices = devices.iter()
.filter(|&d| { .filter(|&d| is_valid_trezor(d.vendor_id, d.product_id) &&
let is_trezor = d.vendor_id == TREZOR_VID; is_valid_hid_device(d.usage_page, d.interface_number)
let is_supported_product = TREZOR_PIDS.contains(&d.product_id); )
let is_valid = d.usage_page == 0xFF00 || d.interface_number == 0;
is_trezor && is_supported_product && is_valid
})
.fold(Vec::new(), |mut v, d| { .fold(Vec::new(), |mut v, d| {
match self.read_device(&usb, &d) { match self.read_device(&usb, &d) {
Ok(info) => { Ok(info) => {
@ -363,7 +342,7 @@ impl <'a>Wallet<'a> for Manager {
} }
DeviceDirection::Left => { DeviceDirection::Left => {
if num_prev_devices > num_curr_devices { if num_prev_devices > num_curr_devices {
Ok(num_prev_devices- num_curr_devices) Ok(num_prev_devices - num_curr_devices)
} else { } else {
Err(Error::NoDeviceLeft) Err(Error::NoDeviceLeft)
} }
@ -413,7 +392,7 @@ impl <'a>Wallet<'a> for Manager {
KeyPath::EthereumClassic => message.set_address_n(ETC_DERIVATION_PATH.to_vec()), KeyPath::EthereumClassic => message.set_address_n(ETC_DERIVATION_PATH.to_vec()),
} }
message.set_show_display(false); message.set_show_display(false);
self.send_device_message(&device, &typ, &message)?; self.send_device_message(&device, typ, &message)?;
let (resp_type, bytes) = self.read_device_response(&device)?; let (resp_type, bytes) = self.read_device_response(&device)?;
match resp_type { match resp_type {
@ -432,8 +411,9 @@ impl <'a>Wallet<'a> for Manager {
} }
} }
// Try to connect to the device using polling in at most the time specified by the `timeout`
fn try_connect_polling(trezor: &Manager, duration: &Duration, dir: DeviceDirection) -> bool { /// Poll the device in maximum `max_polling_duration` if it doesn't succeed
pub fn try_connect_polling(trezor: &Manager, duration: &Duration, dir: DeviceDirection) -> bool {
let start_time = Instant::now(); let start_time = Instant::now();
while start_time.elapsed() <= *duration { while start_time.elapsed() <= *duration {
if let Ok(num_devices) = trezor.update_devices(dir) { if let Ok(num_devices) = trezor.update_devices(dir) {
@ -444,73 +424,43 @@ fn try_connect_polling(trezor: &Manager, duration: &Duration, dir: DeviceDirecti
false false
} }
/// Trezor event handler /// Check if the detected device is a Trezor device by checking both the product ID and the vendor ID
/// A separate thread is handling incoming events pub fn is_valid_trezor(vid: u16, pid: u16) -> bool {
/// vid == TREZOR_VID && TREZOR_PIDS.contains(&pid)
/// Note, that this run to completion and race-conditions can't occur but this can
/// therefore starve other events for being process with a spinlock or similar
struct EventHandler {
trezor: Weak<Manager>,
} }
impl EventHandler {
/// Trezor event handler constructor
pub fn new(trezor: Weak<Manager>) -> Self {
Self { 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() {
if try_connect_polling(&trezor, &POLLING_DURATION, DeviceDirection::Arrived) != true {
trace!(target: "hw", "No Trezor connected");
}
}
}
fn device_left(&mut self, _device: libusb::Device) {
debug!(target: "hw", "Trezor V1 left");
if let Some(trezor) = self.trezor.upgrade() {
if try_connect_polling(&trezor, &POLLING_DURATION, DeviceDirection::Left) != true {
trace!(target: "hw", "No Trezor disconnected");
}
}
}
}
#[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
/// (and unlocked) attached to the machine that's running the test /// (and unlocked) attached to the machine that's running the test
fn test_signature() { fn test_signature() {
use ethereum_types::{H160, H256, U256}; use ethereum_types::Address;
use MAX_POLLING_DURATION;
use super::HardwareWalletManager;
let manager = Manager::new( let manager = HardwareWalletManager::new().unwrap();
Arc::new(Mutex::new(hidapi::HidApi::new().expect("HidApi"))),
Arc::new(AtomicBool::new(false))
).expect("HardwareWalletManager");
let addr: Address = H160::from("some_addr"); assert_eq!(try_connect_polling(&manager.trezor, &MAX_POLLING_DURATION, DeviceDirection::Arrived), true);
assert_eq!(try_connect_polling(&manager.clone(), &POLLING_DURATION, DeviceDirection::Arrived), true); let addr: Address = manager.list_wallets()
.iter()
.filter(|d| d.name == "TREZOR".to_string() && d.manufacturer == "SatoshiLabs".to_string())
.nth(0)
.map(|d| d.address)
.unwrap();
let t_info = TransactionInfo { let t_info = TransactionInfo {
nonce: U256::from(1), nonce: U256::from(1),
gas_price: U256::from(100), gas_price: U256::from(100),
gas_limit: U256::from(21_000), gas_limit: U256::from(21_000),
to: Some(H160::from("some_other_addr")), to: Some(Address::from(1337)),
chain_id: Some(17), chain_id: Some(1),
value: U256::from(1_000_000), value: U256::from(1_000_000),
data: (&[1u8; 3000]).to_vec(), data: (&[1u8; 3000]).to_vec(),
}; };
let signature = manager.sign_transaction(&addr, &t_info).unwrap();
let expected = Signature::from_rsv(
&H256::from("device_specific_r"),
&H256::from("device_specific_s"),
0x01
);
assert_eq!(signature, expected) let signature = manager.trezor.sign_transaction(&addr, &t_info);
assert!(signature.is_ok());
} }