Skip to content

Commit

Permalink
add hw wallet account check prior to executing operations
Browse files Browse the repository at this point in the history
  • Loading branch information
ryankurte authored and Brian Corbin committed Sep 26, 2023
1 parent d08e52d commit cb79479
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 6 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions full-service/src/json_rpc/v2/api/wallet.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
db::{
account::AccountID,
account::{AccountID, AccountModel},
transaction_log::TransactionId,
txo::{TxoID, TxoStatus},
},
Expand Down Expand Up @@ -1331,6 +1331,11 @@ where
let synced_txos = match synced_txos {
Some(synced_txos) => synced_txos,
None => {
let account = service
.get_account(&AccountID(account_id.clone()))
.map_err(format_error)?;
let view_account_keys = account.view_account_key().map_err(format_error)?;

let unverified_txos = service
.list_txos(
Some(account_id.clone()),
Expand Down Expand Up @@ -1360,7 +1365,9 @@ where
})
.collect::<Result<Vec<_>, JsonRPCError>>()?;

sync_txos(unsynced_txos).await.map_err(format_error)?
sync_txos(unsynced_txos, &view_account_keys)
.await
.map_err(format_error)?
}
};

Expand Down
21 changes: 21 additions & 0 deletions full-service/src/service/hardware_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::convert::{TryFrom, TryInto};

use ledger_mob::{DeviceHandle, Filters, LedgerHandle, LedgerProvider, Transport};

use mc_account_keys::ViewAccountKey;
use mc_common::logger::global_log;
use mc_core::account::{ViewAccount, ViewSubaddress};
use mc_crypto_keys::RistrettoPublic;
Expand All @@ -25,6 +26,7 @@ pub enum HardwareWalletServiceError {
KeyImageNotFoundForSignedInput,
RingCT(mc_transaction_core::ring_ct::Error),
CryptoKeys(mc_crypto_keys::KeyError),
CredentialMismatch,
}

impl From<mc_transaction_core::ring_ct::Error> for HardwareWalletServiceError {
Expand Down Expand Up @@ -70,9 +72,18 @@ async fn get_device_handle() -> Result<DeviceHandle<LedgerHandle>, HardwareWalle

pub async fn sync_txos(
unsynced_txos: Vec<(TxOut, u64)>,
view_account: &ViewAccountKey,
) -> Result<Vec<TxoSynced>, HardwareWalletServiceError> {
let mut device_handle = get_device_handle().await?;

// Check device and requested accounts match
let device_keys = device_handle.account_keys(0).await?;
if device_keys.view_private_key() != view_account.view_private_key()
|| device_keys.spend_public_key() != view_account.spend_public_key()
{
return Err(HardwareWalletServiceError::CredentialMismatch);
}

let mut synced_txos = vec![];
for unsynced_txo in unsynced_txos {
let tx_public_key = (&unsynced_txo.0.public_key).try_into()?;
Expand Down Expand Up @@ -103,9 +114,19 @@ pub async fn get_view_only_subaddress_keys(

pub async fn sign_tx_proposal(
unsigned_tx_proposal: UnsignedTxProposal,
view_account: &ViewAccountKey,
) -> Result<TxProposal, HardwareWalletServiceError> {
let mut device_handle = get_device_handle().await?;

// Check device and requested accounts match
let device_keys = device_handle.account_keys(0).await?;
if device_keys.view_private_key() != view_account.view_private_key()
|| device_keys.spend_public_key() != view_account.spend_public_key()
{
return Err(HardwareWalletServiceError::CredentialMismatch);
}

// Sign transaction proposal
global_log::debug!("Signing tx proposal with hardware device");
let (tx, txos_synced) = device_handle
.transaction(0, 60, unsigned_tx_proposal.unsigned_tx)
Expand Down
2 changes: 1 addition & 1 deletion full-service/src/service/models/tx_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl UnsignedTxProposal {
match account.view_only {
true => {
global_log::debug!("signing tx proposal with hardware wallet");
Ok(hardware_wallet::sign_tx_proposal(self).await?)
Ok(hardware_wallet::sign_tx_proposal(self, &account.view_account_key()?).await?)
}
false => {
global_log::debug!("signing tx proposal with local signer");
Expand Down
2 changes: 1 addition & 1 deletion ledger-mob
Submodule ledger-mob updated 55 files
+15 −11 .github/workflows/rust.yml
+5 −0 .gitmodules
+51 −52 Cargo.lock
+10 −0 Cargo.toml
+13 −5 Makefile
+4 −2 README.md
+1 −1 apdu/Cargo.toml
+13 −2 apdu/src/app_info.rs
+1 −1 apdu/src/digest.rs
+13 −3 apdu/src/ident.rs
+1 −0 apdu/src/lib.rs
+1 −1 apdu/src/state.rs
+1 −1 apdu/src/tx/mod.rs
+119 −4 apdu/src/tx/summary.rs
+1 −1 core/Cargo.toml
+13 −1 core/src/engine/error.rs
+100 −6 core/src/engine/function.rs
+13 −12 core/src/engine/ident.rs
+160 −65 core/src/engine/mod.rs
+3 −3 core/src/engine/output.rs
+79 −55 core/src/engine/ring.rs
+57 −22 core/src/engine/summary.rs
+0 −2 core/src/helpers/mod.rs
+31 −31 core/src/lib.rs
+4 −2 core/tests/helpers.rs
+5 −0 docs/apdu.md
+1 −0 fw/.cargo/config.toml
+77 −66 fw/Cargo.lock
+17 −8 fw/Cargo.toml
+6 −4 fw/build.rs
+0 −4 fw/docs/apdu.md
+0 −1 fw/rust-toolchain
+3 −0 fw/rust-toolchain
+2 −1 fw/src/consts.rs
+36 −17 fw/src/main.rs
+42 −14 fw/src/platform.rs
+3 −3 fw/src/ui/address.rs
+5 −33 fw/src/ui/app_info.rs
+0 −88 fw/src/ui/approver.rs
+26 −7 fw/src/ui/menu.rs
+8 −3 fw/src/ui/mod.rs
+2 −10 fw/src/ui/progress.rs
+1 −1 fw/src/ui/settings.rs
+112 −0 fw/src/ui/sync_approver.rs
+1 −1 fw/src/ui/tx_summary_approver.rs
+1 −1 lib/Cargo.toml
+1 −1 lib/src/handle.rs
+44 −28 lib/tests/helpers.rs
+2 −3 lib/tests/mlsag.rs
+2 −2 lib/tests/subaddress_keys.rs
+3 −3 lib/tests/wallet_keys.rs
+1 −1 rust-toolchain
+1 −1 tests/Cargo.toml
+1 −1 tests/src/lib.rs
+1 −0 vendor/mc-rand
+1 −1 vendor/mob

0 comments on commit cb79479

Please sign in to comment.