Skip to content

Commit

Permalink
Various safety improvements
Browse files Browse the repository at this point in the history
- 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'.
  • Loading branch information
hansieodendaal committed Aug 21, 2024
1 parent 6e387a8 commit 9125b93
Show file tree
Hide file tree
Showing 13 changed files with 150 additions and 161 deletions.
2 changes: 1 addition & 1 deletion applications/minotari_ledger_wallet/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions applications/minotari_ledger_wallet/common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions applications/minotari_ledger_wallet/comms/src/accessor_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
)))
},
Expand All @@ -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
)))
},
Expand Down
4 changes: 2 additions & 2 deletions applications/minotari_ledger_wallet/comms/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion applications/minotari_ledger_wallet/wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ default = []
pending_review_screen = []

[package.metadata.ledger]
curve = ["ed25519"]
curve = ["ed25519", "secp256k1"]
flags = "0"
path = ["44'/535348'"]
name = "MinoTari Wallet"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright 2024 The Tari Project
// SPDX-License-Identifier: BSD-3-Clause

use core::ops::Deref;

use ledger_device_sdk::{io::Comm, ui::gadgets::SingleMessage};
use tari_crypto::{ristretto::RistrettoPublicKey, tari_utilities::ByteArray};

Expand Down Expand Up @@ -36,7 +34,7 @@ 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) => k * public_key,
Err(e) => return Err(e),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 9125b93

Please sign in to comment.