Skip to content

Commit

Permalink
fix: branch keys (#6413)
Browse files Browse the repository at this point in the history
Description
---
This corrects mismatched branch key matching as we've had a lot of code
churn and they've changed over the last few days.

Also make a special code path for fetching the ledger comms key.

Motivation and Context
---

How Has This Been Tested?
---
Does the wallet start: Yes

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
brianp authored Jul 19, 2024
1 parent 4327ca7 commit 47e3761
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub fn handler_get_dh_shared_secret(comm: &mut Comm) -> Result<(), AppSW> {
let mut key_bytes = [0u8; 8];
key_bytes.clone_from_slice(&data[16..24]);
let key_int = u64::from_le_bytes(key_bytes);
let key = KeyType::from_branch_key(key_int);
let key = KeyType::from_branch_key(key_int)?;

let public_key: RistrettoPublicKey = get_key_from_canonical_bytes(&data[24..56])?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn handler_get_public_key(comm: &mut Comm) -> Result<(), AppSW> {
let mut key_bytes = [0u8; 8];
key_bytes.clone_from_slice(&data[16..24]);
let key_int = u64::from_le_bytes(key_bytes);
let key = KeyType::from_branch_key(key_int);
let key = KeyType::from_branch_key(key_int)?;

let pk = match derive_from_bip32_key(account, index, key) {
Ok(k) => RistrettoPublicKey::from_secret_key(&k),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
use ledger_device_sdk::io::Comm;
use tari_crypto::{keys::PublicKey, ristretto::RistrettoPublicKey, tari_utilities::ByteArray};

use crate::{utils::derive_from_bip32_key, AppSW, KeyType, RESPONSE_VERSION, STATIC_ALPHA_INDEX};
use crate::{utils::derive_from_bip32_key, AppSW, KeyType, RESPONSE_VERSION, STATIC_SPEND_INDEX};

pub fn handler_get_public_alpha(comm: &mut Comm) -> Result<(), AppSW> {
pub fn handler_get_public_spend_key(comm: &mut Comm) -> Result<(), AppSW> {
let data = comm.get_data().map_err(|_| AppSW::WrongApduLength)?;

let mut account_bytes = [0u8; 8];
account_bytes.clone_from_slice(&data[0..8]);
let account = u64::from_le_bytes(account_bytes);

let pk = match derive_from_bip32_key(account, STATIC_ALPHA_INDEX, KeyType::Alpha) {
let pk = match derive_from_bip32_key(account, STATIC_SPEND_INDEX, KeyType::Spend) {
Ok(k) => RistrettoPublicKey::from_secret_key(&k),
Err(e) => return Err(e),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
AppSW,
KeyType,
RESPONSE_VERSION,
STATIC_ALPHA_INDEX,
STATIC_SPEND_INDEX,
};

const MIN_UNIQUE_KEYS: usize = 2;
Expand Down Expand Up @@ -110,7 +110,7 @@ pub fn handler_get_script_offset(
index_bytes.clone_from_slice(&data[0..8]);
let index = u64::from_le_bytes(index_bytes);

let offset = derive_from_bip32_key(offset_ctx.account, index, KeyType::SenderOffset)?;
let offset = derive_from_bip32_key(offset_ctx.account, index, KeyType::OneSidedSenderOffset)?;
offset_ctx.add_unique_key(offset.clone());
offset_ctx.total_sender_offset_private_key =
Zeroizing::new(offset_ctx.total_sender_offset_private_key.deref() + offset.deref());
Expand All @@ -119,7 +119,7 @@ pub fn handler_get_script_offset(
let end_commitment_keys = end_offset_indexes + offset_ctx.total_commitment_keys;

if (end_offset_indexes..end_commitment_keys).contains(&(chunk as u64)) {
let alpha = derive_from_bip32_key(offset_ctx.account, STATIC_ALPHA_INDEX, KeyType::Alpha)?;
let alpha = derive_from_bip32_key(offset_ctx.account, STATIC_SPEND_INDEX, KeyType::Spend)?;
let blinding_factor: Zeroizing<RistrettoSecretKey> =
get_key_from_canonical_bytes::<RistrettoSecretKey>(&data[0..32])?.into();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
AppSW,
KeyType,
RESPONSE_VERSION,
STATIC_ALPHA_INDEX,
STATIC_SPEND_INDEX,
};

pub fn handler_get_script_signature(comm: &mut Comm) -> Result<(), AppSW> {
Expand All @@ -44,7 +44,7 @@ pub fn handler_get_script_signature(comm: &mut Comm) -> Result<(), AppSW> {
txi_version_bytes.clone_from_slice(&data[16..24]);
let txi_version = u64::from_le_bytes(txi_version_bytes);

let alpha = derive_from_bip32_key(account, STATIC_ALPHA_INDEX, KeyType::Alpha)?;
let alpha = derive_from_bip32_key(account, STATIC_SPEND_INDEX, KeyType::Spend)?;
let blinding_factor: Zeroizing<RistrettoSecretKey> =
get_key_from_canonical_bytes::<RistrettoSecretKey>(&data[24..56])?.into();
let script_private_key = alpha_hasher(alpha, blinding_factor)?;
Expand Down
30 changes: 16 additions & 14 deletions applications/minotari_ledger_wallet/wallet/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ mod app_ui {
}
mod handlers {
pub mod get_dh_shared_secret;
pub mod get_public_alpha;
pub mod get_public_key;
pub mod get_public_spend_key;
pub mod get_script_offset;
pub mod get_script_signature;
pub mod get_version;
Expand All @@ -29,8 +29,8 @@ use app_ui::menu::ui_menu_main;
use critical_section::RawRestoreState;
use handlers::{
get_dh_shared_secret::handler_get_dh_shared_secret,
get_public_alpha::handler_get_public_alpha,
get_public_key::handler_get_public_key,
get_public_spend_key::handler_get_public_spend_key,
get_script_offset::{handler_get_script_offset, ScriptOffsetCtx},
get_script_signature::handler_get_script_signature,
get_version::handler_get_version,
Expand Down Expand Up @@ -94,6 +94,7 @@ pub enum AppSW {
ScriptSignatureFail = 0xB001,
MetadataSignatureFail = 0xB002,
ScriptOffsetNotUnique = 0xB004,
BadBranchKey = 0xB005,
KeyDeriveFail = 0xB009,
KeyDeriveFromCanonical = 0xB010,
KeyDeriveFromUniform = 0xB011,
Expand All @@ -114,7 +115,7 @@ pub enum Instruction {
GetVersion,
GetAppName,
GetPublicKey,
GetPublicAlpha,
GetPublicSpendKey,
GetScriptSignature,
GetScriptOffset { chunk: u8, more: bool },
GetScriptSignatureFromChallenge,
Expand All @@ -123,29 +124,30 @@ pub enum Instruction {
}

const P2_MORE: u8 = 0x01;
const STATIC_ALPHA_INDEX: u64 = 42;
const STATIC_SPEND_INDEX: u64 = 42;
const STATIC_VIEW_INDEX: u64 = 57311; // No significance, just a random number by large dice roll
const MAX_PAYLOADS: u8 = 250;

#[repr(u8)]
pub enum KeyType {
Alpha = 0x01,
Spend = 0x01,
Nonce = 0x02,
Recovery = 0x03,
SenderOffset = 0x04,
ViewKey = 0x05,
ViewKey = 0x03,
OneSidedSenderOffset = 0x04,
}

impl KeyType {
pub fn as_byte(self) -> u8 {
self as u8
}

fn from_branch_key(n: u64) -> Self {
fn from_branch_key(n: u64) -> Result<Self, AppSW> {
// These numbers need to match the TransactionKeyManagerBranches in:
// base_layer/core/src/transactions/key_manager/interface.rs
match n {
1 => Self::Alpha,
7 => Self::SenderOffset,
5 | 2 | _ => Self::Nonce,
7 => Ok(Self::Spend),
6 => Ok(Self::OneSidedSenderOffset),
_ => Err(AppSW::BadBranchKey),
}
}
}
Expand All @@ -168,7 +170,7 @@ impl TryFrom<ApduHeader> for Instruction {
match (value.ins, value.p1, value.p2) {
(0x01, 0, 0) => Ok(Instruction::GetVersion),
(0x02, 0, 0) => Ok(Instruction::GetAppName),
(0x03, 0, 0) => Ok(Instruction::GetPublicAlpha),
(0x03, 0, 0) => Ok(Instruction::GetPublicSpendKey),
(0x04, 0, 0) => Ok(Instruction::GetPublicKey),
(0x05, 0, 0) => Ok(Instruction::GetScriptSignature),
(0x06, 0..=MAX_PAYLOADS, 0 | P2_MORE) => Ok(Instruction::GetScriptOffset {
Expand Down Expand Up @@ -220,7 +222,7 @@ fn handle_apdu(comm: &mut Comm, ins: Instruction, offset_ctx: &mut ScriptOffsetC
Ok(())
},
Instruction::GetPublicKey => handler_get_public_key(comm),
Instruction::GetPublicAlpha => handler_get_public_alpha(comm),
Instruction::GetPublicSpendKey => handler_get_public_spend_key(comm),
Instruction::GetScriptSignature => handler_get_script_signature(comm),
Instruction::GetScriptOffset { chunk, more } => handler_get_script_offset(comm, chunk, more, offset_ctx),
Instruction::GetScriptSignatureFromChallenge => handler_get_script_signature_from_challenge(comm),
Expand Down
27 changes: 22 additions & 5 deletions base_layer/core/src/transactions/key_manager/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,11 +458,28 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
}

async fn get_private_comms_key(&self) -> Result<PrivateKey, KeyManagerServiceError> {
self.get_private_key(&TariKeyId::Managed {
branch: TransactionKeyManagerBranch::Spend.get_branch_key(),
index: 0,
})
.await
let branch = TransactionKeyManagerBranch::Spend.get_branch_key();
let index = 0;

match self.wallet_type {
WalletType::Software | WalletType::Imported(_) => {
self.get_private_key(&TariKeyId::Managed {
branch: branch.clone(),
index,
})
.await
},
WalletType::Ledger(_) => {
let km = self
.key_managers
.get(&branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.read()
.await;
let key = km.get_private_key(index)?;
Ok(key)
},
}
}

fn get_domain_hasher(
Expand Down
2 changes: 2 additions & 0 deletions base_layer/core/src/transactions/key_manager/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub enum TxoStage {

#[repr(u8)]
#[derive(Clone, Copy, EnumIter)]
// These byte reps must stay in sync with the ledger representations at:
// applications/minotari_ledger_wallet/wallet/src/main.rs
pub enum TransactionKeyManagerBranch {
DataEncryption = 0x00,
MetadataEphemeralNonce = 0x01,
Expand Down

0 comments on commit 47e3761

Please sign in to comment.