rust/bt-common: Encapsulate AdvertisingSetId and validate bounds Encapsulate the internal AdvertisingSetId u8 value and enforce valid SID bounds (0x00..=0x0F) per Core Spec v5.3 Vol 6, Part B, Section 2.3.4 and BASS v1.0.1 Section 3.1.1.4 Table 3.5. Bug: 539983871 Test: cargo test Change-Id: I2cf2073f4dbcf4910bbc154a6a044bc5f9db1adc Reviewed-on: https://bluetooth-review.googlesource.com/c/bluetooth/+/3300
diff --git a/rust/bt-bass/src/client.rs b/rust/bt-bass/src/client.rs index 8970b1e..d6d239f 100644 --- a/rust/bt-bass/src/client.rs +++ b/rust/bt-bass/src/client.rs
@@ -737,7 +737,7 @@ BroadcastId::try_from(0x11).unwrap(), AddressType::Public, [0x04, 0x10, 0x00, 0x00, 0x00, 0x00], - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), PaSync::DoNotSync, PeriodicAdvertisingInterval::unknown(), vec![], @@ -759,7 +759,7 @@ source_id: 0x11, source_address_type: AddressType::Public, source_address: [1, 2, 3, 4, 5, 6], - source_adv_sid: AdvertisingSetId(1), + source_adv_sid: AdvertisingSetId::try_from(1).unwrap(), broadcast_id: BroadcastId::try_from(0x11).unwrap(), pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BroadcastCodeRequired, @@ -801,7 +801,7 @@ source_id: 0x11, source_address_type: AddressType::Public, source_address: [1, 2, 3, 4, 5, 6], - source_adv_sid: AdvertisingSetId(1), + source_adv_sid: AdvertisingSetId::try_from(1).unwrap(), broadcast_id: BroadcastId::try_from(0x11).unwrap(), pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BroadcastCodeRequired, @@ -851,7 +851,7 @@ source_id: 0x11, source_address_type: AddressType::Public, source_address: [1, 2, 3, 4, 5, 6], - source_adv_sid: AdvertisingSetId(1), + source_adv_sid: AdvertisingSetId::try_from(1).unwrap(), broadcast_id: BroadcastId::try_from(0x11).unwrap(), pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BroadcastCodeRequired, @@ -922,7 +922,7 @@ source_id: 0x11, source_address_type: AddressType::Public, source_address: [1, 2, 3, 4, 5, 6], - source_adv_sid: AdvertisingSetId(1), + source_adv_sid: AdvertisingSetId::try_from(1).unwrap(), broadcast_id: bid, pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BroadcastCodeRequired, @@ -965,7 +965,7 @@ source_id: 0x01, source_address_type: AddressType::Public, source_address: [1, 2, 3, 4, 5, 6], - source_adv_sid: AdvertisingSetId(1), + source_adv_sid: AdvertisingSetId::try_from(1).unwrap(), broadcast_id: BroadcastId::try_from(0x030201).unwrap(), pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BroadcastCodeRequired,
diff --git a/rust/bt-bass/src/server.rs b/rust/bt-bass/src/server.rs index 4521795..c7bbde7 100644 --- a/rust/bt-bass/src/server.rs +++ b/rust/bt-bass/src/server.rs
@@ -293,7 +293,7 @@ mod tests { use super::*; use bt_bap::types::BroadcastId; - use bt_common::core::AddressType; + use bt_common::core::{AddressType, AdvertisingSetId}; use bt_gatt::server::ReadResponder; use bt_gatt::test_utils::{FakeServer, FakeServerEvent, FakeTypes}; use bt_gatt::types::GattError; @@ -322,7 +322,7 @@ source_id, AddressType::Public, [0x01, 0x02, 0x03, 0x04, 0x05, 0x06], - 1, + AdvertisingSetId::try_from(1).unwrap(), BroadcastId::try_from(0x123456).unwrap(), PaSyncState::NotSynced, EncryptionStatus::NotEncrypted,
diff --git a/rust/bt-bass/src/types.rs b/rust/bt-bass/src/types.rs index b881328..2418d9e 100644 --- a/rust/bt-bass/src/types.rs +++ b/rust/bt-bass/src/types.rs
@@ -210,7 +210,7 @@ let advertiser_address_type = AddressType::try_from(buf[1])?; let mut advertiser_address = [0; ADDRESS_BYTE_SIZE]; advertiser_address.clone_from_slice(&buf[2..8]); - let advertising_sid = AdvertisingSetId(buf[8]); + let advertising_sid = AdvertisingSetId::try_from(buf[8])?; let broadcast_id = BroadcastId::decode(&buf[9..12]).0?; let pa_sync = PaSync::try_from(buf[12])?; let pa_interval = @@ -259,7 +259,7 @@ buf[0] = Self::opcode() as u8; buf[1] = self.advertiser_address_type as u8; buf[2..8].copy_from_slice(&self.advertiser_address); - buf[8] = self.advertising_sid.0; + buf[8] = self.advertising_sid.value(); self.broadcast_id.encode(&mut buf[9..12])?; buf[12] = u8::from(self.pa_sync); buf[13..15].copy_from_slice(&self.pa_interval.0.to_le_bytes()); @@ -767,7 +767,7 @@ source_id: u8, source_address_type: AddressType, source_address: [u8; ADDRESS_BYTE_SIZE], - source_adv_sid: u8, + source_adv_sid: AdvertisingSetId, broadcast_id: BroadcastId, pa_sync_state: PaSyncState, big_encryption: EncryptionStatus, @@ -777,7 +777,7 @@ source_id, source_address_type, source_address, - source_adv_sid: AdvertisingSetId(source_adv_sid), + source_adv_sid, broadcast_id, pa_sync_state, big_encryption, @@ -815,7 +815,7 @@ let source_address_type = AddressType::try_from(buf[1])?; let mut source_address = [0; ADDRESS_BYTE_SIZE]; source_address.clone_from_slice(&buf[2..8]); - let source_adv_sid = AdvertisingSetId(buf[8]); + let source_adv_sid = AdvertisingSetId::try_from(buf[8])?; let broadcast_id = BroadcastId::decode(&buf[9..12]).0?; let pa_sync_state = PaSyncState::try_from(buf[12])?; @@ -876,7 +876,7 @@ buf[0] = self.source_id; buf[1] = self.source_address_type as u8; buf[2..8].copy_from_slice(&self.source_address); - buf[8] = self.source_adv_sid.0; + buf[8] = self.source_adv_sid.value(); self.broadcast_id.encode(&mut buf[9..12])?; buf[12] = u8::from(self.pa_sync_state); let mut idx = 13 + self.big_encryption.encoded_len(); @@ -1177,7 +1177,7 @@ let op = AddSourceOperation::new( AddressType::Public, [0x04, 0x10, 0x00, 0x00, 0x00, 0x00], - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), BroadcastId::try_from(0x11).unwrap(), PaSync::DoNotSync, PeriodicAdvertisingInterval::unknown(), @@ -1209,7 +1209,7 @@ let op = AddSourceOperation::new( AddressType::Random, [0x04, 0x10, 0x00, 0x00, 0x00, 0x00], - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), BroadcastId::try_from(0x11).unwrap(), PaSync::SyncPastAvailable, PeriodicAdvertisingInterval::unknown(), @@ -1234,6 +1234,25 @@ } #[test] + fn invalid_advertising_sid_decoding() { + // AddSourceOperation with invalid Advertising_SID (0x10 > 0x0F) + let invalid_add_source_bytes = vec![ + 0x02, 0x00, 0x04, 0x10, 0x00, 0x00, 0x00, 0x00, 0x10, 0x11, 0x00, 0x00, 0x00, 0xFF, + 0xFF, 0x00, + ]; + let (decoded, _) = AddSourceOperation::decode(&invalid_add_source_bytes); + assert_eq!(decoded, Err(PacketError::OutOfRange)); + + // ReceiveState with invalid Advertising_SID (0x10 > 0x0F) + let invalid_receive_state_bytes = vec![ + 0x01, 0x00, 0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A, 0x10, 0x03, 0x02, 0x01, 0x02, 0x00, + 0x00, + ]; + let (decoded, _) = BroadcastReceiveState::decode(&invalid_receive_state_bytes); + assert_eq!(decoded, Err(PacketError::OutOfRange)); + } + + #[test] fn modify_source_without_subgroups() { // Encoding operation with no subgroups. let op = ModifySourceOperation::new( @@ -1336,7 +1355,7 @@ source_id: 0x01, source_address_type: AddressType::Public, source_address: [0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A], - source_adv_sid: AdvertisingSetId(0x01), + source_adv_sid: AdvertisingSetId::try_from(0x01).unwrap(), broadcast_id: BroadcastId::try_from(0x00010203).unwrap(), pa_sync_state: PaSyncState::Synced, big_encryption: EncryptionStatus::BadCode([ @@ -1370,7 +1389,7 @@ source_id: 0x01, source_address_type: AddressType::Random, source_address: [0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A], - source_adv_sid: AdvertisingSetId(0x01), + source_adv_sid: AdvertisingSetId::try_from(0x01).unwrap(), broadcast_id: BroadcastId::try_from(0x00010203).unwrap(), pa_sync_state: PaSyncState::NotSynced, big_encryption: EncryptionStatus::NotEncrypted,
diff --git a/rust/bt-broadcast-assistant/src/assistant.rs b/rust/bt-broadcast-assistant/src/assistant.rs index b41dcd1..fe6e6a2 100644 --- a/rust/bt-broadcast-assistant/src/assistant.rs +++ b/rust/bt-broadcast-assistant/src/assistant.rs
@@ -291,7 +291,7 @@ fn merge_broadcast_source() { let discovered = DiscoveredBroadcastSources::new(); let bid1 = BroadcastId::try_from(1001).unwrap(); - let key1 = (PeerId(1001), AdvertisingSetId(1)); + let key1 = (PeerId(1001), AdvertisingSetId::try_from(1).unwrap()); // 1. Merge initial source data for SID 1. let (bs, changed) = discovered.merge_broadcast_source_data( @@ -353,7 +353,7 @@ // 4. Merge a new broadcast source with a different SID (SID 2) for the same // peer. let bid2 = BroadcastId::try_from(1002).unwrap(); - let key2 = (PeerId(1001), AdvertisingSetId(2)); + let key2 = (PeerId(1001), AdvertisingSetId::try_from(2).unwrap()); let (bs, changed) = discovered.merge_broadcast_source_data( &key2, &BroadcastSource::default() @@ -381,9 +381,9 @@ // Verify get_by_broadcast_id works for both and maps to correct keys let lock = discovered.0.lock(); let entry1 = lock.iter().find(|(_, v)| v.broadcast_id == Some(bid1)).unwrap(); - assert_eq!(entry1.0 .1, AdvertisingSetId(1)); + assert_eq!(entry1.0 .1, AdvertisingSetId::try_from(1).unwrap()); let entry2 = lock.iter().find(|(_, v)| v.broadcast_id == Some(bid2)).unwrap(); - assert_eq!(entry2.0 .1, AdvertisingSetId(2)); + assert_eq!(entry2.0 .1, AdvertisingSetId::try_from(2).unwrap()); } #[test] @@ -427,7 +427,7 @@ let peer_id = PeerId(1); let address = [1, 2, 3, 4, 5, 6]; let address_type = AddressType::Public; - let sid = AdvertisingSetId(1); + let sid = AdvertisingSetId::try_from(1).unwrap(); let source = assistant.force_discover_broadcast_source(peer_id, address, address_type, sid).unwrap(); @@ -445,7 +445,7 @@ let assistant = BroadcastAssistant::<FakeTypes>::new(FakeCentral::new()); let peer_id = PeerId(1); let metadata = vec![vec![Metadata::BroadcastAudioImmediateRenderingFlag]]; - let sid = AdvertisingSetId(1); + let sid = AdvertisingSetId::try_from(1).unwrap(); let source = assistant .force_discover_broadcast_source_metadata(peer_id, sid, metadata.clone())
diff --git a/rust/bt-broadcast-assistant/src/assistant/event.rs b/rust/bt-broadcast-assistant/src/assistant/event.rs index 1780cf1..656ff93 100644 --- a/rust/bt-broadcast-assistant/src/assistant/event.rs +++ b/rust/bt-broadcast-assistant/src/assistant/event.rs
@@ -203,18 +203,24 @@ return None; }; + let Ok(advertising_sid) = AdvertisingSetId::try_from(sid) else { + return None; + }; + let (broadcast_source, changed) = self.broadcast_sources.merge_broadcast_source_data( - &(peer_id, AdvertisingSetId(sid)), + &(peer_id, advertising_sid), &BroadcastSource::default().with_endpoint(base), ); if broadcast_source.is_ready_to_add() && changed { - if let Some(Some(handle)) = self.active_syncs.remove(&(peer_id, sid)) { + if let Some(Some(handle)) = + self.active_syncs.remove(&(peer_id, advertising_sid.value())) + { handle.abort(); } return Some(Event::FoundBroadcastSource { peer: peer_id, - advertising_sid: AdvertisingSetId(sid), + advertising_sid, source: broadcast_source, }); } @@ -233,7 +239,9 @@ let Some(raw_sid) = scanned.advertising_sid else { return None; }; - let sid = AdvertisingSetId(raw_sid); + let Ok(sid) = AdvertisingSetId::try_from(raw_sid) else { + return None; + }; let (broadcast_source, changed) = self.broadcast_sources.merge_broadcast_source_data(&(scanned.id, sid), &found_source); @@ -261,7 +269,7 @@ // If we are already actively syncing (or establishing a sync) for this // peer/SID, don't start another one. - let key = (scanned.id, sid.0); + let key = (scanned.id, sid.value()); if self.active_syncs.contains_key(&key) { return None; } @@ -269,10 +277,10 @@ self.active_syncs.insert(key, None); let fut = pa.sync_to_advertising_reports( scanned.id, - sid.0, + sid.value(), SyncConfiguration { filter_duplicates: true }, ); - let mapped_fut = fut.map(move |res| (scanned.id, sid.0, res)); + let mapped_fut = fut.map(move |res| (scanned.id, sid.value(), res)); self.establishing_periodic_advertising_syncs.push(Box::pin(mapped_fut)); None @@ -431,7 +439,7 @@ // Pretend somehow address, address type were filled out. let _ = stream.broadcast_sources.merge_broadcast_source_data( - &(broadcast_source_pid, AdvertisingSetId(1)), + &(broadcast_source_pid, AdvertisingSetId::try_from(1).unwrap()), &BroadcastSource::default() .with_address([1, 2, 3, 4, 5, 6]) .with_address_type(AddressType::Public), @@ -480,7 +488,7 @@ }; assert_matches!(event, Event::FoundBroadcastSource { peer, advertising_sid, source } => { assert_eq!(peer, broadcast_source_pid); - assert_eq!(advertising_sid, AdvertisingSetId(1)); + assert_eq!(advertising_sid, AdvertisingSetId::try_from(1).unwrap()); assert_eq!(source.periodic_advertising_interval, Some(PeriodicAdvertisingInterval(0x0100))); assert_eq!(source.address, Some([1, 2, 3, 4, 5, 6])); assert_eq!(source.broadcast_name, Some("Test Broadcast".to_string()));
diff --git a/rust/bt-broadcast-assistant/src/assistant/peer.rs b/rust/bt-broadcast-assistant/src/assistant/peer.rs index 4c8e6e4..2527fb6 100644 --- a/rust/bt-broadcast-assistant/src/assistant/peer.rs +++ b/rust/bt-broadcast-assistant/src/assistant/peer.rs
@@ -287,7 +287,7 @@ { let fut = peer.add_broadcast_source( PeerId(1001), - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), &FakeGetPeerAddr, PaSync::SyncPastUnavailable, HashMap::new(), @@ -298,7 +298,7 @@ } let _ = broadcast_source.merge_broadcast_source_data( - &(PeerId(1001), AdvertisingSetId(1)), + &(PeerId(1001), AdvertisingSetId::try_from(1).unwrap()), &BroadcastSource::default().with_broadcast_id(BroadcastId::try_from(1001).unwrap()), ); @@ -308,7 +308,7 @@ StaticPeerAddr::new_for_peer(PeerId(1002), [1, 2, 3, 4, 5, 6], AddressType::Public); let fut = peer.add_broadcast_source( PeerId(1001), - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), &address_lookup, PaSync::SyncPastUnavailable, HashMap::new(), @@ -324,7 +324,7 @@ StaticPeerAddr::new_for_peer(PeerId(1001), [1, 2, 3, 4, 5, 6], AddressType::Public); let fut = peer.add_broadcast_source( PeerId(1001), - AdvertisingSetId(1), + AdvertisingSetId::try_from(1).unwrap(), &address_lookup, PaSync::SyncPastUnavailable, HashMap::new(),
diff --git a/rust/bt-broadcast-assistant/src/debug.rs b/rust/bt-broadcast-assistant/src/debug.rs index abd545f..b369c4e 100644 --- a/rust/bt-broadcast-assistant/src/debug.rs +++ b/rust/bt-broadcast-assistant/src/debug.rs
@@ -151,6 +151,11 @@ raw_id.try_into().map_err(|e| format!("{e:?}")) } +fn parse_advertising_sid(input: &str) -> Result<AdvertisingSetId, String> { + let raw_sid: u8 = parse_int(input).map_err(|_| format!("failed to parse int from {input}"))?; + AdvertisingSetId::try_from(raw_sid).map_err(|e| format!("{e:?}")) +} + fn parse_bis_sync(input: &str) -> HashMap<SubgroupIndex, BisSync> { let mut map = HashMap::new(); for t in input.split(',') { @@ -297,11 +302,10 @@ return Ok(()); }; - let Ok(sid_val) = parse_int::<u8>(&args[1]) else { + let Ok(advertising_sid) = parse_advertising_sid(&args[1]) else { eprintln!("invalid advertising sid: {}", args[1]); return Ok(()); }; - let advertising_sid = AdvertisingSetId(sid_val); let pa_sync: PaSync = match args[2].parse() { Ok(sync) => sync, @@ -407,11 +411,10 @@ } }; - let Ok(raw_ad_sid) = parse_int::<u8>(&args[3]) else { + let Ok(advertising_sid) = parse_advertising_sid(&args[3]) else { eprintln!("invalid advertising sid: {}", args[3]); return Ok(()); }; - let advertising_sid = AdvertisingSetId(raw_ad_sid); match self.assistant.force_discover_broadcast_source( source_peer_id, @@ -442,11 +445,10 @@ return Ok(()); }; - let Ok(raw_ad_sid) = parse_int::<u8>(&args[1]) else { + let Ok(advertising_sid) = parse_advertising_sid(&args[1]) else { eprintln!("invalid advertising sid: {}", args[1]); return Ok(()); }; - let advertising_sid = AdvertisingSetId(raw_ad_sid); let mut all_big_metadata = Vec::new(); for i in 2..args.len() { @@ -495,11 +497,10 @@ return Ok(()); }; - let Ok(raw_ad_sid) = parse_int::<u8>(&args[1]) else { + let Ok(advertising_sid) = parse_advertising_sid(&args[1]) else { eprintln!("invalid advertising sid: {}", args[1]); return Ok(()); }; - let advertising_sid = AdvertisingSetId(raw_ad_sid); let Ok(num_big) = parse_int::<usize>(&args[2]) else { eprintln!("invalid # of bigs: {}", args[2]);
diff --git a/rust/bt-common/src/core.rs b/rust/bt-common/src/core.rs index d9050d5..8076264 100644 --- a/rust/bt-common/src/core.rs +++ b/rust/bt-common/src/core.rs
@@ -49,13 +49,40 @@ } } -/// Advertising Set ID which is 1 byte long. +/// Advertising Set ID (SID) which is 4 bits long (range 0x00 to 0x0F). +/// See Bluetooth Core Specification Vol 6, Part B, Section 2.3.4 and BASS +/// v1.0.1 Section 3.1.1.4 Table 3.5. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct AdvertisingSetId(pub u8); +pub struct AdvertisingSetId(u8); impl AdvertisingSetId { // Byte size if this is to be encoded. pub const BYTE_SIZE: usize = 1; + + /// Maximum valid ID. See BASS v1.0.1 Section 3.1.1.4 Table 3.5. + pub const MAX_VALUE: u8 = 0x0F; + + pub fn value(&self) -> u8 { + self.0 + } +} + +impl TryFrom<u8> for AdvertisingSetId { + type Error = PacketError; + + fn try_from(value: u8) -> Result<Self, Self::Error> { + if value > Self::MAX_VALUE { + Err(PacketError::OutOfRange) + } else { + Ok(Self(value)) + } + } +} + +impl From<AdvertisingSetId> for u8 { + fn from(sid: AdvertisingSetId) -> u8 { + sid.0 + } } /// SyncInfo Interval value which is 2 bytes long. @@ -292,4 +319,16 @@ }) ); } + + #[test] + fn advertising_set_id_success() { + assert_eq!(AdvertisingSetId::try_from(0x00).unwrap().value(), 0x00); + assert_eq!(AdvertisingSetId::try_from(0x0F).unwrap().value(), 0x0F); + } + + #[test] + fn advertising_set_id_out_of_range() { + assert_eq!(AdvertisingSetId::try_from(0x10), Err(PacketError::OutOfRange)); + assert_eq!(AdvertisingSetId::try_from(0xFF), Err(PacketError::OutOfRange)); + } }