From ce8ce8f105b3efe525f7e9bbda10ec6961513a6e Mon Sep 17 00:00:00 2001 From: Nikolaus Heger Date: Wed, 19 Aug 2026 13:33:20 +0800 Subject: [PATCH] Address the signing request to the account that must sign it The QR carried the payload and nothing else, so a wallet holding several accounts could not tell which key the request wanted, and one holding none of them could not tell that it held the wrong key - it would sign with whatever it had and produce a signature the chain rejects, or worse, one it accepts from an account the user did not mean to spend from. The payload now travels in the same JSON envelope the mobile app, the cold wallet app and the Keystone firmware read: {v, signer, payload}, refused outright if it is not exactly that. cold-sign-sim reads it too, and refuses a request addressed to an account other than the wallet it was asked to sign with, which is what the devices do. Adds `quantus signing-qr`: the first half of the cold signing flow on its own - print the request for an address and stop, submitting nothing and needing no wallet imported first. It reuses the same context capture, payload builder and animated display as the real flow, so what it prints is what a signer is really asked to read, which is what makes it useful for checking how a device displays a call. --- src/cli/cold_signing.rs | 39 +++++++--- src/cli/mod.rs | 31 ++++++++ src/cli/signing_qr.rs | 131 +++++++++++++++++++++++++++++++++ src/qr/mod.rs | 2 + src/qr/sign_request.rs | 155 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 350 insertions(+), 8 deletions(-) create mode 100644 src/cli/signing_qr.rs create mode 100644 src/qr/sign_request.rs diff --git a/src/cli/cold_signing.rs b/src/cli/cold_signing.rs index 5c55f85..4dd5619 100644 --- a/src/cli/cold_signing.rs +++ b/src/cli/cold_signing.rs @@ -20,7 +20,7 @@ use crate::{ chain::client::{ChainConfig, QuantusClient}, error::{QuantusError, Result}, log_print, log_verbose, - qr::{display_ur_until_enter, render_ur_frames, scan_ur, UrSource}, + qr::{display_ur_until_enter, render_ur_frames, scan_ur, SignRequest, UrSource}, }; use colored::Colorize; use qp_dilithium_crypto::types::{Dilithium87SignatureWithPublic, DilithiumSignatureScheme}; @@ -327,8 +327,12 @@ pub async fn sign_and_submit_cold( } } - // 2. Encode as UR and hand it to the cold wallet. - let parts = quantus_ur::encode_bytes(&raw_payload) + // 2. Address the payload to the account that must sign it, encode as UR, and + // hand it to the cold wallet. Without the envelope a signer holding several + // accounts cannot tell which key this wants, and one holding none of them + // cannot tell that it holds the wrong key. + let request = SignRequest::new(cold_address_ss58, raw_payload.clone()); + let parts = quantus_ur::encode_bytes(&request.encode()) .map_err(|e| QuantusError::Generic(format!("Failed to UR-encode payload: {e:?}")))?; // A response file existing before this request is handed out is necessarily @@ -479,7 +483,8 @@ pub async fn handle_cold_sign_sim( Some(path) => UrSource::File(PathBuf::from(path)), None => UrSource::StdinLines, }; - let payload = scan_ur(&request_source, Duration::from_secs(60)).await?; + let request = SignRequest::decode(&scan_ur(&request_source, Duration::from_secs(60)).await?)?; + let payload = request.payload.clone(); if payload.len() < 2 { return Err(QuantusError::Generic(format!( @@ -488,11 +493,20 @@ pub async fn handle_cold_sign_sim( ))); } log_print!("🧾 Sign request: {} bytes", payload.len()); + log_print!(" Signer: {}", request.signer.bright_cyan()); log_print!(" Pallet index: {}, call index: {}", payload[0], payload[1]); log_verbose!(" Payload hex: 0x{}", hex::encode(&payload)); - // 2. Sign exactly like the cold wallet app / Keystone firmware. + // 2. Sign exactly like the cold wallet app / Keystone firmware, which + // includes refusing a request addressed to an account they do not hold. let keypair = crate::wallet::load_keypair_from_wallet(&wallet, password, password_file)?; + let signing_address = keypair.try_to_account_id_ss58check()?; + if signing_address != request.signer { + return Err(QuantusError::Generic(format!( + "This request is for {}, but wallet '{wallet}' is {signing_address}. Nothing was signed.", + request.signer + ))); + } if keypair.scheme != crate::wallet::DilithiumScheme::MlDsa87 { return Err(QuantusError::Generic( "cold-sign-sim and real devices sign ML-DSA-87 only; use an ML-DSA-87 wallet" @@ -693,9 +707,18 @@ mod tests { let call = transfer_call(); let raw = build_raw_signer_payload(&state, &call, &ctx).unwrap(); - // CLI β†’ UR β†’ cold wallet - let request_parts = quantus_ur::encode_bytes(&raw).unwrap(); - let received = quantus_ur::decode_bytes(&request_parts).unwrap(); + // CLI β†’ UR β†’ cold wallet, through the envelope that names the signer, so + // this asserts the wire format the wallets actually read. + let alice_address = { + use sp_core::crypto::Ss58Codec; + alice_account().to_ss58check() + }; + let request = SignRequest::new(alice_address.clone(), raw.clone()); + let request_parts = quantus_ur::encode_bytes(&request.encode()).unwrap(); + let received_request = + SignRequest::decode(&quantus_ur::decode_bytes(&request_parts).unwrap()).unwrap(); + assert_eq!(received_request.signer, alice_address, "the request must name its signer"); + let received = received_request.payload; assert_eq!(received, raw); // Cold wallet signs and answers over UR diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 71b2df0..030b900 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -20,6 +20,7 @@ pub mod reversible; pub mod runtime; pub mod scheduler; pub mod send; +pub mod signing_qr; pub mod storage; pub mod system; pub mod tech_collective; @@ -69,6 +70,33 @@ pub enum Commands { nonce: Option, }, + /// Print the QR a cold wallet scans to sign a call for an account, and stop + SigningQr { + /// Address (or wallet name) of the account that must sign + #[arg(short, long)] + from: String, + + /// Recipient of the sample transfer (defaults to the signer itself) + #[arg(short, long)] + to: Option, + + /// Amount for the sample transfer (e.g., "10", "10.5", "0.0001") + #[arg(short, long, default_value = "1.5")] + amount: String, + + /// Hex-encoded call to sign instead of the sample transfer + #[arg(long)] + call_data: Option, + + /// Optional tip amount (e.g., "1", "0.5") + #[arg(long)] + tip: Option, + + /// Manual nonce override (defaults to the account's next nonce on chain) + #[arg(long)] + nonce: Option, + }, + /// Batch transfer commands and configuration #[command(subcommand)] Batch(batch::BatchCommands), @@ -369,6 +397,9 @@ pub async fn execute_command( execution_mode, ) .await, + Commands::SigningQr { from, to, amount, call_data, tip, nonce } => + signing_qr::handle_signing_qr_command(from, to, amount, call_data, tip, nonce, node_url) + .await, Commands::Batch(batch_cmd) => batch::handle_batch_command(batch_cmd, node_url, execution_mode).await, Commands::Reversible(reversible_cmd) => diff --git a/src/cli/signing_qr.rs b/src/cli/signing_qr.rs new file mode 100644 index 0000000..a2b49d9 --- /dev/null +++ b/src/cli/signing_qr.rs @@ -0,0 +1,131 @@ +//! `quantus signing-qr` β€” print a signing request and stop there. +//! +//! The signing flow in [`crate::cli::cold_signing`] shows the same QR, then +//! waits to scan the signature back and submit it. This command is the first +//! half on its own: point it at an address, get the QR, scan it with the device +//! under test. Nothing is submitted and no wallet has to be imported first, +//! which is what makes it usable for checking how a signer displays a call. +use crate::{ + chain::{client::QuantusClient, quantus_subxt}, + cli::{ + cold_signing::{build_raw_signer_payload, capture_tx_context}, + common::resolve_address_with_subxt_account_id, + send::parse_amount, + }, + error::{QuantusError, Result}, + log_print, log_success, log_verbose, + qr::{display_ur_until_enter, render_ur_frames, SignRequest}, +}; +use colored::Colorize; +use subxt::client::OfflineClientT; + +/// Builds the request for [`from`] and prints it as a QR. +/// +/// The call is a sample transfer unless `call_data` names one, so any call the +/// signer might have to display can be checked by passing its bytes. +pub async fn handle_signing_qr_command( + from: String, + to: Option, + amount: String, + call_data: Option, + tip: Option, + nonce: Option, + node_url: &str, +) -> Result<()> { + let (signer_address, signer_account) = resolve_address_with_subxt_account_id(&from)?; + + let client = QuantusClient::new(node_url).await?; + let tip = match tip { + Some(ref value) => parse_amount(&client, value).await?, + None => 0, + }; + let account = sp_core::crypto::AccountId32::new(signer_account.0); + let context = capture_tx_context(&client, &account, tip, nonce).await?; + let state = client.client().client_state(); + + let raw_payload = match call_data { + Some(ref hex_call) => { + let mut raw = decode_call_data(hex_call)?; + // The same extensions build_raw_signer_payload appends to a typed + // call, for bytes that are already a call. + raw.extend_from_slice(&build_raw_signer_payload(&state, &EmptyCall, &context)?); + raw + }, + None => { + let destination = match to { + Some(ref address) => resolve_address_with_subxt_account_id(address)?.1, + None => signer_account, + }; + let value = parse_amount(&client, &amount).await?; + let call = quantus_subxt::api::tx().balances().transfer_allow_death( + subxt::ext::subxt_core::utils::MultiAddress::Id(destination), + value, + ); + build_raw_signer_payload(&state, &call, &context)? + }, + }; + + let request = SignRequest::new(signer_address.clone(), raw_payload.clone()); + let parts = quantus_ur::encode_bytes(&request.encode()) + .map_err(|e| QuantusError::Generic(format!("Failed to UR-encode the request: {e:?}")))?; + + log_print!(""); + log_print!("{}", "Signing request".bright_cyan().bold()); + log_print!(" Signer: {}", signer_address.bright_cyan()); + log_print!(" Nonce: {}", context.nonce); + log_print!(" Payload: {} bytes", raw_payload.len()); + log_verbose!(" Payload hex: 0x{}", hex::encode(&raw_payload)); + log_print!(""); + // Reuses the signing flow's own display, so a request too big for one frame + // animates here exactly as it does when the CLI is really asking to be signed. + display_ur_until_enter( + &render_ur_frames(&parts)?, + "πŸ“± Scan this QR with the signer under test, then press Enter here…", + ) + .await?; + log_success!("Nothing was submitted: this prints the request and stops."); + Ok(()) +} + +/// A call with no bytes of its own, so [`build_raw_signer_payload`] can be used +/// for its extensions alone when the call is supplied as raw bytes. +struct EmptyCall; + +impl subxt::tx::Payload for EmptyCall { + fn encode_call_data_to( + &self, + _metadata: &subxt::Metadata, + _out: &mut Vec, + ) -> std::result::Result<(), subxt::ext::subxt_core::Error> { + Ok(()) + } +} + +fn decode_call_data(call_data: &str) -> Result> { + let bytes = hex::decode(call_data.trim().trim_start_matches("0x")) + .map_err(|e| QuantusError::Generic(format!("Call data is not hex: {e}")))?; + if bytes.len() < 2 { + return Err(QuantusError::Generic( + "Call data must be at least a pallet and call index".to_string(), + )); + } + Ok(bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn call_data_is_read_with_or_without_the_hex_prefix() { + assert_eq!(decode_call_data("0x0200").unwrap(), vec![0x02, 0x00]); + assert_eq!(decode_call_data(" 0200 ").unwrap(), vec![0x02, 0x00]); + } + + #[test] + fn call_data_shorter_than_an_index_pair_is_refused() { + for input in ["", "0x", "0x02", "nothex"] { + assert!(decode_call_data(input).is_err(), "call data {input:?} was accepted"); + } + } +} diff --git a/src/qr/mod.rs b/src/qr/mod.rs index 9063fdd..638b874 100644 --- a/src/qr/mod.rs +++ b/src/qr/mod.rs @@ -7,6 +7,8 @@ pub mod display; pub mod scanner; +pub mod sign_request; pub use display::{display_ur_until_enter, render_ur_frames}; pub use scanner::{scan_quantus_address, scan_ur, UrSource}; +pub use sign_request::SignRequest; diff --git a/src/qr/sign_request.rs b/src/qr/sign_request.rs new file mode 100644 index 0000000..8ac9b13 --- /dev/null +++ b/src/qr/sign_request.rs @@ -0,0 +1,155 @@ +//! The envelope a signing payload travels in. +//! +//! A payload on its own says nothing about whose key it belongs to: a signer +//! holding several accounts cannot tell which one the request wants, and one +//! holding none of them cannot tell that it holds the wrong key. The envelope +//! names the account, and every wallet refuses a request for an account it does +//! not hold. +//! +//! The JSON is the wire format the Quantus mobile app, the cold wallet app and +//! the Keystone firmware read β€” `SigningRequest` in quantus_sdk +//! (`lib/src/models/signing_request.dart`). Keep the two in step: the wallets +//! accept these three keys and no others, and refuse any other version. +use crate::error::{QuantusError, Result}; +use serde::{Deserialize, Serialize}; + +/// Envelope version the wallets accept. +pub const SIGN_REQUEST_VERSION: u8 = 1; + +/// Largest payload a wallet will read, matching `maxPayloadBytes` in the SDK. +const MAX_PAYLOAD_BYTES: usize = 8 * 1024; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SignRequest { + /// SS58 address of the account that must sign. + pub signer: String, + /// The SCALE signing payload: call plus signed extensions. + pub payload: Vec, +} + +#[derive(Serialize, Deserialize)] +struct Wire { + v: u8, + signer: String, + payload: String, +} + +impl SignRequest { + pub fn new(signer: impl Into, payload: Vec) -> Self { + Self { signer: signer.into(), payload } + } + + /// The bytes that go into the UR frames. + pub fn encode(&self) -> Vec { + let wire = Wire { + v: SIGN_REQUEST_VERSION, + signer: self.signer.clone(), + payload: format!("0x{}", hex::encode(&self.payload)), + }; + // The struct has no field that can fail to serialise. + serde_json::to_vec(&wire).expect("sign request serialises") + } + + /// Reads an envelope, rejecting anything that is not exactly one. + /// + /// Deliberately as strict as the wallets: a request that cannot be read in + /// full is refused rather than signed on a guess about what it meant. + pub fn decode(bytes: &[u8]) -> Result { + let wire: Wire = serde_json::from_slice(bytes).map_err(|e| { + QuantusError::Generic(format!( + "Not a signing request. A wallet built before the request envelope sends a bare \ + payload, which cannot say which account it is for ({e})" + )) + })?; + + if wire.v != SIGN_REQUEST_VERSION { + return Err(QuantusError::Generic(format!( + "Unsupported signing request version: {} (this build reads {SIGN_REQUEST_VERSION})", + wire.v + ))); + } + + let hex_payload = wire.payload.strip_prefix("0x").ok_or_else(|| { + QuantusError::Generic("Signing request payload is not 0x hex".to_string()) + })?; + let payload = hex::decode(hex_payload).map_err(|e| { + QuantusError::Generic(format!("Signing request payload is not hex: {e}")) + })?; + + if payload.is_empty() { + return Err(QuantusError::Generic("Signing request payload is empty".to_string())); + } + if payload.len() > MAX_PAYLOAD_BYTES { + return Err(QuantusError::Generic(format!( + "Signing request payload too large: {} bytes", + payload.len() + ))); + } + + Ok(Self { signer: wire.signer, payload }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const ADDRESS: &str = "qznQKhufTDfU3szAzfgCny7wMhxUN3qjEqneiRUNgC7MjSDyG"; + + #[test] + fn round_trips_through_the_wire_format() { + let request = SignRequest::new(ADDRESS, vec![0x02, 0x00, 0xff]); + + assert_eq!(SignRequest::decode(&request.encode()).unwrap(), request); + } + + #[test] + fn carries_exactly_the_keys_the_wallets_read() { + let encoded = SignRequest::new(ADDRESS, vec![0xab]).encode(); + let json: serde_json::Value = serde_json::from_slice(&encoded).unwrap(); + + assert_eq!(json["v"], 1); + assert_eq!(json["signer"], ADDRESS); + assert_eq!(json["payload"], "0xab"); + assert_eq!(json.as_object().unwrap().len(), 3, "a wallet refuses any other key set"); + } + + #[test] + fn refuses_a_bare_payload() { + // What this CLI used to send, and what a wallet now refuses because it + // names no account. + let bare = vec![0x02, 0x00, 0x01, 0x02]; + + assert!(SignRequest::decode(&bare).is_err()); + } + + #[test] + fn refuses_a_version_it_does_not_read() { + let wire = serde_json::json!({ "v": 2, "signer": ADDRESS, "payload": "0xab" }); + + let error = SignRequest::decode(wire.to_string().as_bytes()).unwrap_err().to_string(); + assert!(error.contains("version"), "unexpected error: {error}"); + } + + #[test] + fn refuses_a_payload_that_is_not_hex_bytes() { + for payload in ["", "0x", "abcd", "0xnothex"] { + let wire = serde_json::json!({ "v": 1, "signer": ADDRESS, "payload": payload }); + assert!( + SignRequest::decode(wire.to_string().as_bytes()).is_err(), + "payload {payload:?} was accepted" + ); + } + } + + #[test] + fn refuses_a_payload_past_the_size_a_wallet_reads() { + let wire = serde_json::json!({ + "v": 1, + "signer": ADDRESS, + "payload": format!("0x{}", hex::encode(vec![0u8; MAX_PAYLOAD_BYTES + 1])), + }); + + assert!(SignRequest::decode(wire.to_string().as_bytes()).is_err()); + } +}