diff --git a/Cargo.lock b/Cargo.lock index f8ec1205..3492f940 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -196,7 +196,7 @@ checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" [[package]] name = "ledger_device_sdk" -version = "1.36.2" +version = "1.37.0" dependencies = [ "const-zero", "include_gif", diff --git a/ledger_device_sdk/CHANGELOG.md b/ledger_device_sdk/CHANGELOG.md index b6ebe4f1..89d7edd3 100644 --- a/ledger_device_sdk/CHANGELOG.md +++ b/ledger_device_sdk/CHANGELOG.md @@ -5,6 +5,33 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.37.0] - 2026-08-24 + +### Added +- `StatusWords::CmdNotAccepted` (0x6901). An APDU received while a previous + command is still being processed is now answered with this status word instead + of being queued or silently dropped. Note that adding a variant to the public + `StatusWords` enum breaks downstream exhaustive `match` expressions over it. + +### Fixed +- `io_new`: BOLOS internal APDUs (CLA 0xB0) arriving while an NBGL screen is + displayed are handled inline again instead of being answered + `CmdNotAccepted`. That handling was gated behind the `stack_usage` feature, so + default builds rejected OS level requests that `next_command` answers. +- `io_new`: a double APDU is now answered on the polling iteration that detects + it. It used to be answered on the next one, so a screen completing in between + discarded it with no response at all, leaving the host waiting. +- `io_new`: a malformed APDU received while a screen is displayed is answered + `BadLen` instead of being ignored, matching `next_command` and `io_legacy`. +- `io_new`: rejecting an APDU no longer leaves `apdu_type` overwritten with the + rejected APDU's transport, which made the in-flight command reply on the + wrong channel. + +### Deprecated +- `NbglHomeAndSettings::show()` is now marked with the `#[deprecated]` attribute, + matching what its documentation already stated. Use `show_and_return()` + instead, which does not force a home screen refresh for every received APDU. + ## [1.36.2] - 2026-08-18 ### Changed diff --git a/ledger_device_sdk/Cargo.toml b/ledger_device_sdk/Cargo.toml index cb00018d..105ad2ab 100644 --- a/ledger_device_sdk/Cargo.toml +++ b/ledger_device_sdk/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ledger_device_sdk" -version = "1.36.2" +version = "1.37.0" authors = ["Ledger"] edition = "2024" license.workspace = true diff --git a/ledger_device_sdk/src/io_legacy.rs b/ledger_device_sdk/src/io_legacy.rs index 9476c7c9..bfd40b4b 100644 --- a/ledger_device_sdk/src/io_legacy.rs +++ b/ledger_device_sdk/src/io_legacy.rs @@ -44,6 +44,7 @@ pub enum StatusWords { Unknown = 0x6d00, Panic = 0xe000, DeviceLocked = 0x5515, + CmdNotAccepted = 0x6901, } #[derive(Debug)] @@ -194,6 +195,10 @@ pub struct Comm { /// Used when replying to BOLOS APDUs where io_rx(false) would deadlock. #[allow(dead_code)] skip_rx_on_send: bool, + /// True from the moment a command is handed to the application until the + /// application replies to it. This is what makes a second incoming APDU a + /// *double* APDU rather than the normal way of receiving work. + apdu_in_progress: bool, } impl Default for Comm { @@ -231,6 +236,7 @@ impl Comm { rx_length: 0, tx_length: 0, skip_rx_on_send: false, + apdu_in_progress: false, } } @@ -300,6 +306,8 @@ impl Comm { } self.tx_length = 0; self.rx_length = 0; + // Replying completes the current command. + self.apdu_in_progress = false; } /// Wait and return next button press event or APDU command. @@ -355,17 +363,41 @@ impl Comm { } } + /// Fetch and process one event while an NBGL screen is displayed, and report + /// whether it was an APDU command. + /// + /// This is called from `ux_sync_wait` in two different situations: + /// + /// * no command is being processed (idle home screen): an incoming APDU is + /// the normal way of receiving work, so it is decoded and `true` is + /// returned so the caller can leave the screen and handle it. + /// * a command is being processed (review, status, … screen): an incoming + /// APDU is a double APDU and is answered [`StatusWords::CmdNotAccepted`]. pub fn next_event_ahead(&mut self) -> bool where T: TryFrom, Reply: From<>::Error>, { let status = sys_seph::io_rx(&mut self.io_buffer, true); + if status <= 0 { + return false; + } - if status > 0 { - return self.detect_apdu::(status); + // Reject a double APDU on the raw frame, before any decoding: the + // in-flight command owns apdu_buffer / apdu_type / rx / tx, so the + // intruder must never be decoded into them. BOLOS APDUs (CLA 0xB0) and + // frames too short to hold a header fall through to keep their existing + // handling in `check_event`. + if self.apdu_in_progress + && Self::is_apdu_packet(self.io_buffer[0]) + && status >= 5 + && self.io_buffer[1] != 0xB0 + { + self.reject_apdu(StatusWords::CmdNotAccepted); + return false; } - false + + self.detect_apdu::(status) } pub fn check_event(&mut self) -> Option> @@ -413,6 +445,9 @@ impl Comm { let res = T::try_from(*self.get_apdu_metadata()); match res { Ok(ins) => { + // The command is about to be handed to the application: any + // APDU arriving from now until the reply is a double APDU. + self.apdu_in_progress = true; return Some(Event::Command(ins)); } Err(sw) => { @@ -602,6 +637,31 @@ impl Comm { } } + /// True if a received SEPH frame carries an APDU rather than an event. + fn is_apdu_packet(packet_type: u8) -> bool { + matches!( + seph::PacketTypes::from(packet_type), + seph::PacketTypes::PacketTypeRawApdu + | seph::PacketTypes::PacketTypeUsbHidApdu + | seph::PacketTypes::PacketTypeUsbWebusbApdu + | seph::PacketTypes::PacketTypeBleApdu + ) + } + + /// Answer `sw` to an APDU received while another command is in flight. + /// + /// The status word is sent on the intruder's own transport, taken from the + /// received frame, and nothing owned by the in-flight command is touched: + /// `apdu_buffer`, `apdu_type`, `rx`, `rx_length`, `tx`, `tx_length` and + /// `event_pending` are all left as they were. This deliberately bypasses + /// [`Comm::reply`]/`apdu_send`, which would flush any staged response bytes, + /// reset the response state and consume a SEPH event. + fn reject_apdu(&self, sw: StatusWords) { + let sw = sw as u16; + let resp = [(sw >> 8) as u8, sw as u8]; + sys_seph::io_tx(self.io_buffer[0], &resp, resp.len()); + } + /// Wait for the next Command event. Discards received button events. /// /// Like `next_event`, `T` can be any type, an enumeration, or any type diff --git a/ledger_device_sdk/src/io_new.rs b/ledger_device_sdk/src/io_new.rs index 675d5ae8..55ab26d7 100644 --- a/ledger_device_sdk/src/io_new.rs +++ b/ledger_device_sdk/src/io_new.rs @@ -147,6 +147,10 @@ pub struct Comm { pending_header: ApduHeader, pending_offset: usize, pending_length: usize, + /// True from the moment a command is handed to the application until the + /// application replies to it. This is what makes a second incoming APDU a + /// *double* APDU rather than the normal way of receiving work. + apdu_in_progress: bool, } impl Comm { @@ -166,9 +170,23 @@ impl Comm { }, pending_offset: 0, pending_length: 0, + apdu_in_progress: false, } } + /// Answer `sw` to an APDU that cannot be delivered to the application, on + /// the transport it arrived on. + /// + /// This deliberately bypasses [`Comm::begin_response`]: it must stage + /// nothing into the shared buffer and must leave `apdu_type`, + /// `pending_apdu` and `apdu_in_progress` alone, as they belong to the + /// command that is still being processed. + pub(crate) fn reject_apdu>(&self, packet_type: u8, sw: T) { + let sw: u16 = sw.into().0; + let resp = sw.to_be_bytes(); + let _ = sys_seph::io_tx(packet_type, resp.as_ref(), resp.len()); + } + pub(crate) fn nbgl_register_comm(&mut self) { // Register NBGL callbacks if not already set and record current Comm singleton. callbacks::set_comm::(self); @@ -252,6 +270,9 @@ impl Comm { continue; } } + // The command is about to be handed to the application: any + // APDU arriving from now until the reply is a double APDU. + self.apdu_in_progress = true; return Command::new(self, header, offset, length); } // Explicitly convert ApduError -> StatusWords so Into is resolved @@ -411,6 +432,8 @@ impl<'a, const N: usize> CommandResponse<'a, N> { // Clear the pending APDU state after sending a reply, so the next // call to try_next_event will fetch a new event from io_rx. self.comm.pending_apdu = false; + // Replying completes the current command. + self.comm.apdu_in_progress = false; Ok(self.comm) } diff --git a/ledger_device_sdk/src/io_new/callbacks.rs b/ledger_device_sdk/src/io_new/callbacks.rs index b77a5eec..46e6e2fe 100644 --- a/ledger_device_sdk/src/io_new/callbacks.rs +++ b/ledger_device_sdk/src/io_new/callbacks.rs @@ -3,9 +3,8 @@ //! This module holds the erased pointer to the current `Comm` instance and the //! generic callback wrappers that are registered through `nbgl_register_callbacks`. -use crate::io_legacy::{ApduHeader, Reply}; +use crate::io_legacy::{ApduHeader, Reply, StatusWords}; -#[cfg(feature = "stack_usage")] use super::bolos::handle_bolos_apdu; use super::{Comm, DecodedEventType}; @@ -67,39 +66,74 @@ fn panic_reply_impl(reply: Reply) { // Implementation wrappers specialized per const N. +/// Fetch and process one event while an NBGL screen is displayed, and report +/// whether it was an APDU command the caller should leave the screen for. +/// +/// This is called from `ux_sync_wait` both while the application is idle (an +/// incoming APDU is then the normal way of receiving work) and while it is +/// processing a command (an incoming APDU is then a double APDU). pub(super) fn next_event_ahead_impl() -> bool { let comm = unsafe { get_comm::() }; - // If there's already a pending APDU, return true immediately without - // fetching another event. This prevents consuming the same APDU repeatedly - // when ux_sync_wait loops with exit_on_apdu=false. + + // Decoding an APDU overwrites `apdu_type` with the transport it arrived on. + // Anything handled or rejected below is not the command the application is + // working on, so its transport is restored before returning; otherwise the + // in-flight command's response would go out on the intruder's channel. + let in_flight_apdu_type = comm.apdu_type; + + // An APDU detected on an earlier iteration that nobody consumed means the + // displayed screen does not exit on APDU. Answer it, so that polling — and + // therefore the screen itself — keeps running. No command can be in flight + // here, as one would have been rejected on the spot below. if comm.pending_apdu { - return true; + comm.pending_apdu = false; + comm.reject_apdu(in_flight_apdu_type, StatusWords::CmdNotAccepted); + return false; } + match comm.next_event().into_type() { DecodedEventType::Apdu { header, offset, length, } => { - // Handle BOLOS internal APDUs (CLA = 0xB0) inline so they don't - // block the ux_sync_wait loop. Without this, a BOLOS APDU arriving - // during an NBGL screen (e.g. stack consumption measurement) would - // set pending_apdu=true and never be consumed when exit_on_apdu=false, - // causing an infinite loop. - #[cfg(feature = "stack_usage")] + // BOLOS internal APDUs (CLA = 0xB0) are answered inline whatever + // the state, the way `next_command` does, so that OS level requests + // keep working while a screen is displayed. if header.cla == 0xB0 { + let in_progress = comm.apdu_in_progress; handle_bolos_apdu::(comm, header.ins, header.p1, header.p2); + // The BOLOS reply must not be taken for the reply to the + // command the application is still processing. + comm.apdu_in_progress = in_progress; + comm.apdu_type = in_flight_apdu_type; + return false; + } + // An APDU arriving while a command is still being processed is a + // double APDU. Answer it on this very iteration: deferring to the + // next one loses it entirely if the screen completes in between. + if comm.apdu_in_progress { + let intruder_apdu_type = comm.apdu_type; + comm.reject_apdu(intruder_apdu_type, StatusWords::CmdNotAccepted); + comm.apdu_type = in_flight_apdu_type; return false; } comm.pending_apdu = true; comm.pending_header = header; comm.pending_offset = offset; comm.pending_length = length; - return true; + true + } + // Answer malformed APDUs instead of leaving the host without a status + // word, as `next_command` does outside of screens. + DecodedEventType::ApduError(e) => { + let intruder_apdu_type = comm.apdu_type; + comm.reject_apdu(intruder_apdu_type, StatusWords::from(e)); + comm.apdu_type = in_flight_apdu_type; + false } - _ => {} + _ => false, } - false } pub(super) fn fetch_apdu_header_impl() -> Option { diff --git a/ledger_device_sdk/src/nbgl/nbgl_home_and_settings.rs b/ledger_device_sdk/src/nbgl/nbgl_home_and_settings.rs index b72cc4d0..e181605a 100644 --- a/ledger_device_sdk/src/nbgl/nbgl_home_and_settings.rs +++ b/ledger_device_sdk/src/nbgl/nbgl_home_and_settings.rs @@ -293,11 +293,13 @@ impl NbglHomeAndSettings { /// Show the home screen and settings page. /// This function will block until an APDU is received or the user quits the app. - /// DEPRECATED as it constraints to refresh screen for every received APDU. - /// Use `show_and_return` instead. /// # Arguments /// * `_comm` - Mutable reference to Comm. #[cfg(feature = "io_new")] + #[deprecated( + since = "1.37.0", + note = "blocking on an APDU forces the home screen to be refreshed for every received APDU; use `show_and_return` instead" + )] pub fn show, const N: usize>( &mut self, _comm: &mut crate::io::Comm, @@ -310,9 +312,11 @@ impl NbglHomeAndSettings { /// Show the home screen and settings page. /// This function will block until an APDU is received or the user quits the app. - /// DEPRECATED as it constraints to refresh screen for every received APDU. - /// Use `show_and_return` instead. #[cfg(not(feature = "io_new"))] + #[deprecated( + since = "1.37.0", + note = "blocking on an APDU forces the home screen to be refreshed for every received APDU; use `show_and_return` instead" + )] pub fn show>(&mut self) -> Event where Reply: From<>::Error>,