Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Trezor Support #6403

Merged
merged 44 commits into from
Sep 14, 2017
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
1c8329d
Copy modal from keepkey branch and generalize
folsen Aug 23, 2017
298d9d8
Add trezor communication code
folsen Aug 23, 2017
38b4c1d
Extend the basic lib to allow Trezor
folsen Aug 23, 2017
f3a902b
Add RPC plumbing needed
folsen Aug 23, 2017
aa3f62e
Add logic to query backend for Trezor and display PinMatrix
folsen Aug 23, 2017
847aa76
Change back to paritytech branch
folsen Aug 24, 2017
3de2d0c
Converting spaces to tabs, as it should be
folsen Aug 24, 2017
2e08493
Incorporate correct handling of EIP-155
folsen Aug 27, 2017
b5dc34e
Some circular logic here that was incorrect
folsen Aug 27, 2017
5be769a
Resolve issue where not clicking fast enough fails
folsen Aug 28, 2017
2763a2e
Scan after pin entry to make accepting it faster
folsen Aug 28, 2017
5f20d51
Remove ability to cancel pin request
folsen Aug 28, 2017
e07c275
Some slight cleanup
folsen Aug 28, 2017
2677d90
Probe for the correct HID Version to determine padding
folsen Aug 28, 2017
f578cfe
Move the PinMatrix from Accounts to Application
folsen Aug 28, 2017
32cfa2f
Removing unused dependencies
folsen Aug 28, 2017
fc22db3
Mistake in copying over stuff from keepkey branch
folsen Aug 28, 2017
ec58a6b
Simplify FormattedMessage
folsen Aug 28, 2017
f344bca
Move generated code to external crate
folsen Aug 29, 2017
9255964
Remove ethcore-util dependency
folsen Aug 29, 2017
1a53ddc
Fix broken import in test
folsen Aug 29, 2017
bacc06f
Merge branch 'master' into fh-4500-trezor-support
folsen Aug 29, 2017
e998189
Merge branch 'master' into fh-4500-trezor-support
folsen Aug 29, 2017
1135ec3
Ignore test that can't be run without trezor device
folsen Aug 30, 2017
2bff086
Fixing grumbles
folsen Aug 30, 2017
1666f78
Fixing UI.
tomusdrw Aug 31, 2017
d56aca9
Debugging trezor.
tomusdrw Aug 31, 2017
1a43dd2
Minor styling tweak
folsen Sep 5, 2017
2fe7170
Merge branch 'master' into fh-4500-trezor-support
folsen Sep 5, 2017
c5e9997
Make message type into an actual type
folsen Sep 5, 2017
1f5d4f7
Merge branch 'master' into fh-4500-trezor-support
folsen Sep 12, 2017
56bc44d
Split the trezor RPC endpoint
folsen Sep 12, 2017
7ff4c81
Reflect RPC method split in javascript
folsen Sep 12, 2017
7e0853c
Fix bug with pin entry
folsen Sep 12, 2017
31e64c0
Fix deadlock for Ledger
folsen Sep 13, 2017
5463246
Avoid having a USB lock in just listing locked wallets
folsen Sep 13, 2017
66d9f3e
Fix javascript issue (see #6509)
folsen Sep 13, 2017
8f965e8
Replace Mutex with RwLock
folsen Sep 13, 2017
918fce3
Update Ledger test
folsen Sep 13, 2017
3df337d
Fix typo causing faulty signatures (sometimes)
folsen Sep 13, 2017
203e121
*Actually* fix tests
folsen Sep 13, 2017
e81ccbc
Update git submodule
folsen Sep 13, 2017
bfce1a1
Swap line orders to prevent possible deadlock
folsen Sep 14, 2017
c4d14ad
Make setPinMatrixRequest an @action
folsen Sep 14, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 25 additions & 3 deletions ethcore/src/account_provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -288,6 +289,15 @@ impl AccountProvider {
Ok(accounts.into_iter().map(|a| a.address).collect())
}

/// Communicate with Trezor hardware wallet
pub fn trezor_message(&self, message_type: String, path: Option<String>, message: Option<String>) -> Result<String, SignError> {
match self.hardware_store.as_ref().map(|h| h.trezor_message(message_type, path, message)) {
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.
Expand Down Expand Up @@ -779,8 +789,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, network_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(),
network_id: network_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),
Expand Down
4 changes: 4 additions & 0 deletions hw/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ authors = ["Parity Technologies <admin@parity.io>"]
[dependencies]
log = "0.3"
parking_lot = "0.4"
protobuf = "1.4"
serde = "1.0"
serde_json = "1.0"
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" }

Expand Down
121 changes: 60 additions & 61 deletions hw/src/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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;
Expand All @@ -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.
Expand Down Expand Up @@ -84,7 +79,7 @@ impl From<hidapi::HidError> for Error {

/// Ledger device manager.
pub struct Manager {
usb: hidapi::HidApi,
usb: Arc<Mutex<hidapi::HidApi>>,
devices: Vec<Device>,
key_path: KeyPath,
}
Expand All @@ -97,19 +92,19 @@ struct Device {

impl Manager {
/// Create a new instance.
pub fn new() -> Result<Manager, Error> {
let manager = Manager {
usb: hidapi::HidApi::new()?,
pub fn new(hidapi: Arc<Mutex<hidapi::HidApi>>) -> Manager {
Manager {
usb: hidapi,
devices: Vec::new(),
key_path: KeyPath::Ethereum,
};
Ok(manager)
}
}

/// 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();
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 {
Expand All @@ -125,7 +120,7 @@ impl Manager {
}
new_devices.push(info);

},
}
Err(e) => debug!("Error reading device info: {}", e),
};
}
Expand All @@ -139,7 +134,8 @@ impl Manager {
}

fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo) -> Result<Device, Error> {
let mut handle = self.open_path(&dev_info.path)?;
let usb = self.usb.lock();
let mut handle = self.open_path(|| usb.open_path(&dev_info.path))?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may keep the lock for the entire 2s in a worst case. If there is another RPC query to read some other devices it will cause the whole RPC to be blocked.
We should consider running hw wallets RPC methods through cpupool.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also change open_path to return a handle and the lock guard?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually upon further inspection I'm not entirely clear on what the goal of returning the guard from open_path would be, or indeed what that would even mean, open_path isn't really aware that it's handling a locked item?

To clear up the lock "issues" I propose (in a separate PR) that we refactor the whole HW wallet interface, and implement a device-dispatcher and unified trait for all kinds of devices, so adding new devices in the future just means implementing the wire protocol.

let address = Self::read_wallet_address(&mut handle, self.key_path)?;
let manufacturer = dev_info.manufacturer_string.clone().unwrap_or("Unknown".to_owned());
let name = dev_info.product_string.clone().unwrap_or("Unknown".to_owned());
Expand Down Expand Up @@ -197,10 +193,10 @@ impl Manager {

/// 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 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 handle = self.open_path(|| usb.open_path(&device.path))?;

let eth_path = &ETH_DERIVATION_PATH_BE[..];
let etc_path = &ETC_DERIVATION_PATH_BE[..];
Expand All @@ -218,7 +214,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;
Expand All @@ -236,11 +232,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),
}
Expand All @@ -253,33 +251,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;
Expand All @@ -303,20 +301,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")),
Expand All @@ -341,7 +339,8 @@ 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 mut manager = Manager::new(hidapi.clone());
manager.update_devices().unwrap();
for d in &manager.devices {
println!("Device: {:?}", d);
Expand Down
Loading