diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs index 18574a3dd8..f46e37d766 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs @@ -17,6 +17,7 @@ use util::bip32::HARDENED; use crate::bip32; use crate::hal::{Memory, Ui}; +use crate::workflow::confirm; use crate::xpubcache::Bip32XpubCache; use bitcoin::taproot::{LeafVersion, TapLeafHash, TapTweakHash}; @@ -395,15 +396,17 @@ impl ParsedPolicy<'_> { }) .await?; - hal.ui() - .confirm(&ConfirmParams { + confirm::confirm_value( + hal, + &ConfirmParams { title: "Name", body: name, scrollable: true, accept_is_nextarrow: true, ..Default::default() - }) - .await?; + }, + ) + .await?; if matches!(mode, Mode::Basic) { if let Err(crate::hal::ui::UserAbort) = hal @@ -419,15 +422,17 @@ impl ParsedPolicy<'_> { } } - hal.ui() - .confirm(&ConfirmParams { + confirm::confirm_value( + hal, + &ConfirmParams { title: "Policy", body: &policy.policy, scrollable: true, accept_is_nextarrow: true, ..Default::default() - }) - .await?; + }, + ) + .await?; let output_xpub_type = match params.coin { BtcCoin::Btc | BtcCoin::Ltc => bip32::XPubType::Xpub, diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs index 3245b47dad..03bfa6630b 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs @@ -140,6 +140,7 @@ mod tests { use crate::hal::testing::TestingHal; use crate::hal::testing::ui::Screen; use crate::keystore::testing::mock_unlocked; + use crate::workflow::confirm::{MAX_CONFIRM_BODY_SIZE, TRUNCATION_WARNING_BODY}; use alloc::boxed::Box; use util::bip32::HARDENED; @@ -189,6 +190,51 @@ mod tests { ); } + #[async_test::test] + pub async fn test_p2wpkh_long_message_warning() { + let msg = "m".repeat(MAX_CONFIRM_BODY_SIZE + 1); + let request = pb::BtcSignMessageRequest { + coin: BtcCoin::Btc as _, + script_config: Some(pb::BtcScriptConfigWithKeypath { + script_config: Some(pb::BtcScriptConfig { + config: Some(Config::SimpleType(SimpleType::P2wpkh as _)), + }), + keypath: vec![84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0], + }), + msg: msg.as_bytes().to_vec(), + host_nonce_commitment: None, + }; + + mock_unlocked(); + let mut mock_hal = TestingHal::new(); + assert!(process(&mut mock_hal, &request).await.is_ok()); + assert_eq!( + mock_hal.ui.screens, + vec![ + Screen::Confirm { + title: "Sign message".into(), + body: "Coin: Bitcoin".into(), + longtouch: false, + }, + Screen::Confirm { + title: "Address".into(), + body: "bc1q k5f9 em9q c8yf pks8 ngfg 3h8h 02n2 e3ye qdyh pt".into(), + longtouch: false, + }, + Screen::Confirm { + title: "Warning".into(), + body: TRUNCATION_WARNING_BODY.into(), + longtouch: false, + }, + Screen::Confirm { + title: "Sign message".into(), + body: msg, + longtouch: true, + }, + ] + ); + } + #[async_test::test] pub async fn test_p2wpkh_testnet() { let request = pb::BtcSignMessageRequest { diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum.rs index 98dfdbfe89..cd3b891a39 100644 --- a/src/rust/bitbox02-rust/src/hww/api/ethereum.rs +++ b/src/rust/bitbox02-rust/src/hww/api/ethereum.rs @@ -23,9 +23,7 @@ use pb::eth_response::Response; use core::convert::TryInto; -// Keep this in sync with src/ui/components/label.h:MAX_LABEL_SIZE. `MAX_CONFIRM_BODY_SIZE` is the -// effective confirmation body limit and intentionally matches that UI label size limit. -const MAX_CONFIRM_BODY_SIZE: usize = 640; +use crate::workflow::confirm::MAX_CONFIRM_BODY_SIZE; /// Returns how many bytes of hex-encoded data to include in a preview body. /// diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs index c0210c7352..9721e50975 100644 --- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs +++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs @@ -2,7 +2,6 @@ use super::super::payment_request; use super::Error; -use super::MAX_CONFIRM_BODY_SIZE; use super::amount::{Amount, calculate_percentage}; use super::params::Params; use super::pb; @@ -13,6 +12,7 @@ use crate::hal::ui::ConfirmParams; use crate::keystore; use crate::hal::Ui; +use crate::workflow::confirm; use crate::workflow::transaction; use alloc::string::String; @@ -452,26 +452,18 @@ async fn verify_standard_transaction( } else { (request.data().len(), hex::encode(request.data())) }; - if body.len() > MAX_CONFIRM_BODY_SIZE { - hal.ui() - .confirm(&ConfirmParams { - title: "Warning", - body: "The next value is\ntoo large to display\nin full", - accept_is_nextarrow: true, - ..Default::default() - }) - .await?; - } - hal.ui() - .confirm(&ConfirmParams { + confirm::confirm_value( + hal, + &ConfirmParams { title: "Transaction\ndata", body: &body, scrollable: true, display_size, accept_is_nextarrow: true, ..Default::default() - }) - .await?; + }, + ) + .await?; } let address = super::address::from_pubkey_hash(&recipient, request.case()?); @@ -659,6 +651,7 @@ mod tests { use crate::hal::testing::TestingHal; use crate::hal::testing::ui::Screen; use crate::keystore::testing::mock_unlocked; + use crate::workflow::confirm::MAX_CONFIRM_BODY_SIZE; use alloc::boxed::Box; use util::bip32::HARDENED; diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs index 8dbe20c4a0..fc20f865a6 100644 --- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs +++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs @@ -8,7 +8,6 @@ //! using SignTypedDataVersion.V4. use super::Error; -use super::MAX_CONFIRM_BODY_SIZE; use super::pb; use super::sighash::DataProducer; use super::truncating_hex_preview_byte_cap; @@ -16,6 +15,7 @@ use super::truncating_hex_preview_byte_cap; use crate::hal::Ui; use crate::hal::ui::ConfirmParams; use crate::keystore; +use crate::workflow::confirm; use pb::eth_request::Request; use pb::eth_response::Response; @@ -427,18 +427,9 @@ async fn encode_member( let lines: Vec<&str> = value_formatted.split('\n').collect(); for (i, &line) in lines.iter().enumerate() { let body = format_display_line_body(&display_path, i, lines.len(), line); - if body.len() > MAX_CONFIRM_BODY_SIZE { - hal.ui() - .confirm(&ConfirmParams { - title: "Warning", - body: "The next value is\ntoo large to display\nin full", - accept_is_nextarrow: true, - ..Default::default() - }) - .await?; - } - hal.ui() - .confirm(&ConfirmParams { + confirm::confirm_value( + hal, + &ConfirmParams { title: &format!( "{}{}", confirm_title(context.root_object), @@ -449,8 +440,9 @@ async fn encode_member( display_size, accept_is_nextarrow: true, ..Default::default() - }) - .await?; + }, + ) + .await?; } } Ok(()) @@ -741,6 +733,7 @@ mod tests { use crate::hal::testing::TestingHal; use crate::hal::testing::ui::Screen; use crate::keystore::testing::mock_unlocked; + use crate::workflow::confirm::MAX_CONFIRM_BODY_SIZE; use util::bip32::HARDENED; use alloc::boxed::Box; diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs index 59f55cdaca..0c53f96518 100644 --- a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs +++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs @@ -97,6 +97,7 @@ mod tests { use crate::hal::testing::TestingHal; use crate::hal::testing::ui::Screen; use crate::keystore::testing::mock_unlocked; + use crate::workflow::confirm::{MAX_CONFIRM_BODY_SIZE, TRUNCATION_WARNING_BODY}; use alloc::boxed::Box; use hex_lit::hex; use util::bip32::HARDENED; @@ -143,6 +144,48 @@ mod tests { ); } + #[async_test::test] + pub async fn test_process_long_message_warning() { + let msg = "m".repeat(MAX_CONFIRM_BODY_SIZE + 1); + + mock_unlocked(); + let mut mock_hal = TestingHal::new(); + assert!( + process( + &mut mock_hal, + &pb::EthSignMessageRequest { + coin: pb::EthCoin::Eth as _, + keypath: KEYPATH.to_vec(), + msg: msg.as_bytes().to_vec(), + host_nonce_commitment: None, + chain_id: 0, + }, + ) + .await + .is_ok() + ); + assert_eq!( + mock_hal.ui.screens, + vec![ + Screen::Confirm { + title: "Ethereum".into(), + body: EXPECTED_ADDRESS.into(), + longtouch: false, + }, + Screen::Confirm { + title: "Warning".into(), + body: TRUNCATION_WARNING_BODY.into(), + longtouch: false, + }, + Screen::Confirm { + title: "Sign message".into(), + body: msg, + longtouch: true, + }, + ] + ); + } + #[async_test::test] pub async fn test_process_warn_unusual_keypath() { const SIGNATURE: [u8; 64] = [b'1'; 64]; diff --git a/src/rust/bitbox02-rust/src/workflow.rs b/src/rust/bitbox02-rust/src/workflow.rs index 2237bea652..54c58ebf48 100644 --- a/src/rust/bitbox02-rust/src/workflow.rs +++ b/src/rust/bitbox02-rust/src/workflow.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +pub mod confirm; #[cfg_attr( all(feature = "c-unit-testing", not(feature = "testing")), path = "workflow/mnemonic_c_unit_tests.rs" diff --git a/src/rust/bitbox02-rust/src/workflow/confirm.rs b/src/rust/bitbox02-rust/src/workflow/confirm.rs new file mode 100644 index 0000000000..04d9a47496 --- /dev/null +++ b/src/rust/bitbox02-rust/src/workflow/confirm.rs @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: Apache-2.0 + +use crate::hal::Ui; +use crate::hal::ui::{ConfirmParams, UserAbort}; + +// Keep this in sync with src/ui/components/label.h:MAX_LABEL_SIZE. `MAX_CONFIRM_BODY_SIZE` is the +// effective confirmation body limit and intentionally matches that UI label size limit. +pub(crate) const MAX_CONFIRM_BODY_SIZE: usize = 640; + +pub(crate) const TRUNCATION_WARNING_BODY: &str = "The next value is\ntoo large to display\nin full"; + +/// Confirm a potentially long value. +/// +/// If the value exceeds the UI label limit, the UI will truncate it with `...`, so warn the user +/// before showing it. +pub(crate) async fn confirm_value( + hal: &mut impl crate::hal::Hal, + params: &ConfirmParams<'_>, +) -> Result<(), UserAbort> { + if params.body.len() > MAX_CONFIRM_BODY_SIZE { + hal.ui() + .confirm(&ConfirmParams { + title: "Warning", + body: TRUNCATION_WARNING_BODY, + accept_is_nextarrow: true, + ..Default::default() + }) + .await?; + } + hal.ui().confirm(params).await +} diff --git a/src/rust/bitbox02-rust/src/workflow/verify_message.rs b/src/rust/bitbox02-rust/src/workflow/verify_message.rs index dc53ce15a9..1288f70dcb 100644 --- a/src/rust/bitbox02-rust/src/workflow/verify_message.rs +++ b/src/rust/bitbox02-rust/src/workflow/verify_message.rs @@ -3,8 +3,6 @@ use crate::hal::ui::ConfirmParams; use alloc::vec::Vec; -use crate::hal::Ui; - use util::ascii; pub enum Error { @@ -62,20 +60,21 @@ pub async fn verify( longtouch: is_last && is_final, ..Default::default() }; - hal.ui().confirm(¶ms).await?; + crate::workflow::confirm::confirm_value(hal, ¶ms).await?; } Ok(()) } else { + let body = hex::encode(msg); let params = ConfirmParams { title: &format!("{}\ndata (hex)", title_long), - body: &hex::encode(msg), + body: &body, scrollable: true, display_size: msg.len(), accept_is_nextarrow: true, // longtouch takes priority over this if enabled longtouch: is_final, ..Default::default() }; - hal.ui().confirm(¶ms).await?; + crate::workflow::confirm::confirm_value(hal, ¶ms).await?; Ok(()) } } @@ -86,6 +85,7 @@ mod tests { use crate::hal::testing::TestingHal; use crate::hal::testing::ui::Screen; + use crate::workflow::confirm::{MAX_CONFIRM_BODY_SIZE, TRUNCATION_WARNING_BODY}; #[async_test::test] async fn test_verify_multiline_text() { @@ -186,4 +186,113 @@ mod tests { ] ); } + + #[async_test::test] + async fn test_verify_long_ascii_boundary() { + let msg = "a".repeat(MAX_CONFIRM_BODY_SIZE); + let mut hal = TestingHal::new(); + assert!( + verify(&mut hal, "Sign message", "Sign", msg.as_bytes(), true) + .await + .is_ok() + ); + assert_eq!( + hal.ui.screens, + vec![Screen::Confirm { + title: "Sign message".into(), + body: msg, + longtouch: true, + }] + ); + + let msg = "a".repeat(MAX_CONFIRM_BODY_SIZE + 1); + let mut hal = TestingHal::new(); + assert!( + verify(&mut hal, "Sign message", "Sign", msg.as_bytes(), true) + .await + .is_ok() + ); + assert_eq!( + hal.ui.screens, + vec![ + Screen::Confirm { + title: "Warning".into(), + body: TRUNCATION_WARNING_BODY.into(), + longtouch: false, + }, + Screen::Confirm { + title: "Sign message".into(), + body: msg, + longtouch: true, + }, + ] + ); + } + + #[async_test::test] + async fn test_verify_multiline_warns_only_for_overlong_lines() { + let overlong_line = "b".repeat(MAX_CONFIRM_BODY_SIZE + 1); + let msg = format!("ok\n{overlong_line}"); + let mut hal = TestingHal::new(); + assert!( + verify(&mut hal, "Sign message", "Sign", msg.as_bytes(), true) + .await + .is_ok() + ); + assert_eq!( + hal.ui.screens, + vec![ + Screen::Confirm { + title: "Sign 1/2".into(), + body: "ok".into(), + longtouch: false, + }, + Screen::Confirm { + title: "Warning".into(), + body: TRUNCATION_WARNING_BODY.into(), + longtouch: false, + }, + Screen::Confirm { + title: "Sign 2/2".into(), + body: overlong_line, + longtouch: true, + }, + ] + ); + } + + #[async_test::test] + async fn test_verify_binary_hex_boundary() { + let mut hal = TestingHal::new(); + assert!( + verify(&mut hal, "OP_RETURN", "OP_RETURN", &[0xff; 320], false) + .await + .is_ok() + ); + assert_eq!(hal.ui.screens.len(), 1); + assert_eq!(hal.ui.confirm_display_sizes, vec![320]); + + let mut hal = TestingHal::new(); + assert!( + verify(&mut hal, "OP_RETURN", "OP_RETURN", &[0xff; 321], false) + .await + .is_ok() + ); + assert_eq!( + hal.ui.screens[0], + Screen::Confirm { + title: "Warning".into(), + body: TRUNCATION_WARNING_BODY.into(), + longtouch: false, + } + ); + assert_eq!(hal.ui.confirm_display_sizes, vec![0, 321]); + match &hal.ui.screens[1] { + Screen::Confirm { title, body, .. } => { + assert_eq!(title, "OP_RETURN\ndata (hex)"); + assert_eq!(body.len(), MAX_CONFIRM_BODY_SIZE + 2); + } + _ => panic!("unexpected screen"), + } + } } diff --git a/src/ui/components/label.h b/src/ui/components/label.h index f115e0ff22..08c23a626e 100644 --- a/src/ui/components/label.h +++ b/src/ui/components/label.h @@ -9,7 +9,7 @@ // Max size of text shown (excl. null terminator). The current size of 640 is chosen to be able to // show up to 320 bytes of Ethereum tx data in hex format. -// Keep this in sync with src/rust/bitbox02-rust/src/hww/api/ethereum.rs:MAX_CONFIRM_BODY_SIZE. +// Keep this in sync with src/rust/bitbox02-rust/src/workflow/confirm.rs:MAX_CONFIRM_BODY_SIZE. #define MAX_LABEL_SIZE 640 /**