Fixup sign_transaction; pass derivation_path by reference (#8194)

* Fixup sign_transaction; pass derivation_path by reference

* Pass total message length as BE u16

* Remove live integration tests (to ledger-app-solana)
This commit is contained in:
Tyera Eulberg 2020-02-11 11:45:00 -07:00 committed by GitHub
parent 517fe73734
commit 25d1f841ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 48 additions and 123 deletions

12
Cargo.lock generated
View File

@ -1396,7 +1396,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "hex"
version = "0.4.0"
version = "0.4.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
@ -3518,7 +3518,7 @@ dependencies = [
"bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
"crossbeam-channel 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
"ed25519-dalek 1.0.0-pre.1 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)",
"rand_chacha 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@ -3544,7 +3544,7 @@ dependencies = [
name = "solana-archiver-utils"
version = "0.24.0"
dependencies = [
"hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)",
"solana-chacha 0.24.0",
@ -4185,7 +4185,7 @@ dependencies = [
name = "solana-merkle-tree"
version = "0.24.0"
dependencies = [
"hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"solana-sdk 0.24.0",
]
@ -4386,7 +4386,7 @@ dependencies = [
"byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"ed25519-dalek 1.0.0-pre.1 (registry+https://github.com/rust-lang/crates.io-index)",
"generic-array 0.13.2 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"hmac 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
"itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
@ -6167,7 +6167,7 @@ dependencies = [
"checksum heck 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "20564e78d53d2bb135c343b3f47714a56af2061f1c928fdb541dc7b9fdd94205"
"checksum hermit-abi 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "307c3c9f937f38e3534b1d6447ecf090cafcc9744e4a6360e8b037b2cf5af120"
"checksum hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "805026a5d0141ffc30abb3be3173848ad46a1b1664fe632428479619a3644d77"
"checksum hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "023b39be39e3a2da62a94feb433e91e8bcd37676fbc8bea371daf52b7a769a3e"
"checksum hex 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "76cdda6bf525062a0c9e8f14ee2b37935c86b8efb6c8b69b3c83dfb518a914af"
"checksum hex-literal 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "961de220ec9a91af2e1e5bd80d02109155695e516771762381ef8581317066e0"
"checksum hex-literal-impl 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "06095d08c7c05760f11a071b3e1d4c5b723761c01bd8d7201c30a9536668a612"
"checksum hex_fmt 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b07f60793ff0a4d9cef0f18e63b5357e06209987153a64648c972c1e5aff336f"

View File

@ -481,7 +481,7 @@ impl CliConfig {
derivation_path = derivation;
}
let ledger = get_ledger_from_info(remote_wallet_info)?;
return Ok(ledger.get_pubkey(derivation_path)?);
return Ok(ledger.get_pubkey(&derivation_path)?);
}
}
Ok(self.keypair.pubkey())

View File

@ -108,7 +108,7 @@ fn get_pubkey_from_matches(
derivation_path = derivation;
}
let ledger = get_ledger_from_info(remote_wallet_info)?;
Ok(ledger.get_pubkey(derivation_path)?)
Ok(ledger.get_pubkey(&derivation_path)?)
} else {
read_keypair_file(keypair).map(|keypair| keypair.pubkey())
}

View File

@ -9,12 +9,10 @@ use std::{cmp::min, fmt, sync::Arc};
const APDU_TAG: u8 = 0x05;
const APDU_CLA: u8 = 0xe0;
const APDU_PAYLOAD_HEADER_LEN: usize = 7;
const APDU_PAYLOAD_HEADER_LEN: usize = 8;
const SOL_DERIVATION_PATH_BE: [u8; 8] = [0x80, 0, 0, 44, 0x80, 0, 0x01, 0xF5]; // 44'/501', Solana
// const SOL_DERIVATION_PATH_BE: [u8; 8] = [0x80, 0, 0, 44, 0x80, 0, 0x00, 0x94]; // 44'/148', Stellar
/// Ledger vendor ID
const LEDGER_VID: u16 = 0x2c97;
/// Ledger product IDs: Nano S and Nano X
@ -30,8 +28,6 @@ const LEDGER_NANO_X_PIDS: [u16; 33] = [
];
const LEDGER_TRANSPORT_HEADER_LEN: usize = 5;
const MAX_CHUNK_SIZE: usize = 255;
const HID_PACKET_SIZE: usize = 64 + HID_PREFIX_ZERO;
#[cfg(windows)]
@ -40,9 +36,10 @@ const HID_PREFIX_ZERO: usize = 1;
const HID_PREFIX_ZERO: usize = 0;
mod commands {
#[allow(dead_code)]
pub const GET_APP_CONFIGURATION: u8 = 0x06;
pub const GET_SOL_PUBKEY: u8 = 0x02;
pub const SIGN_SOL_TRANSACTION: u8 = 0x04;
pub const SIGN_SOL_TRANSACTION: u8 = 0x03;
}
/// Ledger Wallet device
@ -100,14 +97,15 @@ impl LedgerWallet {
]);
if sequence_number == 0 {
let data_len = data.len() + 5;
chunk[5..12].copy_from_slice(&[
let data_len = data.len() + 6;
chunk[5..13].copy_from_slice(&[
(data_len >> 8) as u8,
(data_len & 0xff) as u8,
APDU_CLA,
command,
p1,
p2,
(data.len() >> 8) as u8,
data.len() as u8,
]);
}
@ -224,7 +222,7 @@ impl LedgerWallet {
self.read()
}
fn get_firmware_version(&self) -> Result<FirmwareVersion, RemoteWalletError> {
fn _get_firmware_version(&self) -> Result<FirmwareVersion, RemoteWalletError> {
let ver = self.send_apdu(commands::GET_APP_CONFIGURATION, 0, 0, &[])?;
if ver.len() != 4 {
return Err(RemoteWalletError::Protocol("Version packet size mismatch"));
@ -258,7 +256,7 @@ impl RemoteWallet for LedgerWallet {
.serial_number
.clone()
.unwrap_or_else(|| "Unknown".to_owned());
self.get_pubkey(DerivationPath::default())
self.get_pubkey(&DerivationPath::default())
.map(|pubkey| RemoteWalletInfo {
model,
manufacturer,
@ -267,10 +265,15 @@ impl RemoteWallet for LedgerWallet {
})
}
fn get_pubkey(&self, derivation: DerivationPath) -> Result<Pubkey, RemoteWalletError> {
fn get_pubkey(&self, derivation: &DerivationPath) -> Result<Pubkey, RemoteWalletError> {
let derivation_path = get_derivation_path(derivation);
let key = self.send_apdu(commands::GET_SOL_PUBKEY, 0, 0, &derivation_path)?;
let key = self.send_apdu(
commands::GET_SOL_PUBKEY,
0, // In the naive implementation, default request is for no device confirmation
0,
&derivation_path,
)?;
if key.len() != 32 {
return Err(RemoteWalletError::Protocol("Key packet size mismatch"));
}
@ -279,44 +282,28 @@ impl RemoteWallet for LedgerWallet {
fn sign_transaction(
&self,
derivation: DerivationPath,
derivation: &DerivationPath,
transaction: Transaction,
) -> Result<Signature, RemoteWalletError> {
let mut chunk = [0_u8; MAX_CHUNK_SIZE];
let derivation_path = get_derivation_path(derivation);
let data = transaction.message_data();
let _firmware_version = self.get_firmware_version();
// Copy the address of the key (only done once)
chunk[0..derivation_path.len()].copy_from_slice(&derivation_path);
let key_length = derivation_path.len();
let max_payload_size = MAX_CHUNK_SIZE - key_length;
let data_len = data.len();
let mut result = Vec::new();
let mut offset = 0;
while offset < data_len {
let p1 = if offset == 0 { 0 } else { 0x80 };
let take = min(max_payload_size, data_len - offset);
// Fetch piece of data and copy it!
{
let (_key, d) = &mut chunk.split_at_mut(key_length);
let (dst, _rem) = &mut d.split_at_mut(take);
dst.copy_from_slice(&data[offset..(offset + take)]);
}
result = self.send_apdu(
commands::SIGN_SOL_TRANSACTION,
p1,
0,
&chunk[0..(key_length + take)],
)?;
offset += take;
let mut payload = get_derivation_path(derivation);
let mut data = transaction.message_data();
if data.len() > u16::max_value() as usize {
return Err(RemoteWalletError::InvalidInput(
"Message to sign is too long".to_string(),
));
}
for byte in (data.len() as u16).to_be_bytes().iter() {
payload.push(*byte);
}
payload.append(&mut data);
trace!("Serialized payload length {:?}", payload.len());
let result = self.send_apdu(
commands::SIGN_SOL_TRANSACTION,
1, // In the naive implementation, default request is for requred device confirmation
0,
&payload,
)?;
if result.len() != 64 {
return Err(RemoteWalletError::Protocol(
@ -334,7 +321,7 @@ pub fn is_valid_ledger(vendor_id: u16, product_id: u16) -> bool {
}
/// Build the derivation path byte array from a DerivationPath selection
fn get_derivation_path(derivation: DerivationPath) -> Vec<u8> {
fn get_derivation_path(derivation: &DerivationPath) -> Vec<u8> {
let byte = if derivation.change.is_some() { 4 } else { 3 };
let mut concat_derivation = vec![byte];
concat_derivation.extend_from_slice(&SOL_DERIVATION_PATH_BE);
@ -375,68 +362,3 @@ pub fn get_ledger_from_info(
};
wallet_manager.get_ledger(&wallet_base_pubkey)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::remote_wallet::initialize_wallet_manager;
use std::collections::HashSet;
/// This test can't be run without an actual ledger device connected with the `Ledger Wallet Solana application` running
#[test]
#[ignore]
fn ledger_pubkey_test() {
let wallet_manager = initialize_wallet_manager();
// Update device list
wallet_manager.update_devices().expect("No Ledger found, make sure you have a unlocked Ledger connected with the Ledger Wallet Solana running");
assert!(wallet_manager.list_devices().len() > 0);
// Fetch the base pubkey of a connected ledger device
let ledger_base_pubkey = wallet_manager
.list_devices()
.iter()
.filter(|d| d.manufacturer == "ledger".to_string())
.nth(0)
.map(|d| d.pubkey.clone())
.expect("No ledger device detected");
let ledger = wallet_manager
.get_ledger(&ledger_base_pubkey)
.expect("get device");
let mut pubkey_set = HashSet::new();
pubkey_set.insert(ledger_base_pubkey);
let pubkey_0_0 = ledger
.get_pubkey(DerivationPath {
account: 0,
change: Some(0),
})
.expect("get pubkey");
pubkey_set.insert(pubkey_0_0);
let pubkey_0_1 = ledger
.get_pubkey(DerivationPath {
account: 0,
change: Some(1),
})
.expect("get pubkey");
pubkey_set.insert(pubkey_0_1);
let pubkey_1 = ledger
.get_pubkey(DerivationPath {
account: 1,
change: None,
})
.expect("get pubkey");
pubkey_set.insert(pubkey_1);
let pubkey_1_0 = ledger
.get_pubkey(DerivationPath {
account: 1,
change: Some(0),
})
.expect("get pubkey");
pubkey_set.insert(pubkey_1_0);
assert_eq!(pubkey_set.len(), 5); // Ensure keys at various derivation paths are unique
}
}

View File

@ -28,6 +28,9 @@ pub enum RemoteWalletError {
#[error("invalid derivation path: {0}")]
InvalidDerivationPath(String),
#[error("invalid input: {0}")]
InvalidInput(String),
#[error("invalid path: {0}")]
InvalidPath(String),
@ -150,12 +153,12 @@ pub trait RemoteWallet {
) -> Result<RemoteWalletInfo, RemoteWalletError>;
/// Get solana pubkey from a RemoteWallet
fn get_pubkey(&self, derivation: DerivationPath) -> Result<Pubkey, RemoteWalletError>;
fn get_pubkey(&self, derivation: &DerivationPath) -> Result<Pubkey, RemoteWalletError>;
/// Sign transaction data with wallet managing pubkey at derivation path m/44'/501'/<account>'/<change>'.
fn sign_transaction(
&self,
derivation: DerivationPath,
derivation: &DerivationPath,
transaction: Transaction,
) -> Result<Signature, RemoteWalletError>;
}