Trezor Support (#6403)

* Copy modal from keepkey branch and generalize

The keepkey PinMatrix modal needs to be the same for Trezor, but we
should probably try to keep it general since it can be used for both.

* Add trezor communication code

This is a result of much trial-and-error and a couple of dead-ends in
how to communicate and wire everything up.

Code here is still a bit WIP with lots of debug prints and stuff.

The test works though, it is possible to sign a transaction.

* Extend the basic lib to allow Trezor

This is kind of ugly and needs some cleanup and generalization. I’ve
just copy-pasted some things to bring in the trezor wallets. I’ve also
had to add a lock to the USB API so that only one thing talks to the
USB at once.

* Add RPC plumbing needed

We need to be able to get “locked” devices from the frontend to figure
out if we’re going to display the PinMatrix or not. Then we need to be
able to send a pin to a device.

* Add logic to query backend for Trezor and display PinMatrix

There’s a bug somewhere here because signing a transaction fails if you
take too long to press the confirm button on the device.

* Change back to paritytech branch

As my fork has been merged in.

* Converting spaces to tabs, as it should be

* Incorporate correct handling of EIP-155

Turns out the Trezor was adjusting the v part of the signature, and
we’re already doing that so it was done twice.

* Some circular logic here that was incorrect

BE-encoded U256 is almost the same as RLP encoded without the
size-byte, except for <u8 sized values. What’s really done is
BE-encoded U256 and then left-trimmed to the smallest size. Kind of
obvious in hindsight.

* Resolve issue where not clicking fast enough fails

The device will not repeat a ButtonRequest when you read from it, so
you need to have a blocking `read` for whatever amount of time that you
want to give the user to click. You could also have a shorter timeout
but keep retrying for some amount of time, but it would amount to the
same thing.

* Scan after pin entry to make accepting it faster

* Remove ability to cancel pin request

* Some slight cleanup

* Probe for the correct HID Version to determine padding

* Move the PinMatrix from Accounts to Application

* Removing unused dependencies

* Mistake in copying over stuff from keepkey branch

* Simplify FormattedMessage

* Move generated code to external crate

* Remove ethcore-util dependency

* Fix broken import in test

This test is useless without a connected Trezor, not sure how to make
it useful without one.

* Merge branch 'master' into fh-4500-trezor-support

# Conflicts:
#	rpc/src/v1/helpers/dispatch.rs

* Ignore test that can't be run without trezor device

* Fixing grumbles

* Avoiding owning data in RPC method
* Checking for overflow in v part of signature
* s/network_id/chain_id
* Propagating an error from the HID Api
* Condensing code a little bit

* Fixing UI.

* Debugging trezor.

* Minor styling tweak

* Make message type into an actual type

This makes the message type that the RPC message accepts into an actual
type as opposed to just a string, based on feedback. Although I’m not
100% sure this has actually improved the situation.

Overall I think the hardware wallet interface needs some refactoring
love.

* Split the trezor RPC endpoint

It’s split into two more generic endpoints that should be suitable for
any hardware wallets with the same behavior to sit behind.

* Reflect RPC method split in javascript

* Fix bug with pin entry

* Fix deadlock for Ledger

* Avoid having a USB lock in just listing locked wallets

* Fix javascript issue (see #6509)

* Replace Mutex with RwLock

* Update Ledger test

* Fix typo causing faulty signatures (sometimes)

* *Actually* fix tests

* Update git submodule

Needed to make tests pass

* Swap line orders to prevent possible deadlock

* Make setPinMatrixRequest an @action

ethcore/src/account_provider/mod.rs

@@ -31,7 +31,8 @@ use ethstore::{
 use ethstore::dir::MemoryDirectory;
 use ethstore::ethkey::{Address, Message, Public, Secret, Random, Generator};
 use ethjson::misc::AccountMeta;
-use hardware_wallet::{Error as HardwareError, HardwareWalletManager, KeyPath};
+use hardware_wallet::{Error as HardwareError, HardwareWalletManager, KeyPath, TransactionInfo};
+use super::transaction::{Action, Transaction};
 pub use ethstore::ethkey::Signature;
 pub use ethstore::{Derivation, IndexDerivation, KeyFile};
 
@@ -288,6 +289,24 @@ impl AccountProvider {
 		Ok(accounts.into_iter().map(|a| a.address).collect())
 	}
 
+	/// Get a list of paths to locked hardware wallets
+	pub fn locked_hardware_accounts(&self) -> Result<Vec<String>, SignError> {
+		match self.hardware_store.as_ref().map(|h| h.list_locked_wallets()) {
+			None => Err(SignError::NotFound),
+			Some(Err(e)) => Err(SignError::Hardware(e)),
+			Some(Ok(s)) => Ok(s),
+		}
+	}
+
+	/// Provide a pin to a locked hardware wallet on USB path to unlock it
+	pub fn hardware_pin_matrix_ack(&self, path: &str, pin: &str) -> Result<bool, SignError> {
+		match self.hardware_store.as_ref().map(|h| h.pin_matrix_ack(path, pin)) {
+			None => Err(SignError::NotFound),
+			Some(Err(e)) => Err(SignError::Hardware(e)),
+			Some(Ok(s)) => Ok(s),
+		}
+	}
+
 	/// Sets addresses of accounts exposed for unknown dapps.
 	/// `None` means that all accounts will be visible.
 	/// If not `None` or empty it will also override default account.
@@ -779,8 +798,20 @@ impl AccountProvider {
 	}
 
 	/// Sign transaction with hardware wallet.
-	pub fn sign_with_hardware(&self, address: Address, transaction: &[u8]) -> Result<Signature, SignError> {
-		match self.hardware_store.as_ref().map(|s| s.sign_transaction(&address, transaction)) {
+	pub fn sign_with_hardware(&self, address: Address, transaction: &Transaction, chain_id: Option<u64>, rlp_encoded_transaction: &[u8]) -> Result<Signature, SignError> {
+		let t_info = TransactionInfo {
+			nonce: transaction.nonce,
+			gas_price: transaction.gas_price,
+			gas_limit: transaction.gas,
+			to: match transaction.action {
+				Action::Create => None,
+				Action::Call(ref to) => Some(to.clone()),
+			},
+			value: transaction.value,
+			data: transaction.data.to_vec(),
+			chain_id: chain_id,
+		};
+		match self.hardware_store.as_ref().map(|s| s.sign_transaction(&address, &t_info, rlp_encoded_transaction)) {
 			None | Some(Err(HardwareError::KeyNotFound)) => Err(SignError::NotFound),
 			Some(Err(e)) => Err(From::from(e)),
 			Some(Ok(s)) => Ok(s),

hw/Cargo.toml

@@ -9,8 +9,10 @@ authors = ["Parity Technologies <admin@parity.io>"]
 [dependencies]
 log = "0.3"
 parking_lot = "0.4"
+protobuf = "1.4"
 hidapi = { git = "https://github.com/paritytech/hidapi-rs" }
 libusb = { git = "https://github.com/paritytech/libusb-rs" }
+trezor-sys = { git = "https://github.com/paritytech/trezor-sys" }
 ethkey = { path = "../ethkey" }
 ethcore-bigint = { path = "../util/bigint" }
 

hw/src/ledger.rs

@@ -17,19 +17,23 @@
 //! Ledger hardware wallet module. Supports Ledger Blue and Nano S.
 /// See https://github.com/LedgerHQ/blue-app-eth/blob/master/doc/ethapp.asc for protocol details.
 
+use super::{WalletInfo, KeyPath};
+
+use bigint::hash::H256;
+use ethkey::{Address, Signature};
 use hidapi;
-use std::fmt;
+use parking_lot::{Mutex, RwLock};
+
 use std::cmp::min;
+use std::fmt;
 use std::str::FromStr;
+use std::sync::Arc;
 use std::time::Duration;
-use super::WalletInfo;
-use ethkey::{Address, Signature};
-use bigint::hash::H256;
 
 const LEDGER_VID: u16 = 0x2c97;
 const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001]; // Nano S and Blue
-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 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 APDU_TAG: u8 = 0x05;
 const APDU_CLA: u8 = 0xe0;
@@ -43,16 +47,7 @@ mod commands {
 	pub const SIGN_ETH_TRANSACTION: u8 = 0x04;
 }
 
-/// Key derivation paths used on ledger wallets.
-#[derive(Debug, Clone, Copy)]
-pub enum KeyPath {
-	/// Ethereum.
-	Ethereum,
-	/// Ethereum classic.
-	EthereumClassic,
-}
-
-/// Hardware waller error.
+/// Hardware wallet error.
 #[derive(Debug)]
 pub enum Error {
 	/// Ethereum wallet protocol error.
@@ -84,9 +79,9 @@ impl From<hidapi::HidError> for Error {
 
 /// Ledger device manager.
 pub struct Manager {
-	usb: hidapi::HidApi,
-	devices: Vec<Device>,
-	key_path: KeyPath,
+	usb: Arc<Mutex<hidapi::HidApi>>,
+	devices: RwLock<Vec<Device>>,
+	key_path: RwLock<KeyPath>,
 }
 
 #[derive(Debug)]
@@ -97,50 +92,50 @@ struct Device {
 
 impl Manager {
 	/// Create a new instance.
-	pub fn new() -> Result<Manager, Error> {
-		let manager = Manager {
-			usb: hidapi::HidApi::new()?,
-			devices: Vec::new(),
-			key_path: KeyPath::Ethereum,
-		};
-		Ok(manager)
+	pub fn new(hidapi: Arc<Mutex<hidapi::HidApi>>) -> Manager {
+		Manager {
+			usb: hidapi,
+			devices: RwLock::new(Vec::new()),
+			key_path: RwLock::new(KeyPath::Ethereum),
+		}
 	}
 
 	/// Re-populate device list. Only those devices that have Ethereum app open will be added.
-	pub fn update_devices(&mut self) -> Result<usize, Error> {
-		self.usb.refresh_devices();
-		let devices = self.usb.devices();
+	pub fn update_devices(&self) -> Result<usize, Error> {
+		let mut usb = self.usb.lock();
+		usb.refresh_devices();
+		let devices = usb.devices();
 		let mut new_devices = Vec::new();
 		let mut num_new_devices = 0;
 		for device in devices {
 			trace!("Checking device: {:?}", device);
 			if device.vendor_id != LEDGER_VID || !LEDGER_PIDS.contains(&device.product_id) {
 				continue;
 			}
-			match self.read_device_info(&device) {
+			match self.read_device_info(&usb, &device) {
 				Ok(info) => {
 					debug!("Found device: {:?}", info);
-					if !self.devices.iter().any(|d| d.path == info.path) {
+					if !self.devices.read().iter().any(|d| d.path == info.path) {
 						num_new_devices += 1;
 					}
 					new_devices.push(info);
 
-				},
+				}
 				Err(e) => debug!("Error reading device info: {}", e),
 			};
 		}
-		self.devices = new_devices;
+		*self.devices.write() = new_devices;
 		Ok(num_new_devices)
 	}
 
 	/// Select key derivation path for a known chain.
-	pub fn set_key_path(&mut self, key_path: KeyPath) {
-		self.key_path = key_path;
+	pub fn set_key_path(&self, key_path: KeyPath) {
+		*self.key_path.write() = key_path;
 	}
 
-	fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo) -> Result<Device, Error> {
-		let mut handle = self.open_path(&dev_info.path)?;
-		let address = Self::read_wallet_address(&mut handle, self.key_path)?;
+	fn read_device_info(&self, usb: &hidapi::HidApi, dev_info: &hidapi::HidDeviceInfo) -> Result<Device, Error> {
+		let mut handle = self.open_path(|| usb.open_path(&dev_info.path))?;
+		let address = Self::read_wallet_address(&mut handle, *self.key_path.read())?;
 		let manufacturer = dev_info.manufacturer_string.clone().unwrap_or("Unknown".to_owned());
 		let name = dev_info.product_string.clone().unwrap_or("Unknown".to_owned());
 		let serial = dev_info.serial_number.clone().unwrap_or("Unknown".to_owned());
@@ -187,24 +182,24 @@ impl Manager {
 
 	/// List connected wallets. This only returns wallets that are ready to be used.
 	pub fn list_devices(&self) -> Vec<WalletInfo> {
-		self.devices.iter().map(|d| d.info.clone()).collect()
+		self.devices.read().iter().map(|d| d.info.clone()).collect()
 	}
 
 	/// Get wallet info.
 	pub fn device_info(&self, address: &Address) -> Option<WalletInfo> {
-		self.devices.iter().find(|d| &d.info.address == address).map(|d| d.info.clone())
+		self.devices.read().iter().find(|d| &d.info.address == address).map(|d| d.info.clone())
 	}
 
 	/// Sign transaction data with wallet managing `address`.
 	pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result<Signature, Error> {
-		let device = self.devices.iter().find(|d| &d.info.address == address)
-			.ok_or(Error::KeyNotFound)?;
-
-		let handle = self.open_path(&device.path)?;
+		let usb = self.usb.lock();
+		let devices = self.devices.read();
+		let device = devices.iter().find(|d| &d.info.address == address).ok_or(Error::KeyNotFound)?;
+		let handle = self.open_path(|| usb.open_path(&device.path))?;
 
 		let eth_path = &ETH_DERIVATION_PATH_BE[..];
 		let etc_path = &ETC_DERIVATION_PATH_BE[..];
-		let derivation_path = match self.key_path {
+		let derivation_path = match *self.key_path.read() {
 			KeyPath::Ethereum => eth_path,
 			KeyPath::EthereumClassic => etc_path,
 		};
@@ -218,7 +213,7 @@ impl Manager {
 			let p1 = if data_pos == 0 { 0x00 } else { 0x80 };
 			let dest_left = MAX_CHUNK_SIZE - dest_offset;
 			let chunk_data_size = min(dest_left, data.len() - data_pos);
-			&mut chunk [dest_offset..][0..chunk_data_size].copy_from_slice(&data[data_pos..][0..chunk_data_size]);
+			&mut chunk[dest_offset..][0..chunk_data_size].copy_from_slice(&data[data_pos..][0..chunk_data_size]);
 			result = Self::send_apdu(&handle, commands::SIGN_ETH_TRANSACTION, p1, 0, &chunk[0..(dest_offset + chunk_data_size)])?;
 			dest_offset = 0;
 			data_pos += chunk_data_size;
@@ -236,11 +231,13 @@ impl Manager {
 		Ok(Signature::from_rsv(&r, &s, v))
 	}
 
-	fn open_path(&self, path: &str) -> Result<hidapi::HidDevice, Error> {
+	fn open_path<R, F>(&self, f: F) -> Result<R, Error>
+		where F: Fn() -> Result<R, &'static str>
+	{
 		let mut err = Error::KeyNotFound;
 		/// Try to open device a few times.
 		for _ in 0..10 {
-			match self.usb.open_path(&path) {
+			match f() {
 				Ok(handle) => return Ok(handle),
 				Err(e) => err = From::from(e),
 			}
@@ -253,33 +250,33 @@ impl Manager {
 		const HID_PACKET_SIZE: usize = 64 + HID_PREFIX_ZERO;
 		let mut offset = 0;
 		let mut chunk_index = 0;
-			loop {
-				let mut hid_chunk: [u8; HID_PACKET_SIZE] = [0; HID_PACKET_SIZE];
-				let mut chunk_size = if chunk_index == 0 { 12 } else { 5 };
-				let size = min(64 - chunk_size, data.len() - offset);
-				{
-					let mut chunk = &mut hid_chunk[HID_PREFIX_ZERO..];
-					&mut chunk[0..5].copy_from_slice(&[0x01, 0x01, APDU_TAG, (chunk_index >> 8) as u8, (chunk_index & 0xff) as u8 ]);
-
-					if chunk_index == 0 {
-						let data_len = data.len() + 5;
-						&mut chunk[5..12].copy_from_slice(&[ (data_len >> 8) as u8, (data_len & 0xff) as u8, APDU_CLA, command, p1, p2, data.len() as u8 ]);
-					}
-
-					&mut chunk[chunk_size..chunk_size + size].copy_from_slice(&data[offset..offset + size]);
-					offset += size;
-					chunk_size += size;
-				}
-				trace!("writing {:?}", &hid_chunk[..]);
-				let n = handle.write(&hid_chunk[..])?;
-				if n < chunk_size {
-					return Err(Error::Protocol("Write data size mismatch"));
-				}
-				if offset == data.len() {
-					break;
+		loop {
+			let mut hid_chunk: [u8; HID_PACKET_SIZE] = [0; HID_PACKET_SIZE];
+			let mut chunk_size = if chunk_index == 0 { 12 } else { 5 };
+			let size = min(64 - chunk_size, data.len() - offset);
+			{
+				let mut chunk = &mut hid_chunk[HID_PREFIX_ZERO..];
+				&mut chunk[0..5].copy_from_slice(&[0x01, 0x01, APDU_TAG, (chunk_index >> 8) as u8, (chunk_index & 0xff) as u8 ]);
+
+				if chunk_index == 0 {
+					let data_len = data.len() + 5;
+					&mut chunk[5..12].copy_from_slice(&[ (data_len >> 8) as u8, (data_len & 0xff) as u8, APDU_CLA, command, p1, p2, data.len() as u8 ]);
 				}
-				chunk_index += 1;
+
+				&mut chunk[chunk_size..chunk_size + size].copy_from_slice(&data[offset..offset + size]);
+				offset += size;
+				chunk_size += size;
+			}
+			trace!("writing {:?}", &hid_chunk[..]);
+			let n = handle.write(&hid_chunk[..])?;
+			if n < chunk_size {
+				return Err(Error::Protocol("Write data size mismatch"));
 			}
+			if offset == data.len() {
+				break;
+			}
+			chunk_index += 1;
+		}
 
 		// read response
 		chunk_index = 0;
@@ -303,20 +300,20 @@ impl Manager {
 				if chunk_size < 7 {
 					return Err(Error::Protocol("Unexpected chunk header"));
 				}
-				message_size = (chunk[5] as usize) << 8  | (chunk[6] as usize);
+				message_size = (chunk[5] as usize) << 8 | (chunk[6] as usize);
 				offset += 2;
 			}
 			message.extend_from_slice(&chunk[offset..chunk_size]);
 			message.truncate(message_size);
 			if message.len() == message_size {
 				break;
 			}
-			chunk_index +=1;
+			chunk_index += 1;
 		}
 		if message.len() < 2 {
 			return Err(Error::Protocol("No status word"));
 		}
-		let status = (message[message.len() - 2] as usize) << 8  | (message[message.len() - 1] as usize);
+		let status = (message[message.len() - 2] as usize) << 8 | (message[message.len() - 1] as usize);
 		debug!("Read status {:x}", status);
 		match status {
 			0x6700 => Err(Error::Protocol("Incorrect length")),
@@ -341,9 +338,10 @@ impl Manager {
 #[test]
 fn smoke() {
 	use rustc_hex::FromHex;
-	let mut manager = Manager::new().unwrap();
+	let hidapi = Arc::new(Mutex::new(hidapi::HidApi::new().unwrap()));
+	let manager = Manager::new(hidapi.clone());
 	manager.update_devices().unwrap();
-	for d in &manager.devices {
+	for d in &*manager.devices.read() {
 		println!("Device: {:?}", d);
 	}