refactor(rust/trezor-thp): remember handshake pairing state
What the developer wrote
Authored by Martin Milata
refactor(rust/trezor-thp): remember handshake pairing state
What changed, and why it matters
This commit refactors how a Trezor hardware wallet's Rust-based secure channel code remembers whether a pairing handshake succeeded. It moves the pairing-state information from a temporary handshake object into the long-lived channel object, adds a check that the handshake is fully done before completing, and exposes the remote device's public key and pairing state to the rest of the application. The changes look like a defensive cleanup rather than an active vulnerability fix, but they close a small window where pairing state could be lost or queried incorrectly.
Treat as a routine refactor with minor defensive-security value. Reviewers should verify that callers now use `Channel::handshake_pairing_state()` correctly and that the new `complete()` guard does not break any legitimate early-completion paths. No urgent action is indicated unless this commit is later linked to a disclosed security issue.
Security signals we found
State lifecycle hardening: pairing state is now persisted in the long-lived Channel instead of being transiently available on ChannelOpen
Added handshake_done() guard before complete() on the device side, preventing premature channel completion
Exposed remote static public key through Channel and NoiseCiphers, likely to support downstream credential verification / pairing decisions
Added credential_verifier() mutable accessor on device ChannelOpen, increasing visibility/control over credential verification state
No explicit security bug or CVE referenced in commit message or diff
Evidence from the diff
The patch modifies the THP (Trezor Host Protocol) channel implementation in Rust. Previously, ChannelOpen exposed pairing_state() only while in a specific handshake state. After this change, the pairing state is copied into Channel when complete() is called, and a new handshake_pairing_state() accessor is provided on Channel. It also adds remote_static_pubkey() accessors on Channel and NoiseCiphers, and a credential_verifier() accessor on the device-side ChannelOpen. A guard if !self.handshake_done() { return Err(Error::not_ready()); } is added to the device-side complete() method. The PairingState enum gains PartialEq/Eq and TryFrom<u8>/From<PairingState> for u8 conversions. These are structural improvements that make pairing state available after the handshake object is consumed.
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.rsInspect captured patch +81 / −34
diff --git a/rust/trezor-thp/src/channel/device.rs b/rust/trezor-thp/src/channel/device.rs
index a61eca6e..fbf891ca 100644
--- a/rust/trezor-thp/src/channel/device.rs
+++ b/rust/trezor-thp/src/channel/device.rs
@@ -385,13 +385,6 @@ impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
Ok(ps)
}
- pub fn pairing_state(&self) -> Option<PairingState> {
- match self.state {
- HandshakeState::SendingCompletionResponse { pairing_state } => Some(pairing_state),
- _ => None,
- }
- }
-
/// True if handshake finished and [`ChannelOpen::complete()`] can be called.
pub fn handshake_done(&self) -> bool {
// Done only after peer acknowledges completion response.
@@ -423,13 +416,19 @@ impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
///
/// [Pairing phase]: https://docs.trezor.io/trezor-firmware/common/thp/specification.html#pairing-phase
/// [Credential phase]: https://docs.trezor.io/trezor-firmware/common/thp/specification.html#credential-phase
- pub fn complete(self) -> Result<Channel<B>, Error> {
+ pub fn complete(mut self) -> Result<Channel<B>, Error> {
if self.channel.noise.is_none() {
return Err(Error::unexpected_input());
}
+ if !self.handshake_done() {
+ return Err(Error::not_ready());
+ }
log::debug!("Handshake complete.");
Ok(match self.state {
- HandshakeState::SendingCompletionResponse { .. } => self.channel,
+ HandshakeState::SendingCompletionResponse { pairing_state } => {
+ self.channel.pairing_state = pairing_state;
+ self.channel
+ }
_ => return Err(Error::unexpected_input()),
})
}
@@ -467,6 +466,10 @@ impl<C: CredentialVerifier, B: Backend> ChannelOpen<C, B> {
self.state = HandshakeState::SendingInitiationResponse;
Ok(())
}
+
+ pub fn credential_verifier(&mut self) -> &mut C {
+ &mut self.cred_verif
+ }
}
impl<C, B> ChannelIO for ChannelOpen<C, B>
diff --git a/rust/trezor-thp/src/channel/host.rs b/rust/trezor-thp/src/channel/host.rs
index b0d57c3e..37cb1043 100644
--- a/rust/trezor-thp/src/channel/host.rs
+++ b/rust/trezor-thp/src/channel/host.rs
@@ -435,17 +435,9 @@ impl<C: CredentialStore, B: Backend> ChannelOpen<C, B> {
self.device_properties.as_slice()
}
- /// Returns pairing state if handshake finished, or None otherwise.
- pub fn pairing_state(&self) -> Option<PairingState> {
- match self.state {
- HandshakeState::Finished { pairing_state } => Some(pairing_state),
- _ => None,
- }
- }
-
/// True if handshake finished and [`ChannelOpen::complete()`] can be called.
pub fn handshake_done(&self) -> bool {
- self.pairing_state().is_some()
+ matches!(self.state, HandshakeState::Finished { .. })
}
/// True if the handshake failed and the object should be discarded.
@@ -465,13 +457,16 @@ impl<C: CredentialStore, B: Backend> ChannelOpen<C, B> {
///
/// [Pairing phase]: https://docs.trezor.io/trezor-firmware/common/thp/specification.html#pairing-phase
/// [Credential phase]: https://docs.trezor.io/trezor-firmware/common/thp/specification.html#credential-phase
- pub fn complete(self) -> Result<Channel<B>, Error> {
+ pub fn complete(mut self) -> Result<Channel<B>, Error> {
if self.channel.noise.is_none() {
return Err(Error::unexpected_input());
}
log::debug!("Handshake complete.");
Ok(match self.state {
- HandshakeState::Finished { .. } => self.channel,
+ HandshakeState::Finished { pairing_state } => {
+ self.channel.pairing_state = pairing_state;
+ self.channel
+ }
_ => return Err(Error::unexpected_input()),
})
}
diff --git a/rust/trezor-thp/src/channel/mod.rs b/rust/trezor-thp/src/channel/mod.rs
index 4d9f358a..43c844a4 100644
--- a/rust/trezor-thp/src/channel/mod.rs
+++ b/rust/trezor-thp/src/channel/mod.rs
@@ -48,7 +48,7 @@ impl Nonce {
/// Sent by device after successful handshake to indicate whether pairing is required.
#[repr(u8)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq)]
pub enum PairingState {
Unpaired = 0,
Paired = 1,
@@ -66,14 +66,31 @@ impl TryFrom<&[u8]> for PairingState {
fn try_from(bytes: &[u8]) -> Result<Self> {
Ok(match bytes {
- [0] => Self::Unpaired,
- [1] => Self::Paired,
- [2] => Self::PairedAutoconnect,
+ [n] => PairingState::try_from(*n)?,
_ => return Err(Error::malformed_data()),
})
}
}
+impl TryFrom<u8> for PairingState {
+ type Error = Error;
+
+ fn try_from(val: u8) -> Result<Self> {
+ Ok(match val {
+ 0 => Self::Unpaired,
+ 1 => Self::Paired,
+ 2 => Self::PairedAutoconnect,
+ _ => return Err(Error::malformed_data()),
+ })
+ }
+}
+
+impl From<PairingState> for u8 {
+ fn from(pairing_state: PairingState) -> Self {
+ pairing_state as u8
+ }
+}
+
/// Is the channel currently sending or receiving a message?
enum ChannelState<R: Role> {
/// Ready to send or receive.
@@ -103,6 +120,7 @@ pub struct Channel<R: Role, B: Backend> {
noise: Option<NoiseCiphers<B>>,
send_ack: Option<SyncBits>,
state: ChannelState<R>,
+ pairing_state: PairingState,
}
impl<R: Role, B: Backend> Channel<R, B> {
@@ -113,6 +131,7 @@ impl<R: Role, B: Backend> Channel<R, B> {
noise: None,
send_ack: None,
state: ChannelState::Idle,
+ pairing_state: PairingState::Unpaired,
}
}
@@ -128,6 +147,21 @@ impl<R: Role, B: Backend> Channel<R, B> {
self.noise.as_ref().unwrap().handshake_hash()
}
+ /// Returns the channel pairing state at the end of the handshake.
+ /// This is read-only attribute to inform the application whether it needs to perform
+ /// pairing, or can directly transition to encrypted transport state.
+ pub fn handshake_pairing_state(&self) -> PairingState {
+ self.pairing_state
+ }
+
+ pub fn remote_static_pubkey(&self) -> &[u8; PUBKEY_LEN] {
+ self.noise.as_ref().unwrap().remote_static_pubkey()
+ }
+
+ pub fn is_failed(&self) -> bool {
+ matches!(self.state, ChannelState::Failed { .. })
+ }
+
/// Return the retransmission attempt number (the first transmission returns 0),
/// or `None` if the channel is currently not sending anything.
pub fn sending_retry(&self) -> Option<u8> {
diff --git a/rust/trezor-thp/src/channel/noise.rs b/rust/trezor-thp/src/channel/noise.rs
index 09fd1867..219a75fa 100644
--- a/rust/trezor-thp/src/channel/noise.rs
+++ b/rust/trezor-thp/src/channel/noise.rs
@@ -48,10 +48,12 @@ pub struct NoiseHandshake<R: Role, B: Backend> {
hss: HandshakeState<B::DH, B::Cipher, B::Hash>,
_phantom: PhantomData<R>,
}
+
pub struct NoiseCiphers<B: Backend> {
encrypt: CipherState<B::Cipher>,
decrypt: CipherState<B::Cipher>,
handshake_hash: [u8; HANDSHAKE_HASH_LEN],
+ remote_static_pubkey: [u8; PUBKEY_LEN],
}
impl<B: Backend> NoiseCiphers<B> {
@@ -75,6 +77,10 @@ impl<B: Backend> NoiseCiphers<B> {
pub fn handshake_hash(&self) -> &[u8; HANDSHAKE_HASH_LEN] {
&self.handshake_hash
}
+
+ pub fn remote_static_pubkey(&self) -> &[u8; PUBKEY_LEN] {
+ &self.remote_static_pubkey
+ }
}
impl<B: Backend> NoiseHandshake<Host, B> {
@@ -119,10 +125,13 @@ impl<B: Backend> NoiseHandshake<Host, B> {
self.hss.read_message(incoming, &mut [])?;
// Look up static key based on remote keys, or generate a new one.
- let remote_static_key = self.hss.get_rs().ok_or_else(Error::crypto_error)?;
- let remote_ephemeral_key = self.hss.get_re().ok_or_else(Error::crypto_error)?;
- let (local_static, pairing_credential) =
- Self::credential_from_store(cred_store, &remote_ephemeral_key, &remote_static_key)?;
+ let remote_static_pubkey = self.hss.get_rs().ok_or_else(Error::crypto_error)?;
+ let remote_ephemeral_pubkey = self.hss.get_re().ok_or_else(Error::crypto_error)?;
+ let (local_static, pairing_credential) = Self::credential_from_store(
+ cred_store,
+ &remote_ephemeral_pubkey,
+ &remote_static_pubkey,
+ )?;
self.hss.set_s(local_static);
buffer.fill(0);
@@ -137,12 +146,12 @@ impl<B: Backend> NoiseHandshake<Host, B> {
return Err(Error::crypto_error());
}
let (encrypt, decrypt) = self.hss.get_ciphers();
- let mut handshake_hash = [0u8; HANDSHAKE_HASH_LEN];
- handshake_hash.copy_from_slice(self.hss.get_hash());
+ let handshake_hash = self.hss.get_hash().try_into().unwrap();
let nc = NoiseCiphers {
encrypt,
decrypt,
handshake_hash,
+ remote_static_pubkey: remote_static_pubkey.as_slice().try_into().unwrap(),
};
Ok((nc, dest))
}
@@ -235,16 +244,22 @@ impl<B: Backend> NoiseHandshake<Device, B> {
return Err(Error::crypto_error());
}
- let remote_static_pubkey = self.hss.get_rs().ok_or_else(Error::crypto_error)?;
+ let remote_static_pubkey = self
+ .hss
+ .get_rs()
+ .ok_or_else(Error::crypto_error)?
+ .as_slice()
+ .try_into()
+ .unwrap();
+ let handshake_hash = self.hss.get_hash().try_into().unwrap();
let (decrypt, encrypt) = self.hss.get_ciphers();
- let mut handshake_hash = [0u8; HANDSHAKE_HASH_LEN];
- handshake_hash.copy_from_slice(self.hss.get_hash());
let mut nc = NoiseCiphers {
encrypt,
decrypt,
handshake_hash,
+ remote_static_pubkey,
};
- let pairing_state = cred_verifier.verify(remote_static_pubkey.as_slice(), &cred);
+ let pairing_state = cred_verifier.verify(&nc.remote_static_pubkey, &cred);
let payload = &[pairing_state as u8];
let plaintext_len = payload.len();
let dest = dest
Why this scored 26/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.