refactor(rust/trezor-thp): keep device_properties in Mux
What changed, and why it matters
This commit is a straightforward internal code refactor in Trezor's Rust firmware. It moves the storage of 'device_properties' from the credential verifier object into the communication multiplexer (Mux). The actual data being sent during device pairing remains the same; only which internal component holds the data changes. There is no indication this fixes a security bug or introduces a vulnerability.
No security action required. Treat as normal refactoring during code review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change relocates device_properties from the CredentialVerifier trait implementation to the Mux struct in rust/trezor-thp. Mux::new() now accepts device_properties: &[u8] and stores it in a heapless::Vec. ChannelOpen::new() receives the properties from Mux instead of calling cred_verif.device_properties(). The reset() method was updated to preserve device_properties while clearing other state, and the Default implementation was removed because new() now requires an argument. Tests were updated accordingly. No functional protocol change is visible.
Changed components
rust/trezor-thp/src/channel/device.rsrust/trezor-thp/src/channel/test.rsrust/trezor-thp/src/credential.rsInspect captured patch +31 / −30
diff --git a/rust/trezor-thp/src/channel/device.rs b/rust/trezor-thp/src/channel/device.rs
index f9669c57..ed5350e2 100644
--- a/rust/trezor-thp/src/channel/device.rs
+++ b/rust/trezor-thp/src/channel/device.rs
@@ -4,8 +4,9 @@ use crate::{
Backend, ChannelIO, Device, Error,
alternating_bit::SyncBits,
channel::{
- HANDSHAKE_BUFFER_DTH_LEN, HANDSHAKE_BUFFER_HTD_LEN, Nonce, PRIVKEY_LEN, PacketInResult,
- PairingState, ReceiveState, SendState, noise::NoiseHandshake,
+ HANDSHAKE_BUFFER_DTH_LEN, HANDSHAKE_BUFFER_HTD_LEN, MAX_DEVICE_PROPERTIES_LEN, Nonce,
+ PRIVKEY_LEN, PacketInResult, PairingState, ReceiveState, SendState,
+ noise::NoiseHandshake,
},
control_byte::ControlByte,
credential::CredentialVerifier,
@@ -39,6 +40,7 @@ pub type Channel<B> = super::Channel<Device, B>;
pub struct Mux<B> {
outgoing: heapless::Deque<MuxOutgoing, BROADCAST_OUTGOING_QUEUE_LEN>,
new_channel: Option<Nonce>,
+ device_properties: heapless::Vec<u8, MAX_DEVICE_PROPERTIES_LEN>,
_phantom: PhantomData<B>,
}
@@ -62,17 +64,22 @@ impl<B> Mux<B>
where
B: Backend,
{
- pub const fn new() -> Self {
- Self {
+ pub fn new(device_properties: &[u8]) -> Result<Self, Error> {
+ let device_properties = heapless::Vec::from_slice(device_properties)
+ .map_err(|_| Error::insufficient_buffer())?;
+ Ok(Self {
outgoing: heapless::Deque::new(),
new_channel: None,
+ device_properties,
_phantom: PhantomData,
- }
+ })
}
/// Reset everything to initial state - discard outgoing messages and channel allocation.
+ /// Keep device_properties.
pub fn reset(&mut self) {
- *self = Self::new()
+ self.outgoing.clear();
+ self.new_channel = None;
}
/// Create new [`ChannelOpen`] when channel allocation request is pending.
@@ -90,7 +97,7 @@ where
let Some(nonce) = self.new_channel.take() else {
return Err(Error::not_ready());
};
- ChannelOpen::<C, B>::new(channel_id, nonce, cred_verif)
+ ChannelOpen::<C, B>::new(channel_id, nonce, &self.device_properties, cred_verif)
}
/// Returns `true` if there is channel allocation request pending.
@@ -178,15 +185,6 @@ where
}
}
-impl<B> Default for Mux<B>
-where
- B: Backend,
-{
- fn default() -> Self {
- Self::new()
- }
-}
-
impl<B> ChannelIO for Mux<B>
where
B: Backend,
@@ -298,7 +296,12 @@ pub struct ChannelOpen<C: CredentialVerifier, B: Backend> {
}
impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
- fn new(channel_id: u16, nonce: Nonce, cred_verif: C) -> Result<Self, Error> {
+ fn new(
+ channel_id: u16,
+ nonce: Nonce,
+ device_properties: &[u8],
+ cred_verif: C,
+ ) -> Result<Self, Error> {
let mut send_buffer = heapless::Vec::new();
send_buffer
.extend_from_slice(nonce.as_slice())
@@ -307,7 +310,7 @@ impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
.extend_from_slice(&channel_id.to_be_bytes())
.map_err(|_| Error::insufficient_buffer())?;
send_buffer
- .extend_from_slice(cred_verif.device_properties())
+ .extend_from_slice(device_properties)
.map_err(|_| Error::insufficient_buffer())?;
let mut receive_buffer = heapless::Vec::new();
prepare_zeroed(&mut receive_buffer);
@@ -318,7 +321,7 @@ impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
Ok(Self {
channel,
state: HandshakeState::SendingChannelResponse,
- noise: NoiseHandshake::prepare_responder(cred_verif.device_properties()),
+ noise: NoiseHandshake::prepare_responder(device_properties),
send_buffer,
receive_buffer,
cred_verif,
diff --git a/rust/trezor-thp/src/channel/test.rs b/rust/trezor-thp/src/channel/test.rs
index 20a371d4..d01b6d9f 100644
--- a/rust/trezor-thp/src/channel/test.rs
+++ b/rust/trezor-thp/src/channel/test.rs
@@ -42,6 +42,11 @@ const DEFAULT_PACKET_LEN: usize = 64;
static SETUP: Once = Once::new();
const DEVICE_KEY: &[u8; PRIVKEY_LEN] = &[0u8; PRIVKEY_LEN];
+// internal_model: Some("T2W1"), model_variant: Some(0), protocol_version_major: Some(2),
+// protocol_version_minor: Some(0), pairing_methods: [CodeEntry, SkipPairing]
+const DEVICE_PROPERTIES: &[u8] =
+ b"\x0a\x04\x54\x32\x57\x31\x10\x00\x18\x02\x20\x00\x28\x02\x28\x01";
+
fn setup() {
SETUP.call_once(|| {
env_logger::init_from_env(env_logger::Env::default().filter_or("RUST_LOG", "info"));
@@ -56,12 +61,6 @@ impl CredentialVerifier for TestCredentialVerifier {
fn verify(&self, _remote_static_pubkey: &[u8], _credential: &[u8]) -> PairingState {
PairingState::Unpaired
}
-
- fn device_properties(&self) -> &[u8] {
- // internal_model: Some("T2W1"), model_variant: Some(0), protocol_version_major: Some(2),
- // protocol_version_minor: Some(0), pairing_methods: [CodeEntry, SkipPairing]
- b"\x0a\x04\x54\x32\x57\x31\x10\x00\x18\x02\x20\x00\x28\x02\x28\x01"
- }
}
pub struct WithKey<C: CredentialVerifier, B: Backend> {
@@ -536,7 +535,9 @@ fn create_mux() -> (
) {
let mut hm = host::Mux::<RustCrypto>::new().into_buffered();
hm.set_packet_len(DEFAULT_PACKET_LEN);
- let mut dm = device::Mux::<RustCrypto>::new().into_buffered();
+ let mut dm = device::Mux::<RustCrypto>::new(DEVICE_PROPERTIES)
+ .unwrap()
+ .into_buffered();
dm.set_packet_len(DEFAULT_PACKET_LEN);
let cids = device::ChannelIdAllocator::new_random::<RustCrypto>();
(hm, dm, cids)
@@ -623,7 +624,7 @@ fn test_packet_length(packet_len: usize, ack_piggybacking: bool) -> Result<()> {
fn test_one_device_multiple_hosts() -> Result<()> {
const NHOSTS: usize = 4;
setup();
- let mut dm = device::Mux::<RustCrypto>::new().into_buffered();
+ let mut dm = device::Mux::<RustCrypto>::new(DEVICE_PROPERTIES)?.into_buffered();
dm.set_packet_len(DEFAULT_PACKET_LEN);
let cids = device::ChannelIdAllocator::new_from(42);
diff --git a/rust/trezor-thp/src/credential.rs b/rust/trezor-thp/src/credential.rs
index f13ad5ea..46d67f91 100644
--- a/rust/trezor-thp/src/credential.rs
+++ b/rust/trezor-thp/src/credential.rs
@@ -38,7 +38,4 @@ impl CredentialStore for NullCredentialStore {
pub trait CredentialVerifier {
/// Validate given protobuf-encoded credential.
fn verify(&self, remote_static_pubkey: &[u8], credential: &[u8]) -> PairingState;
-
- /// Return protobuf-encoded device properties to be used in `ChannelAllocationResponse` message.
- fn device_properties(&self) -> &[u8];
}
Why this scored 13/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.