diff --git a/lib-network/src/blockchain_sync/mod.rs b/lib-network/src/blockchain_sync/mod.rs index f8f07f8f..07312aef 100644 --- a/lib-network/src/blockchain_sync/mod.rs +++ b/lib-network/src/blockchain_sync/mod.rs @@ -55,9 +55,14 @@ use std::time::Duration; #[deprecated(since = "0.1.0", note = "Use crate::mtu::BLE_CHUNK_SIZE instead")] pub const BLE_CHUNK_SIZE_COMPAT: usize = BLE_CHUNK_SIZE; +/// Backward compatibility: Direct re-export of BLE_CHUNK_SIZE +#[deprecated(since = "0.1.0", note = "Use crate::mtu::BLE_CHUNK_SIZE instead")] +pub use crate::mtu::BLE_CHUNK_SIZE; + #[deprecated(since = "0.1.0", note = "Use crate::mtu::BLUETOOTH_CLASSIC_CHUNK_SIZE instead")] pub const CLASSIC_CHUNK_SIZE: usize = BLUETOOTH_CLASSIC_CHUNK_SIZE; +/// Backward compatibility: Direct re-export of WIFI_CHUNK_SIZE #[deprecated(since = "0.1.0", note = "Use crate::mtu::WIFI_DIRECT_CHUNK_SIZE instead")] pub const WIFI_CHUNK_SIZE: usize = WIFI_DIRECT_CHUNK_SIZE; diff --git a/lib-network/src/fragmentation.rs b/lib-network/src/fragmentation.rs index a2614264..78a8f2f9 100644 --- a/lib-network/src/fragmentation.rs +++ b/lib-network/src/fragmentation.rs @@ -83,6 +83,15 @@ impl FragmentHeader { let total_fragments = u16::from_le_bytes([bytes[4], bytes[5]]); let fragment_index = u16::from_le_bytes([bytes[6], bytes[7]]); + // Validate fragment_index is within bounds + if fragment_index >= total_fragments { + return Err(anyhow!( + "Invalid fragment: index {} >= total_fragments {}", + fragment_index, + total_fragments + )); + } + Ok(Self { message_id, total_fragments, @@ -137,6 +146,26 @@ impl Fragment { /// Fragment a message into chunks suitable for transmission /// +/// This function splits a payload into multiple fragments, each with an 8-byte header +/// containing sequence information for reassembly. +/// +/// ## Message ID Generation - Collision Risk ⚠️ +/// +/// **WARNING**: The message_id is generated using a simple wrapping sum of payload bytes: +/// ```ignore +/// message_id = payload.iter().fold(0u32, |acc, &b| acc.wrapping_add(b as u32)) +/// ``` +/// +/// This algorithm is **NOT cryptographically secure** and can easily produce collisions: +/// - Two different messages with the same byte sum will get the same message_id +/// - Messages sent in the same session could have their fragments mixed during reassembly +/// - Attackers could craft messages to intentionally cause collisions +/// +/// **For production use, prefer `fragmentation_v2` which provides:** +/// - Session-scoped identity `(session_id, message_seq, fragment_index)` +/// - Sequential message_seq counters (zero collisions) +/// - Proper isolation between concurrent sessions +/// /// ## Arguments /// /// - `payload`: The message data to fragment @@ -160,6 +189,7 @@ pub fn fragment_message(payload: &[u8], chunk_size: usize) -> Vec { } // Generate unique message ID (use hash of payload for determinism) + // WARNING: Simple wrapping sum is NOT collision-resistant - see function docs let message_id = payload.iter().fold(0u32, |acc, &b| acc.wrapping_add(b as u32)); let total_fragments = ((payload.len() + chunk_size - 1) / chunk_size) as u16; diff --git a/lib-network/src/protocols/bluetooth/gatt.rs b/lib-network/src/protocols/bluetooth/gatt.rs index 633e8e22..2f9368ef 100644 --- a/lib-network/src/protocols/bluetooth/gatt.rs +++ b/lib-network/src/protocols/bluetooth/gatt.rs @@ -126,8 +126,27 @@ pub fn fragment_data(data: &[u8], mtu: u16) -> Vec> { /// **REFACTORED**: Now uses centralized fragmentation with standardized 8-byte headers. /// Returns Vec of wire-format fragments ready for GATT transmission. /// +/// ## Wire Format Compatibility Warning +/// +/// **BREAKING CHANGE**: The wire format has changed from 12-byte headers (old format) +/// to 8-byte headers (new v1 format): +/// +/// - **Old format** (12 bytes): `message_id: u64, total_fragments: u16, fragment_index: u16` +/// - **New v1 format** (8 bytes): `message_id: u32, total_fragments: u16, fragment_index: u16` +/// +/// This change is **NOT backward compatible**. Old clients will fail to parse fragments +/// created by this function, and vice versa. For production deployments requiring +/// compatibility, use `fragment_large_message_v2` which includes protocol versioning. +/// +/// ## Message ID Parameter +/// +/// **Note**: The `message_id` parameter (u64) is currently **ignored**. Message IDs are +/// generated internally as u32 by the centralized fragmentation logic. Callers should +/// not rely on a specific `message_id` being preserved. This breaks backward compatibility +/// with code that depends on specific message IDs. +/// /// **DEPRECATED**: Use `fragment_large_message_v2` with a GattSession for new code. -pub fn fragment_large_message(message_id: u64, data: &[u8], mtu: u16) -> Vec> { +pub fn fragment_large_message(_message_id: u64, data: &[u8], mtu: u16) -> Vec> { // ATT overhead is 3 bytes, leave room for our 8-byte header let chunk_size = (mtu as usize).saturating_sub(3 + 8).max(20); @@ -205,18 +224,26 @@ impl FragmentReassembler { let result = self.inner.add_fragment(parsed)?; if let Some(ref data) = result { - info!("✅ Reassembled message from {} fragments ({} bytes)", - self.inner.pending_count(), data.len()); + info!("✅ Reassembled complete message ({} bytes)", data.len()); } Ok(result) } /// Clear stale fragments older than timeout - pub fn cleanup_stale_fragments(&mut self, _message_id: u64) { - // Clear all pending (centralized reassembler doesn't track individual message cleanup) - self.inner.clear(); - warn!("🗑️ Cleaned up all stale fragments"); + /// + /// **Note**: The centralized v1 reassembler does not support per-message cleanup. + /// This method only logs the request. To avoid disrupting reassembly of valid + /// in-progress messages, use v2 fragmentation with GattSession which provides + /// automatic timeout-based cleanup. + pub fn cleanup_stale_fragments(&mut self, message_id: u64) { + // Centralized reassembler currently does not support per-message cleanup. + // To avoid disrupting reassembly of other in-progress messages, we only log for now. + debug!( + "Requested cleanup of stale fragments for message_id {}, \ + but centralized reassembler does not support targeted cleanup; no action taken", + message_id + ); } }