From 65e76fcb58718e41b37aded4b68ada0d8c2b5472 Mon Sep 17 00:00:00 2001 From: Lewis Lakerink Date: Tue, 17 Mar 2026 13:29:04 +1100 Subject: [PATCH] Fix mesh broadcast packet structure alignment for cross-firmware compatibility - Restore struct alignment to match wire format expectations for encryption/decryption - Fix undefined behavior in status packet data access - Add comprehensive field offset validation tests for broadcast encryption path - Bump version to ensure firmware compatibility detection This resolves status advertisement decryption failures when interoperating with legacy firmware versions that use the original struct layout. --- rust/src/sdk/ble_app/ble_ll_pair.rs | 40 ++++++++++- .../sdk/ble_app/light_ll/mesh_management.rs | 4 +- .../sdk/ble_app/light_ll/packet_processing.rs | 2 +- rust/src/sdk/packet_types.rs | 67 +++++++++++++++---- rust/src/version.rs | 2 +- sdk/version.in | 2 +- 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/rust/src/sdk/ble_app/ble_ll_pair.rs b/rust/src/sdk/ble_app/ble_ll_pair.rs index aa4f7d0..4b9ecfc 100644 --- a/rust/src/sdk/ble_app/ble_ll_pair.rs +++ b/rust/src/sdk/ble_app/ble_ll_pair.rs @@ -1397,7 +1397,7 @@ mod tests { ttl: 0, internal_par2: [0; 4], dst_adr: 0, - no_use: [0; 5], + no_use: [0; 4], }, }; packet @@ -1509,6 +1509,44 @@ mod tests { mock_aes_att_decryption_packet(Any, Any, Any, Any).assert_called(1); } + #[test] + #[mry::lock(aes_att_decryption_packet)] + fn test_pair_dec_packet_mesh_broadcast_mic_offset() { + // Verify that broadcast decryption reads the MIC from internal_par2[1] (absolute offset 41). + // When MeshPkt incorrectly uses repr(C, packed) instead of repr(C, align(4)), + // internal_par2 shifts from offset 40 to 39, causing the MIC to be read from + // offset 40 instead of 41, breaking cross-firmware compatibility. + SECURITY_ENABLE.set(true); + let mut packet = create_test_mesh_packet(true); // broadcast + PAIR_STATE.lock().pair_ltk.fill(0xDD); + + // Write distinguishable bytes at the critical offsets + let base = core::ptr::addr_of!(packet) as *mut u8; + unsafe { + // Offset 40 = internal_par2[0] with align(4). Would be internal_par2[1] if packed. + *base.add(40) = 0xAA; + // Offset 41 = internal_par2[1] with align(4). This is the correct MIC position. + *base.add(41) = 0xBB; + *base.add(42) = 0xCC; + } + + mock_aes_att_decryption_packet(Any, Any, Any, Any).returns_with( + move |_sk: Vec, _iv: Vec, mic: Vec, _data: Vec| -> bool { + // MIC must be [0xBB, 0xCC] from offset 41-42, NOT [0xAA, 0xBB] from offset 40-41 + assert_eq!( + mic, + vec![0xBB, 0xCC], + "Broadcast MIC must be read from internal_par2[1] at offset 41, not offset 40" + ); + true + }, + ); + + let result = pair_dec_packet_mesh(&mut packet); + assert!(result); + mock_aes_att_decryption_packet(Any, Any, Any, Any).assert_called(1); + } + #[test] #[mry::lock(aes_att_decryption_packet)] fn test_pair_dec_packet_mesh_direct_success() { diff --git a/rust/src/sdk/ble_app/light_ll/mesh_management.rs b/rust/src/sdk/ble_app/light_ll/mesh_management.rs index 37a3f98..e9f7224 100644 --- a/rust/src/sdk/ble_app/light_ll/mesh_management.rs +++ b/rust/src/sdk/ble_app/light_ll/mesh_management.rs @@ -713,7 +713,7 @@ pub fn mesh_send_online_status() { // Get direct access to packet payload for efficient manipulation let pktdata = unsafe { &mut *slice_from_raw_parts_mut( - addr_of!(pkt_light_adv_status.att_write().value) as *mut u8, + addr_of_mut!(pkt_light_adv_status.att_write_mut().value) as *mut u8, core::mem::size_of::(), ) }; @@ -851,7 +851,7 @@ pub fn mesh_construct_packet( internal_par1: [0; 5], // Internal parameters ttl: 0, // Time-to-live hop counter internal_par2: [0; 4], // Additional internal parameters - no_use: [0; 5], // Reserved/unused bytes + no_use: [0; 4], // Reserved/unused bytes }; // Convert 32-bit sequence number to 24-bit little-endian format diff --git a/rust/src/sdk/ble_app/light_ll/packet_processing.rs b/rust/src/sdk/ble_app/light_ll/packet_processing.rs index 5973a51..891a6c6 100644 --- a/rust/src/sdk/ble_app/light_ll/packet_processing.rs +++ b/rust/src/sdk/ble_app/light_ll/packet_processing.rs @@ -1593,7 +1593,7 @@ mod tests { internal_par1: [0; 5], ttl: 0, internal_par2: [0; 4], - no_use: [0; 5], + no_use: [0; 4], }, } } diff --git a/rust/src/sdk/packet_types.rs b/rust/src/sdk/packet_types.rs index b199222..186bd00 100644 --- a/rust/src/sdk/packet_types.rs +++ b/rust/src/sdk/packet_types.rs @@ -170,7 +170,7 @@ pub struct PacketAttData { } #[derive(Clone, Copy, Default)] -#[repr(C, packed)] +#[repr(C, align(4))] pub struct MeshPkt { pub head: PacketL2capHead, // 0 pub src_tx: u16, // 10 @@ -179,12 +179,12 @@ pub struct MeshPkt { pub src_adr: u16, // 16 pub dst_adr: u16, // 18 pub op: u8, // 20 - pub vendor_id: u16, // 21 - pub par: [u8; 10], // 23 - pub internal_par1: [u8; 5], // 33 - pub ttl: u8, // 38 - pub internal_par2: [u8; 4], // 39 - pub no_use: [u8; 5], // 43 size must 48, when is set to be rf tx address. + pub vendor_id: u16, // 22 (1 byte padding after op for u16 alignment) + pub par: [u8; 10], // 24 + pub internal_par1: [u8; 5], // 34 + pub ttl: u8, // 39 + pub internal_par2: [u8; 4], // 40 + pub no_use: [u8; 4], // 44 size must 48, when is set to be rf tx address. } const_assert!(mem::size_of::() == 48); @@ -637,7 +637,7 @@ mod tests { internal_par1: [0x18, 0x19, 0x1A, 0x1B, 0x1C], ttl: 0x1D, internal_par2: [0x1E, 0x1F, 0x20, 0x21], - no_use: [0x22, 0x23, 0x24, 0x25, 0x26], + no_use: [0x22, 0x23, 0x24, 0x25], }, }; @@ -777,10 +777,53 @@ mod tests { assert_eq!(mem::offset_of!(MeshPkt, src_adr), 16); assert_eq!(mem::offset_of!(MeshPkt, dst_adr), 18); assert_eq!(mem::offset_of!(MeshPkt, op), 20); - assert_eq!(mem::offset_of!(MeshPkt, vendor_id), 21); - assert_eq!(mem::offset_of!(MeshPkt, par), 23); - assert_eq!(mem::offset_of!(MeshPkt, ttl), 38); - assert_eq!(mem::offset_of!(MeshPkt, no_use), 43); + assert_eq!(mem::offset_of!(MeshPkt, vendor_id), 22); + assert_eq!(mem::offset_of!(MeshPkt, par), 24); + assert_eq!(mem::offset_of!(MeshPkt, internal_par1), 34); + assert_eq!(mem::offset_of!(MeshPkt, ttl), 39); + assert_eq!(mem::offset_of!(MeshPkt, internal_par2), 40); + assert_eq!(mem::offset_of!(MeshPkt, no_use), 44); + } + + /// Tests that MeshPkt broadcast encryption fields are at correct absolute byte offsets. + /// pair_dec_packet_mesh/pair_enc_packet_mesh for broadcast packets (chan_id == 0xffff) use: + /// - IV: 8 bytes starting from head.rf_len (offset 5) + /// - MIC: 2 bytes at internal_par2[1] (must be offset 41) + /// - Data: 0x1c bytes starting from sno (offset 13, so data covers bytes 13..41) + /// The MIC must immediately follow the data region (offset 41) for AES-CCM to work correctly. + /// If MeshPkt uses packed repr instead of align(4), internal_par2 shifts to offset 39, + /// putting internal_par2[1] at offset 40, which overlaps the data region and breaks decryption. + #[test] + fn test_meshpkt_broadcast_encryption_field_offsets() { + let pkt = MeshPkt::default(); + let base = core::ptr::addr_of!(pkt) as usize; + + // IV source: rf_len in head (offset 5 within PacketL2capHead) + let rf_len_offset = unsafe { core::ptr::addr_of!(pkt.head.rf_len) as usize - base }; + assert_eq!( + rf_len_offset, 5, + "rf_len must be at offset 5 for broadcast IV" + ); + + // Encrypted data region: 0x1c (28) bytes starting from sno + let sno_offset = mem::offset_of!(MeshPkt, sno); + assert_eq!( + sno_offset, 13, + "sno must be at offset 13 for broadcast data" + ); + + // MIC location: internal_par2[1] - must be at offset 41 (sno + 0x1c = 13 + 28) + let internal_par2_offset = mem::offset_of!(MeshPkt, internal_par2); + assert_eq!( + internal_par2_offset, 40, + "internal_par2 must be at offset 40" + ); + let mic_offset = internal_par2_offset + 1; // internal_par2[1] + assert_eq!( + mic_offset, + sno_offset + 0x1c, + "broadcast MIC (internal_par2[1]) must immediately follow the 0x1c-byte data region" + ); } /// Tests PktBuf structure diff --git a/rust/src/version.rs b/rust/src/version.rs index 069534d..9b0cc61 100644 --- a/rust/src/version.rs +++ b/rust/src/version.rs @@ -1 +1 @@ -pub static BUILD_VERSION: u32 = 3518; +pub static BUILD_VERSION: u32 = 3520; diff --git a/sdk/version.in b/sdk/version.in index d740839..261e1d8 100644 --- a/sdk/version.in +++ b/sdk/version.in @@ -1,2 +1,2 @@ -.equ BUILD_VERSION,3517 +.equ BUILD_VERSION,3520 .equ XTAL_16MHZ,0