feat: various safety improvements to ledger wallet code (#6494)

Description
---
- Applied proper random nonces in the one-sided metadata signature to be
collision resistant.
- Removed double zeroizing on 'RistrettoSecretKey'.
- Ensured hasher output is zeroized.
- Changed Ledger BIP32 derivation to use 'secp256k1'.

Motivation and Context
---
See #6484, #6485, #6488 and #6490.

How Has This Been Tested?
---
System-level testing using a ledger device with `cargo run --release
--example ledger_demo`

What process can a PR reviewer use to test or verify this change?
---
- Code review
- `cargo run --release --example ledger_demo`

<!-- 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
---

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

<!-- Does this include a breaking change? If so, include this line as a
footer -->
BREAKING CHANGE: All ledger derived keys and signatures will be
different.

applications/minotari_ledger_wallet/common/src/lib.rs

@@ -11,7 +11,7 @@ extern crate alloc;
 pub mod common_types;
 mod utils;
 pub use utils::{
-    get_public_spend_key_from_tari_dual_address,
+    get_public_spend_key_bytes_from_tari_dual_address,
     hex_to_bytes_serialized,
     tari_dual_address_display,
     PUSH_PUBKEY_IDENTIFIER,

applications/minotari_ledger_wallet/common/src/utils.rs

@@ -68,13 +68,13 @@ pub fn tari_dual_address_display(address_bytes: &[u8; TARI_DUAL_ADDRESS_SIZE]) -
 }
 
 /// Get the public spend key bytes from a serialized Tari dual address
-pub fn get_public_spend_key_from_tari_dual_address(
+pub fn get_public_spend_key_bytes_from_tari_dual_address(
     address_bytes: &[u8; TARI_DUAL_ADDRESS_SIZE],
 ) -> Result<[u8; 32], String> {
     validate_checksum(address_bytes.as_ref())?;
-    let mut public_spend_key = [0u8; 32];
-    public_spend_key.copy_from_slice(&address_bytes[34..66]);
-    Ok(public_spend_key)
+    let mut public_spend_key_bytes = [0u8; 32];
+    public_spend_key_bytes.copy_from_slice(&address_bytes[34..66]);
+    Ok(public_spend_key_bytes)
 }
 
 // Determine whether a byte slice ends with a valid checksum

applications/minotari_ledger_wallet/comms/examples/ledger_demo/main.rs

@@ -222,13 +222,14 @@ fn main() {
     // GetViewKey
     println!("\ntest: GetViewKey");
 
-    match ledger_get_view_key(account) {
-        Ok(view_key) => println!("view_key:       {}", view_key.to_hex()),
+    let view_key_1 = match ledger_get_view_key(account) {
+        Ok(val) => val,
         Err(e) => {
             println!("\nError: {}\n", e);
             return;
         },
-    }
+    };
+    println!("view_key:       {}", view_key_1.to_hex());
 
     // GetDHSharedSecret
     println!("\ntest: GetDHSharedSecret");
@@ -384,7 +385,10 @@ fn main() {
     println!("\ntest: Ledger app restart");
     prompt_with_message("Start the 'MinoTari Wallet' Ledger app and press Enter to continue..");
     match ledger_get_view_key(account) {
-        Ok(view_key) => println!("view_key:       {}", view_key.to_hex()),
+        Ok(view_key_2) => {
+            println!("view_key:       {}", view_key_2.to_hex());
+            assert_eq!(view_key_1, view_key_2, "View key not repeatable")
+        },
         Err(e) => {
             println!("\nError: {}\n", e);
             return;

applications/minotari_ledger_wallet/comms/src/accessor_methods.rs

@@ -126,25 +126,25 @@ fn verify() -> Result<(), LedgerDeviceError> {
             Ok(public_key) => {
                 if !signature.verify(&public_key, nonce) {
                     return Err(LedgerDeviceError::Processing(
-                        "'Minotari Wallet' application could not create a valid signature. Please update the firmware \
-                         on your device."
+                        "Error 1: 'Minotari Wallet' application could not create a valid signature. Please update the \
+                         firmware on your device."
                             .to_string(),
                     ));
                 }
                 signature
             },
             Err(e) => {
                 return Err(LedgerDeviceError::Processing(format!(
-                    "'Minotari Wallet' application could not retrieve a public key ({:?}). Please update the firmware \
-                     on your device.",
+                    "Error 2: 'Minotari Wallet' application could not retrieve a public key ({:?}). Please update the \
+                     firmware on your device.",
                     e
                 )))
             },
         },
         Err(e) => {
             return Err(LedgerDeviceError::Processing(format!(
-                "'Minotari Wallet' application could not create a signature ({:?}). Please update the firmware on \
-                 your device.",
+                "Error 3: 'Minotari Wallet' application could not create a signature ({:?}). Please update the \
+                 firmware on your device.",
                 e
             )))
         },
@@ -153,16 +153,16 @@ fn verify() -> Result<(), LedgerDeviceError> {
         Ok(signature_b) => {
             if signature_a == signature_b {
                 return Err(LedgerDeviceError::Processing(
-                    "'Minotari Wallet' application is not creating unique signatures. Please update the firmware on \
-                     your device."
+                    "Error 4: 'Minotari Wallet' application is not creating unique signatures. Please update the \
+                     firmware on your device."
                         .to_string(),
                 ));
             }
         },
         Err(e) => {
             return Err(LedgerDeviceError::Processing(format!(
-                "'Minotari Wallet' application could not create a signature ({:?}). Please update the firmware on \
-                 your device.",
+                "Error 5: 'Minotari Wallet' application could not create a signature ({:?}). Please update the \
+                 firmware on your device.",
                 e
             )))
         },

applications/minotari_ledger_wallet/comms/src/lib.rs

@@ -28,7 +28,7 @@ pub mod ledger_wallet;
 mod test {
     use borsh::BorshSerialize;
     use minotari_ledger_wallet_common::{
-        get_public_spend_key_from_tari_dual_address,
+        get_public_spend_key_bytes_from_tari_dual_address,
         hex_to_bytes_serialized,
         tari_dual_address_display,
         PUSH_PUBKEY_IDENTIFIER,
@@ -111,7 +111,7 @@ mod test {
         );
         // Getting the public spend key from the address
         assert_eq!(
-            get_public_spend_key_from_tari_dual_address(&tari_address_bytes)
+            get_public_spend_key_bytes_from_tari_dual_address(&tari_address_bytes)
                 .unwrap()
                 .to_vec(),
             tari_address.public_spend_key().to_vec()

applications/minotari_ledger_wallet/wallet/Cargo.toml

@@ -39,7 +39,7 @@ default = []
 pending_review_screen = []
 
 [package.metadata.ledger]
-curve = ["ed25519"]
+curve = ["secp256k1"]
 flags = "0"
 path = ["44'/535348'"]
 name = "MinoTari Wallet"

applications/minotari_ledger_wallet/wallet/src/handlers/get_dh_shared_secret.rs

@@ -5,6 +5,7 @@ use core::ops::Deref;
 
 use ledger_device_sdk::{io::Comm, ui::gadgets::SingleMessage};
 use tari_crypto::{ristretto::RistrettoPublicKey, tari_utilities::ByteArray};
+use zeroize::Zeroizing;
 
 use crate::{
     utils::{derive_from_bip32_key, get_key_from_canonical_bytes},
@@ -36,12 +37,12 @@ pub fn handler_get_dh_shared_secret(comm: &mut Comm) -> Result<(), AppSW> {
     let public_key: RistrettoPublicKey = get_key_from_canonical_bytes(&data[24..56])?;
 
     let shared_secret_key = match derive_from_bip32_key(account, index, key) {
-        Ok(k) => k.deref() * public_key,
+        Ok(k) => Zeroizing::new(k * public_key),
         Err(e) => return Err(e),
     };
 
     comm.append(&[RESPONSE_VERSION]); // version
-    comm.append(shared_secret_key.as_bytes());
+    comm.append(shared_secret_key.deref().as_bytes());
     comm.reply_ok();
 
     Ok(())

applications/minotari_ledger_wallet/wallet/src/handlers/get_one_sided_metadata_signature.rs

@@ -2,20 +2,21 @@
 // SPDX-License-Identifier: BSD-3-Clause
 
 use alloc::{format, string::String};
-use core::ops::Deref;
 
 use blake2::Blake2b;
-use digest::consts::{U32, U64};
+use digest::{
+    consts::{U32, U64},
+    Digest,
+};
 use ledger_device_sdk::{
     io::Comm,
-    random::Random,
     ui::{
         bitmaps::{CROSSMARK, EYE, VALIDATE_14},
         gadgets::{Field, MultiFieldReview, SingleMessage},
     },
 };
 use minotari_ledger_wallet_common::{
-    get_public_spend_key_from_tari_dual_address,
+    get_public_spend_key_bytes_from_tari_dual_address,
     hex_to_bytes_serialized,
     tari_dual_address_display,
     PUSH_PUBKEY_IDENTIFIER,
@@ -39,7 +40,7 @@ use zeroize::Zeroizing;
 use crate::{
     alloc::string::ToString,
     hashing::DomainSeparatedConsensusHasher,
-    utils::{derive_from_bip32_key, get_key_from_canonical_bytes, get_key_from_uniform_bytes},
+    utils::{derive_from_bip32_key, get_key_from_canonical_bytes, get_key_from_uniform_bytes, get_random_nonce},
     AppSW,
     KeyType,
     RESPONSE_VERSION,
@@ -69,8 +70,7 @@ pub fn handler_get_one_sided_metadata_signature(comm: &mut Comm) -> Result<(), A
     let value_u64 = u64::from_le_bytes(value_bytes);
     let value = Minotari::new(u64::from_le_bytes(value_bytes));
 
-    let commitment_mask: Zeroizing<RistrettoSecretKey> =
-        get_key_from_canonical_bytes::<RistrettoSecretKey>(&data[40..72])?.into();
+    let commitment_mask: RistrettoSecretKey = get_key_from_canonical_bytes::<RistrettoSecretKey>(&data[40..72])?.into();
 
     let mut receiver_address_bytes = [0u8; TARI_DUAL_ADDRESS_SIZE]; // 67 bytes
     receiver_address_bytes.clone_from_slice(&data[72..139]);
@@ -109,25 +109,25 @@ pub fn handler_get_one_sided_metadata_signature(comm: &mut Comm) -> Result<(), A
         return Err(AppSW::UserCancelled);
     }
 
-    let value_as_private_key: Zeroizing<RistrettoSecretKey> = Zeroizing::new(value_u64.into());
+    let value_as_private_key: RistrettoSecretKey = value_u64.into();
 
     let sender_offset_private_key =
         derive_from_bip32_key(account, sender_offset_key_index, KeyType::OneSidedSenderOffset)?;
     let sender_offset_public_key = RistrettoPublicKey::from_secret_key(&sender_offset_private_key);
 
-    let r_a = derive_from_bip32_key(account, u32::random().into(), KeyType::Nonce)?;
-    let r_x = derive_from_bip32_key(account, u32::random().into(), KeyType::Nonce)?;
-    let ephemeral_private_key = derive_from_bip32_key(account, u32::random().into(), KeyType::Nonce)?;
+    let r_a = get_random_nonce()?;
+    let r_x = get_random_nonce()?;
+    let ephemeral_private_key = get_random_nonce()?;
 
     let factory = ExtendedPedersenCommitmentFactory::default();
 
-    let commitment = factory.commit(&commitment_mask, value_as_private_key.deref());
+    let commitment = factory.commit(&commitment_mask, &value_as_private_key);
     let ephemeral_commitment = factory.commit(&r_x, &r_a);
     let ephemeral_pubkey = RistrettoPublicKey::from_secret_key(&ephemeral_private_key);
 
-    let receiver_public_spend_key: Zeroizing<RistrettoPublicKey> =
-        match get_public_spend_key_from_tari_dual_address(&receiver_address_bytes) {
-            Ok(bytes) => get_key_from_canonical_bytes::<RistrettoPublicKey>(&bytes)?.into(),
+    let receiver_public_spend_key: RistrettoPublicKey =
+        match get_public_spend_key_bytes_from_tari_dual_address(&receiver_address_bytes) {
+            Ok(bytes) => get_key_from_canonical_bytes::<RistrettoPublicKey>(&bytes)?,
             Err(e) => {
                 SingleMessage::new(&format!("Error: {:?}", e.to_string())).show_and_wait();
                 return Err(AppSW::MetadataSignatureFail);
@@ -203,17 +203,18 @@ fn metadata_signature_message_from_script_and_common(network: u64, script: &[u8;
 
 fn message_from_script(
     network: u64,
-    commitment_mask: &Zeroizing<RistrettoSecretKey>,
-    receiver_public_spend_key: &Zeroizing<RistrettoPublicKey>,
+    commitment_mask: &RistrettoSecretKey,
+    receiver_public_spend_key: &RistrettoPublicKey,
 ) -> Result<[u8; 32], AppSW> {
-    let hasher = DomainSeparatedHasher::<Blake2b<U64>, KeyManagerTransactionsHashDomain>::new_with_label("script key");
-    let hashed_bytes = hasher.chain(commitment_mask.as_bytes()).finalize();
-    let hashed_commitment_mask = get_key_from_uniform_bytes(hashed_bytes.as_ref())?;
-    let hashed_commitment_mask_public_key =
-        Zeroizing::new(RistrettoPublicKey::from_secret_key(&hashed_commitment_mask));
-    let stealth_key = Zeroizing::new(receiver_public_spend_key.deref() + hashed_commitment_mask_public_key.deref());
-
-    let serialized_script = match hex_to_bytes_serialized(PUSH_PUBKEY_IDENTIFIER, &stealth_key.deref().to_hex()) {
+    let mut raw_key_hashed = Zeroizing::new([0u8; 64]);
+    DomainSeparatedHasher::<Blake2b<U64>, KeyManagerTransactionsHashDomain>::new_with_label("script key")
+        .chain(commitment_mask.as_bytes())
+        .finalize_into(raw_key_hashed.as_mut().into());
+    let hashed_commitment_mask = get_key_from_uniform_bytes(&raw_key_hashed)?;
+    let hashed_commitment_mask_public_key = RistrettoPublicKey::from_secret_key(&hashed_commitment_mask);
+    let stealth_key = receiver_public_spend_key + hashed_commitment_mask_public_key;
+
+    let serialized_script = match hex_to_bytes_serialized(PUSH_PUBKEY_IDENTIFIER, &stealth_key.to_hex()) {
         Ok(script) => script,
         Err(e) => {
             SingleMessage::new(&format!("Script error: {:?}", e.to_string())).show_and_wait();

applications/minotari_ledger_wallet/wallet/src/handlers/get_schnorr_signature.rs

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 
 use alloc::format;
-use core::ops::Deref;
 
 use ledger_device_sdk::{io::Comm, ui::gadgets::SingleMessage};
 use tari_crypto::{
@@ -59,14 +58,13 @@ pub fn handler_get_raw_schnorr_signature(comm: &mut Comm) -> Result<(), AppSW> {
     let mut challenge_bytes = [0u8; 64];
     challenge_bytes.clone_from_slice(&data[40..104]);
 
-    let signature =
-        match RistrettoSchnorr::sign_raw_uniform(&private_key, private_nonce.deref().clone(), &challenge_bytes) {
-            Ok(sig) => sig,
-            Err(e) => {
-                SingleMessage::new(&format!("Signing error: {:?}", e.to_string())).show_and_wait();
-                return Err(AppSW::RawSchnorrSignatureFail);
-            },
-        };
+    let signature = match RistrettoSchnorr::sign_raw_uniform(&private_key, private_nonce.clone(), &challenge_bytes) {
+        Ok(sig) => sig,
+        Err(e) => {
+            SingleMessage::new(&format!("Signing error: {:?}", e.to_string())).show_and_wait();
+            return Err(AppSW::RawSchnorrSignatureFail);
+        },
+    };
 
     comm.append(&[RESPONSE_VERSION]); // version
     comm.append(&signature.get_public_nonce().to_vec());
@@ -101,7 +99,7 @@ pub fn handler_get_script_schnorr_signature(comm: &mut Comm) -> Result<(), AppSW
     let mut nonce_bytes = [0u8; 32];
     nonce_bytes.clone_from_slice(&data[24..56]);
 
-    let random_nonce = get_random_nonce()?.deref().clone();
+    let random_nonce = get_random_nonce()?.clone();
     let signature =
         match CheckSigSchnorrSignature::sign_with_nonce_and_message(&private_key, random_nonce, &nonce_bytes) {
             Ok(sig) => sig,