refactor(rust/trezor-thp): expose length constants
What the developer wrote
Authored by Martin Milata
refactor(rust/trezor-thp): expose length constants
What changed, and why it matters
This commit is a code cleanup: it moves several fixed-size buffer length constants out of individual Rust source files and into a shared module so they can be reused consistently. The actual buffer sizes used during the cryptographic handshake are recalculated and, in some cases, slightly changed (for example, the device-properties limit drops from 128 to 64 bytes and a new allocation-response size is introduced). There is no direct evidence in the commit that this fixes a security vulnerability; it reads as a refactoring to make the code easier to maintain.
No immediate security action is required. Treat as a normal refactoring commit. If auditing, verify that the newly derived constants still cover the largest possible handshake payload and that the reduction of MAX_DEVICE_PROPERTIES_LEN from 128 to 64 does not break any supported device property encoding.
Security signals we found
Refactoring of cryptographic handshake buffer sizes
Reduction of MAX_DEVICE_PROPERTIES_LEN from 128 to 64
Introduction of derived length constants for handshake messages
No explicit security claim or CVE reference in commit message
Evidence from the diff
The change refactors length constants in the Trezor THP (Trezor Host Protocol) Rust implementation. Previously device.rs and host.rs each defined a local INTERNAL_BUFFER_LEN of 192 and host.rs defined MAX_DEVICE_PROPERTIES_LEN of 128. This patch introduces shared constants in channel/mod.rs: MAX_DEVICE_PROPERTIES_LEN = 64, MAX_CREDENTIAL_LEN = 128, HANDSHAKE_BUFFER_HTD_LEN, MAX_ALLOC_RESPONSE_LEN, HANDSHAKE_BUFFER_DTH_LEN, and HANDSHAKE_BUFFER_LEN. It also adds a small max helper in util.rs because Ord::max is not const. The noise.rs module now uses MAX_CREDENTIAL_LEN instead of the old MAX_KEY_AND_CREDENTIAL_LEN and adjusts an internal lookup buffer to PRIVKEY_LEN + MAX_CREDENTIAL_LEN. The visible effect is that buffer sizes are now derived from protocol field sizes rather than hard-coded magic numbers.
Changed components
rust/trezor-thp/src/channel/device.rsrust/trezor-thp/src/channel/host.rsrust/trezor-thp/src/channel/mod.rsrust/trezor-thp/src/channel/noise.rsrust/trezor-thp/src/util.rsInspect captured patch +38 / −24
diff --git a/rust/trezor-thp/src/channel/device.rs b/rust/trezor-thp/src/channel/device.rs
index 543cfe8c..1e297ad9 100644
--- a/rust/trezor-thp/src/channel/device.rs
+++ b/rust/trezor-thp/src/channel/device.rs
@@ -4,7 +4,8 @@ use crate::{
Backend, ChannelIO, Device, Error,
alternating_bit::SyncBits,
channel::{
- ChannelState, Nonce, PRIVKEY_LEN, PacketInResult, PairingState, noise::NoiseHandshake,
+ ChannelState, HANDSHAKE_BUFFER_LEN, Nonce, PRIVKEY_LEN, PacketInResult, PairingState,
+ noise::NoiseHandshake,
},
credential::CredentialVerifier,
error::TransportError,
@@ -21,11 +22,6 @@ use core::{
sync::atomic::{AtomicU16, Ordering},
};
-// Must fit any of:
-// - device_properties + overhead
-// - 2 DH keys + 2 AEAD tags (2*32+2*16=96) + overhead
-// - DH key + credential + 2 AEAD tags + overhead
-const INTERNAL_BUFFER_LEN: usize = 192;
// As long as `packet_out` is called soon after `packet_in` there shouldn't be an accumulation
// of outgoing messages. However there still can be >1 during normal operation, e.g. when we're
// responding to PING at the same time the application requests sending an error.
@@ -295,7 +291,7 @@ pub struct ChannelOpen<C: CredentialVerifier, B: Backend> {
channel: Channel<B>,
state: HandshakeState,
noise: NoiseHandshake<Device, B>,
- internal_buffer: heapless::Vec<u8, INTERNAL_BUFFER_LEN>,
+ internal_buffer: heapless::Vec<u8, HANDSHAKE_BUFFER_LEN>,
cred_verif: C,
}
diff --git a/rust/trezor-thp/src/channel/host.rs b/rust/trezor-thp/src/channel/host.rs
index bd55b12c..02c50ed8 100644
--- a/rust/trezor-thp/src/channel/host.rs
+++ b/rust/trezor-thp/src/channel/host.rs
@@ -3,7 +3,10 @@ use heapless;
use crate::{
Backend, ChannelIO, Error, Host,
alternating_bit::SyncBits,
- channel::{ChannelState, Nonce, PacketInResult, PairingState, noise::NoiseHandshake},
+ channel::{
+ ChannelState, HANDSHAKE_BUFFER_LEN, MAX_ALLOC_RESPONSE_LEN, MAX_DEVICE_PROPERTIES_LEN,
+ Nonce, PacketInResult, PairingState, noise::NoiseHandshake,
+ },
credential::CredentialStore,
fragment::{Fragmenter, Reassembler},
header::{
@@ -15,13 +18,6 @@ use crate::{
use core::marker::PhantomData;
-// Must fit any of:
-// - device_properties + overhead
-// - 2 DH keys + 2 AEAD tags (2*32+2*16=96) + overhead
-// - DH key + credential + 2 AEAD tags + overhead
-const INTERNAL_BUFFER_LEN: usize = 192;
-const MAX_DEVICE_PROPERTIES_LEN: usize = 128;
-
pub type Channel<B> = super::Channel<Host, B>;
enum AllocationState {
@@ -59,7 +55,7 @@ enum PingState {
/// Because host often only needs a single channel, you can throw away the Mux
/// after allocating one, if you don't need the keep-alive functionality.
pub struct Mux<B> {
- internal_buffer: heapless::Vec<u8, MAX_DEVICE_PROPERTIES_LEN>,
+ internal_buffer: heapless::Vec<u8, MAX_ALLOC_RESPONSE_LEN>,
channel_allocation: AllocationState,
ping: PingState,
_phantom: PhantomData<B>,
@@ -343,7 +339,7 @@ pub struct ChannelOpen<C: CredentialStore, B: Backend> {
channel: Channel<B>,
state: HandshakeState,
noise: NoiseHandshake<Host, B>,
- internal_buffer: heapless::Vec<u8, INTERNAL_BUFFER_LEN>,
+ internal_buffer: heapless::Vec<u8, HANDSHAKE_BUFFER_LEN>,
device_properties: heapless::Vec<u8, MAX_DEVICE_PROPERTIES_LEN>,
cred_store: C,
}
diff --git a/rust/trezor-thp/src/channel/mod.rs b/rust/trezor-thp/src/channel/mod.rs
index 4ea73745..477c56ac 100644
--- a/rust/trezor-thp/src/channel/mod.rs
+++ b/rust/trezor-thp/src/channel/mod.rs
@@ -9,9 +9,11 @@ mod test;
use crate::{
Error, Role,
alternating_bit::{ChannelSync, SyncBits},
+ crc32::CHECKSUM_LEN,
error::{Result, TransportError},
fragment::{Fragmenter, Reassembler},
header::{BROADCAST_CHANNEL_ID, Header, NONCE_LEN, parse_cb_channel, parse_u16},
+ util::max,
};
use core::num::NonZeroU16;
@@ -21,6 +23,23 @@ pub use noise::{
Backend, Cipher, DH, HANDSHAKE_HASH_LEN, Hash, PRIVKEY_LEN, PUBKEY_LEN, TAG_LEN, U8Array,
};
+pub const MAX_DEVICE_PROPERTIES_LEN: usize = 64;
+pub const MAX_CREDENTIAL_LEN: usize = 128;
+
+// Size of internal buffer needed when opening a channel: host-to-device direction.
+const HANDSHAKE_BUFFER_HTD_LEN: usize =
+ PUBKEY_LEN + MAX_CREDENTIAL_LEN + 2 * TAG_LEN + CHECKSUM_LEN; // HandshakeCompletionRequest
+
+const MAX_ALLOC_RESPONSE_LEN: usize =
+ (NONCE_LEN as usize) + 2 + MAX_DEVICE_PROPERTIES_LEN + CHECKSUM_LEN;
+// Size of internal buffer needed when opening a channel: device-to-host direction.
+const HANDSHAKE_BUFFER_DTH_LEN: usize = max(
+ MAX_ALLOC_RESPONSE_LEN, // ChannelAllocationResponse
+ 2 * PUBKEY_LEN + 2 * TAG_LEN + CHECKSUM_LEN, // HandshakeInitiationResponse
+);
+
+const HANDSHAKE_BUFFER_LEN: usize = max(HANDSHAKE_BUFFER_HTD_LEN, HANDSHAKE_BUFFER_DTH_LEN);
+
const APP_HEADER_LEN: usize = 3; // session id (1) + message type (2)
/// Used during channel allocation on broadcast channel.
diff --git a/rust/trezor-thp/src/channel/noise.rs b/rust/trezor-thp/src/channel/noise.rs
index 219a75fa..53aebd67 100644
--- a/rust/trezor-thp/src/channel/noise.rs
+++ b/rust/trezor-thp/src/channel/noise.rs
@@ -12,7 +12,7 @@ pub use trezor_noise_protocol::{Cipher, DH, Hash, U8Array};
use crate::{
Device, Error, Host, Role,
- channel::PairingState,
+ channel::{MAX_CREDENTIAL_LEN, PairingState},
credential::{CredentialStore, CredentialVerifier},
util::prepare_zeroed,
};
@@ -23,7 +23,6 @@ pub const HANDSHAKE_HASH_LEN: usize = 32;
pub const PRIVKEY_LEN: usize = 32;
pub const PUBKEY_LEN: usize = 32;
pub const TAG_LEN: usize = 16;
-pub const MAX_KEY_AND_CREDENTIAL_LEN: usize = 128;
/// Cryptography backend trait.
///
@@ -160,11 +159,11 @@ impl<B: Backend> NoiseHandshake<Host, B> {
cs: &impl CredentialStore,
re: &DHPubKey<B>,
rs: &DHPubKey<B>,
- ) -> Result<(DHPrivKey<B>, heapless::Vec<u8, MAX_KEY_AND_CREDENTIAL_LEN>), Error>
+ ) -> Result<(DHPrivKey<B>, heapless::Vec<u8, MAX_CREDENTIAL_LEN>), Error>
where
DHPrivKey<B>: U8Array,
{
- let mut buf = heapless::Vec::new();
+ let mut buf = heapless::Vec::<u8, { PRIVKEY_LEN + MAX_CREDENTIAL_LEN }>::new();
prepare_zeroed(&mut buf);
let result = cs.lookup(re.as_slice(), rs.as_slice(), buf.as_mut_slice());
if let Some(found) = result {
@@ -173,9 +172,8 @@ impl<B: Backend> NoiseHandshake<Host, B> {
.map_err(|_| Error::insufficient_buffer())?;
return Ok((found_key, found_credential));
}
- buf.clear();
let new_key = <B::DH as DH>::genkey();
- Ok((new_key, buf))
+ Ok((new_key, heapless::Vec::new()))
}
}
@@ -235,7 +233,7 @@ impl<B: Backend> NoiseHandshake<Device, B> {
return Err(Error::malformed_data());
}
let cred_len = incoming.len().saturating_sub(overhead_len);
- let mut cred = heapless::Vec::<u8, MAX_KEY_AND_CREDENTIAL_LEN>::new();
+ let mut cred = heapless::Vec::<u8, MAX_CREDENTIAL_LEN>::new();
cred.resize(cred_len, 0u8)
.map_err(|_| Error::insufficient_buffer())?;
self.hss.read_message(incoming, &mut cred)?;
diff --git a/rust/trezor-thp/src/util.rs b/rust/trezor-thp/src/util.rs
index 97253f9f..2d056e5a 100644
--- a/rust/trezor-thp/src/util.rs
+++ b/rust/trezor-thp/src/util.rs
@@ -2,3 +2,8 @@ pub(crate) fn prepare_zeroed<const C: usize>(buf: &mut heapless::Vec<u8, C>) {
buf.clear();
buf.resize(buf.capacity(), 0u8).unwrap();
}
+
+// Ord::max is not (yet) const.
+pub(crate) const fn max(lhs: usize, rhs: usize) -> usize {
+ if lhs > rhs { lhs } else { rhs }
+}
Why this scored 12/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.