rust/bt-csis: Encapsulate characteristic value types Make the fields private so their representation is not part of the public API. Ensure that the SIRK is not printed. Bug: 534436497 Test: cargo test Change-Id: Id63f28d50a6f38a6cec8446f2a2185f056e6a65f Reviewed-on: https://bluetooth-review.googlesource.com/c/bluetooth/+/4160
diff --git a/rust/bt-csis/src/client.rs b/rust/bt-csis/src/client.rs index 125d6e2..9c5adeb 100644 --- a/rust/bt-csis/src/client.rs +++ b/rust/bt-csis/src/client.rs
@@ -171,7 +171,7 @@ } if let (Some(r), Some(s)) = (rank, size) { - if r.0.get() > s.0.get() { + if r.value().get() > s.value().get() { return Err(Error::InvalidCharacteristic( "SetMemberRank cannot be greater than CoordinatedSetSize".to_string(), )); @@ -357,9 +357,9 @@ assert_eq!(client.lock_handle, Some(LOCK_HANDLE)); assert_eq!(client._rank_handle, Some(RANK_HANDLE)); - assert_eq!(client.sirk().sirk_type, SirkType::Plaintext); - assert_eq!(client.size(), Some(CoordinatedSetSize(NonZeroU8::new(2).unwrap()))); - assert_eq!(client.rank(), Some(SetMemberRank(NonZeroU8::new(1).unwrap()))); + assert_eq!(client.sirk().sirk_type(), SirkType::Plaintext); + assert_eq!(client.size(), Some(CoordinatedSetSize::new(NonZeroU8::new(2).unwrap()))); + assert_eq!(client.rank(), Some(SetMemberRank::new(NonZeroU8::new(1).unwrap()))); assert_eq!(client.lock_state(), Some(SetMemberLock::Unlocked)); } @@ -407,7 +407,7 @@ assert_eq!(client.size_handle, None); assert_eq!(client.lock_handle, None); assert_eq!(client._rank_handle, Some(RANK_HANDLE)); - assert_eq!(client.rank().unwrap().0.get(), 1); + assert_eq!(client.rank().unwrap().value().get(), 1); } #[test]
diff --git a/rust/bt-csis/src/client/event.rs b/rust/bt-csis/src/client/event.rs index 0f78dcf..99095bf 100644 --- a/rust/bt-csis/src/client/event.rs +++ b/rust/bt-csis/src/client/event.rs
@@ -214,7 +214,7 @@ maybe_truncated: false, }), ); - assert_eq!(client.size(), Some(CoordinatedSetSize(NonZeroU8::new(2).unwrap()))); + assert_eq!(client.size(), Some(CoordinatedSetSize::new(NonZeroU8::new(2).unwrap()))); let mut stream = client.take_notification_stream().expect("stream available"); service.notify( @@ -230,7 +230,7 @@ }; assert_eq!( event, - CsisNotification::SizeChanged(CoordinatedSetSize(NonZeroU8::new(3).unwrap())) + CsisNotification::SizeChanged(CoordinatedSetSize::new(NonZeroU8::new(3).unwrap())) ); } @@ -242,8 +242,8 @@ let mut stream = client.take_notification_stream().expect("stream available"); // Values read during discovery are visible before any notification. - assert_eq!(client.sirk().value, [0xAA; 16]); - assert_eq!(client.size(), Some(CoordinatedSetSize(NonZeroU8::new(2).unwrap()))); + assert_eq!(client.sirk().value(), &[0xAA; 16]); + assert_eq!(client.size(), Some(CoordinatedSetSize::new(NonZeroU8::new(2).unwrap()))); assert_eq!(client.lock_state(), Some(SetMemberLock::Unlocked)); assert_matches!(stream.poll_next_unpin(&mut noop_cx), Poll::Pending); @@ -277,9 +277,9 @@ }; assert_eq!( event, - CsisNotification::SizeChanged(CoordinatedSetSize(NonZeroU8::new(3).unwrap())) + CsisNotification::SizeChanged(CoordinatedSetSize::new(NonZeroU8::new(3).unwrap())) ); - assert_eq!(client.size(), Some(CoordinatedSetSize(NonZeroU8::new(3).unwrap()))); + assert_eq!(client.size(), Some(CoordinatedSetSize::new(NonZeroU8::new(3).unwrap()))); service.notify( &SIRK_HANDLE, @@ -294,12 +294,12 @@ }; assert_eq!( event, - CsisNotification::SirkChanged(SetIdentityResolvingKey { - sirk_type: SirkType::Plaintext, - value: [0xBB; 16], - }) + CsisNotification::SirkChanged(SetIdentityResolvingKey::new( + SirkType::Plaintext, + [0xBB; 16] + )) ); - assert_eq!(client.sirk().value, [0xBB; 16]); + assert_eq!(client.sirk().value(), &[0xBB; 16]); } #[test] @@ -335,7 +335,7 @@ stream.poll_next_unpin(&mut noop_cx), Poll::Ready(Some(Err(Error::UnexpectedNotification(RANK_HANDLE)))) ); - assert_eq!(client.rank(), Some(SetMemberRank(NonZeroU8::new(1).unwrap()))); + assert_eq!(client.rank(), Some(SetMemberRank::new(NonZeroU8::new(1).unwrap()))); // The SIRK characteristic is still subscribed. service.notify(
diff --git a/rust/bt-csis/src/types.rs b/rust/bt-csis/src/types.rs index d91b730..04ef0d0 100644 --- a/rust/bt-csis/src/types.rs +++ b/rust/bt-csis/src/types.rs
@@ -31,14 +31,42 @@ /// The Set Identity Resolving Key (SIRK) characteristic exposes the key /// associated with the Coordinated Set (1 octet Type + 16 octets Value). /// See CSIS v1.1 Section 5.1. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Hash)] pub struct SetIdentityResolvingKey { - pub sirk_type: SirkType, - pub value: [u8; 16], + sirk_type: SirkType, + value: [u8; 16], } impl SetIdentityResolvingKey { pub const BYTE_SIZE: usize = 17; + + pub fn new(sirk_type: SirkType, value: [u8; 16]) -> Self { + Self { sirk_type, value } + } + + pub fn sirk_type(&self) -> SirkType { + self.sirk_type + } + + pub fn value(&self) -> &[u8; 16] { + &self.value + } +} + +/// Omits the key. +impl std::fmt::Debug for SetIdentityResolvingKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SetIdentityResolvingKey") + .field("sirk_type", &self.sirk_type) + .finish_non_exhaustive() + } +} + +/// Omits the key. +impl std::fmt::Display for SetIdentityResolvingKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?} SIRK", self.sirk_type) + } } impl Decodable for SetIdentityResolvingKey { @@ -79,10 +107,18 @@ /// The Set Member Rank characteristic exposes a numeric value that is unique /// within a Coordinated Set (0x01 to set size). See CSIS v1.1 Section 5.4. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct SetMemberRank(pub NonZeroU8); +pub struct SetMemberRank(NonZeroU8); impl SetMemberRank { pub const BYTE_SIZE: usize = 1; + + pub fn new(rank: NonZeroU8) -> Self { + Self(rank) + } + + pub fn value(&self) -> NonZeroU8 { + self.0 + } } impl Decodable for SetMemberRank { @@ -124,10 +160,18 @@ /// The Coordinated Set Size characteristic exposes the number of devices /// comprising the Coordinated Set (0x01 to 0xFF). See CSIS v1.1 Section 5.2. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct CoordinatedSetSize(pub NonZeroU8); +pub struct CoordinatedSetSize(NonZeroU8); impl CoordinatedSetSize { pub const BYTE_SIZE: usize = 1; + + pub fn new(size: NonZeroU8) -> Self { + Self(size) + } + + pub fn value(&self) -> NonZeroU8 { + self.0 + } } impl Decodable for CoordinatedSetSize { @@ -226,13 +270,13 @@ let (res, consumed) = SetIdentityResolvingKey::decode(&buf); assert_eq!(consumed, 17); let sirk = res.unwrap(); - assert_eq!(sirk.sirk_type, SirkType::Plaintext); - assert_eq!(sirk.value, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); + assert_eq!(sirk.sirk_type(), SirkType::Plaintext); + assert_eq!(sirk.value(), &[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); } #[test] fn sirk_encoding() { - let sirk = SetIdentityResolvingKey { sirk_type: SirkType::Encrypted, value: [1; 16] }; + let sirk = SetIdentityResolvingKey::new(SirkType::Encrypted, [1; 16]); let mut buf = [0; 17]; sirk.encode(&mut buf).unwrap(); assert_eq!(buf[0], 0x00); @@ -245,7 +289,7 @@ buf[0] = 0x05; let (res, consumed) = SetMemberRank::decode(&buf); assert_eq!(consumed, 1); - assert_eq!(res.unwrap().0.get(), 5); + assert_eq!(res.unwrap().value().get(), 5); buf[0] = 0x00; // Invalid let (res, _) = SetMemberRank::decode(&buf); @@ -254,7 +298,7 @@ #[test] fn set_member_rank_encoding() { - let rank = SetMemberRank(NonZeroU8::new(5).unwrap()); + let rank = SetMemberRank::new(NonZeroU8::new(5).unwrap()); assert_eq!(rank.encoded_len(), 1); let mut buf = [0; 1]; @@ -271,7 +315,7 @@ buf[0] = 0x02; let (res, consumed) = CoordinatedSetSize::decode(&buf); assert_eq!(consumed, 1); - assert_eq!(res.unwrap().0.get(), 2); + assert_eq!(res.unwrap().value().get(), 2); buf[0] = 0x00; // Invalid let (res, _) = CoordinatedSetSize::decode(&buf); @@ -280,7 +324,7 @@ #[test] fn coordinated_set_size_encoding() { - let size = CoordinatedSetSize(NonZeroU8::new(2).unwrap()); + let size = CoordinatedSetSize::new(NonZeroU8::new(2).unwrap()); assert_eq!(size.encoded_len(), 1); let mut buf = [0; 1];
diff --git a/rust/bt-set-coordinator/src/types.rs b/rust/bt-set-coordinator/src/types.rs index 513898a..ef479d9 100644 --- a/rust/bt-set-coordinator/src/types.rs +++ b/rust/bt-set-coordinator/src/types.rs
@@ -36,7 +36,7 @@ /// See CSIP v1.1 Section 4.8 & Core Spec Vol 3 Part H Section 1.3. /// An encrypted SIRK cannot resolve RSIs until decrypted. pub fn resolves_rsi(&self, rsi: &[u8; 6]) -> bool { - if self.sirk.sirk_type == SirkType::Encrypted { + if self.sirk.sirk_type() == SirkType::Encrypted { return false; } @@ -50,7 +50,7 @@ prand.copy_from_slice(&rsi[3..6]); let hash_expected = &rsi[0..3]; - let hash_computed = set_identity_hash(&self.sirk.value, &prand); + let hash_computed = set_identity_hash(self.sirk.value(), &prand); hash_computed == *hash_expected } @@ -121,14 +121,13 @@ #[test] fn resolves_rsi() { - let sirk = SetIdentityResolvingKey { sirk_type: SirkType::Plaintext, value: SAMPLE_KEY }; + let sirk = SetIdentityResolvingKey::new(SirkType::Plaintext, SAMPLE_KEY); let set = CoordinatedSet::new(sirk); assert!(set.resolves_rsi(&SAMPLE_RSI)); // Encrypted SIRK cannot resolve RSI - let encrypted_sirk = - SetIdentityResolvingKey { sirk_type: SirkType::Encrypted, value: SAMPLE_KEY }; + let encrypted_sirk = SetIdentityResolvingKey::new(SirkType::Encrypted, SAMPLE_KEY); let encrypted_set = CoordinatedSet::new(encrypted_sirk); assert!(!encrypted_set.resolves_rsi(&SAMPLE_RSI)); @@ -140,7 +139,7 @@ #[test] fn matches_scan_result_rsi() { - let sirk = SetIdentityResolvingKey { sirk_type: SirkType::Plaintext, value: SAMPLE_KEY }; + let sirk = SetIdentityResolvingKey::new(SirkType::Plaintext, SAMPLE_KEY); let set = CoordinatedSet::new(sirk); // Matching standalone RSI AD Type (0x2E)