diff --git a/Cargo.lock b/Cargo.lock index 05d219cd09..0b048ff798 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1397,7 +1397,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]] @@ -3446,7 +3446,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)", @@ -3472,7 +3472,7 @@ dependencies = [ name = "solana-archiver-utils" version = "0.23.3" 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)", "rand_chacha 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -4143,7 +4143,7 @@ dependencies = [ name = "solana-merkle-tree" version = "0.23.3" 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.23.3", ] @@ -4354,7 +4354,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)", @@ -6179,7 +6179,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" diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 380b326238..f15f0d6da2 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -478,7 +478,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()) diff --git a/keygen/src/keygen.rs b/keygen/src/keygen.rs index 96aaef3d2d..8aafc9ea54 100644 --- a/keygen/src/keygen.rs +++ b/keygen/src/keygen.rs @@ -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()) } diff --git a/remote-wallet/src/ledger.rs b/remote-wallet/src/ledger.rs index 9de9a61d06..b8b90e8734 100644 --- a/remote-wallet/src/ledger.rs +++ b/remote-wallet/src/ledger.rs @@ -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 { + fn _get_firmware_version(&self) -> Result { 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 { + fn get_pubkey(&self, derivation: &DerivationPath) -> Result { 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 { - 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 { +fn get_derivation_path(derivation: &DerivationPath) -> Vec { 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 - } -} diff --git a/remote-wallet/src/remote_wallet.rs b/remote-wallet/src/remote_wallet.rs index 7e55f50d65..9dd8d7b2d8 100644 --- a/remote-wallet/src/remote_wallet.rs +++ b/remote-wallet/src/remote_wallet.rs @@ -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; /// Get solana pubkey from a RemoteWallet - fn get_pubkey(&self, derivation: DerivationPath) -> Result; + fn get_pubkey(&self, derivation: &DerivationPath) -> Result; /// Sign transaction data with wallet managing pubkey at derivation path m/44'/501'/'/'. fn sign_transaction( &self, - derivation: DerivationPath, + derivation: &DerivationPath, transaction: Transaction, ) -> Result; }