From 06f883c6b2467c6a7de7ac4b8c12eb488262216d Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Fri, 17 May 2024 12:10:17 +0300 Subject: [PATCH 01/20] Log failed UUID parse --- libsignal-service/src/envelope.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libsignal-service/src/envelope.rs b/libsignal-service/src/envelope.rs index a68aa6edd..1c853715e 100644 --- a/libsignal-service/src/envelope.rs +++ b/libsignal-service/src/envelope.rs @@ -140,7 +140,12 @@ impl Envelope { .as_deref() .map(Uuid::parse_str) .transpose() - .expect("valid uuid checked in constructor") + .unwrap_or_else(|e| { + panic!( + "valid uuid checked in constructor: {:?} -- {:?}", + self.source_service_id, e + ) + }) .expect("source_service_id is set"); ServiceAddress { uuid } From 2061a62e27000961e62a68e54cda1af816c10f62 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 18 May 2024 09:58:43 +0300 Subject: [PATCH 02/20] Add ServiceIdType to ServiceAddress --- libsignal-service/src/cipher.rs | 8 ++-- libsignal-service/src/envelope.rs | 19 ++------- libsignal-service/src/push_service.rs | 2 +- libsignal-service/src/service_address.rs | 50 +++++++++++++++++++----- 4 files changed, 49 insertions(+), 30 deletions(-) diff --git a/libsignal-service/src/cipher.rs b/libsignal-service/src/cipher.rs index 063b4207b..f996bf26e 100644 --- a/libsignal-service/src/cipher.rs +++ b/libsignal-service/src/cipher.rs @@ -278,13 +278,13 @@ where ) .await?; - let sender = ServiceAddress { - uuid: Uuid::parse_str(&sender_uuid).map_err(|_| { + let sender = ServiceAddress::try_from(sender_uuid.as_str()) + .map_err(|e| { + tracing::error!("{:?}", e); SignalProtocolError::InvalidSealedSenderMessage( "invalid sender UUID".to_string(), ) - })?, - }; + })?; let needs_receipt = if envelope.source_service_id.is_some() { tracing::warn!(?envelope, "Received an unidentified delivery over an identified channel. Marking needs_receipt=false"); diff --git a/libsignal-service/src/envelope.rs b/libsignal-service/src/envelope.rs index 1c853715e..64d9c575d 100644 --- a/libsignal-service/src/envelope.rs +++ b/libsignal-service/src/envelope.rs @@ -3,7 +3,6 @@ use std::convert::{TryFrom, TryInto}; use aes::cipher::block_padding::Pkcs7; use aes::cipher::{BlockDecryptMut, KeyIvInit}; use prost::Message; -use uuid::Uuid; use crate::{ configuration::SignalingKey, push_service::ServiceError, @@ -135,20 +134,10 @@ impl Envelope { } pub fn source_address(&self) -> ServiceAddress { - let uuid = self - .source_service_id - .as_deref() - .map(Uuid::parse_str) - .transpose() - .unwrap_or_else(|e| { - panic!( - "valid uuid checked in constructor: {:?} -- {:?}", - self.source_service_id, e - ) - }) - .expect("source_service_id is set"); - - ServiceAddress { uuid } + match self.source_service_id.as_deref() { + Some(uuid) => ServiceAddress::try_from(uuid).unwrap(), + None => panic!("source_service_id is set"), + } } } diff --git a/libsignal-service/src/push_service.rs b/libsignal-service/src/push_service.rs index 845ef9b9d..5c626a1a9 100644 --- a/libsignal-service/src/push_service.rs +++ b/libsignal-service/src/push_service.rs @@ -75,7 +75,7 @@ pub const STICKER_PATH: &str = "stickers/%s/full/%d"; pub const KEEPALIVE_TIMEOUT_SECONDS: Duration = Duration::from_secs(55); pub const DEFAULT_DEVICE_ID: u32 = 1; -#[derive(Debug, Clone, Copy, Eq, PartialEq)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)] pub enum ServiceIdType { /// Account Identity (ACI) /// diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 423ff3737..7fbc41822 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -4,6 +4,8 @@ use libsignal_protocol::{DeviceId, ProtocolAddress}; use serde::{Deserialize, Serialize}; use uuid::Uuid; +pub use crate::push_service::ServiceIdType; + #[derive(thiserror::Error, Debug, Clone)] pub enum ParseServiceAddressError { #[error("Supplied UUID could not be parsed")] @@ -16,6 +18,7 @@ pub enum ParseServiceAddressError { #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct ServiceAddress { pub uuid: Uuid, + pub identity: ServiceIdType, } impl ServiceAddress { @@ -37,7 +40,10 @@ impl ServiceAddress { impl From for ServiceAddress { fn from(uuid: Uuid) -> Self { - Self { uuid } + Self { + uuid, + identity: ServiceIdType::AccountIdentity, + } } } @@ -45,9 +51,35 @@ impl TryFrom<&str> for ServiceAddress { type Error = ParseServiceAddressError; fn try_from(value: &str) -> Result { - Ok(ServiceAddress { - uuid: Uuid::parse_str(value)?, - }) + if value.starts_with("PNI:") { + Ok(ServiceAddress { + uuid: Uuid::parse_str(value.strip_prefix("PNI:").unwrap())?, + identity: ServiceIdType::PhoneNumberIdentity, + }) + } else { + Ok(ServiceAddress { + uuid: Uuid::parse_str(value)?, + identity: ServiceIdType::AccountIdentity, + }) + } + } +} + +impl TryFrom<&[u8]> for ServiceAddress { + type Error = ParseServiceAddressError; + + fn try_from(value: &[u8]) -> Result { + if value.starts_with(b"PNI:") { + Ok(ServiceAddress { + uuid: Uuid::from_slice(value.strip_prefix(b"PNI:").unwrap())?, + identity: ServiceIdType::PhoneNumberIdentity, + }) + } else { + Ok(ServiceAddress { + uuid: Uuid::from_slice(value)?, + identity: ServiceIdType::AccountIdentity, + }) + } } } @@ -55,9 +87,8 @@ impl TryFrom> for ServiceAddress { type Error = ParseServiceAddressError; fn try_from(value: Option<&str>) -> Result { - match value.map(Uuid::parse_str) { - Some(Ok(uuid)) => Ok(ServiceAddress { uuid }), - Some(Err(e)) => Err(ParseServiceAddressError::InvalidUuid(e)), + match value { + Some(uuid) => ServiceAddress::try_from(uuid), None => Err(ParseServiceAddressError::NoUuid), } } @@ -67,9 +98,8 @@ impl TryFrom> for ServiceAddress { type Error = ParseServiceAddressError; fn try_from(value: Option<&[u8]>) -> Result { - match value.map(Uuid::from_slice) { - Some(Ok(uuid)) => Ok(ServiceAddress { uuid }), - Some(Err(e)) => Err(ParseServiceAddressError::InvalidUuid(e)), + match value { + Some(uuid) => ServiceAddress::try_from(uuid), None => Err(ParseServiceAddressError::NoUuid), } } From f9252384884e5e3657df4ea02a90ce5a27aea754 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 25 May 2024 15:19:27 +0300 Subject: [PATCH 03/20] MessageSenderError::NotFound: Return ServiceAddress instead of UUID --- libsignal-service/src/sender.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libsignal-service/src/sender.rs b/libsignal-service/src/sender.rs index 8ebbdf43d..f8dfb4211 100644 --- a/libsignal-service/src/sender.rs +++ b/libsignal-service/src/sender.rs @@ -8,7 +8,6 @@ use libsignal_protocol::{ use rand::{CryptoRng, Rng}; use tracing::{info, trace}; use tracing_futures::Instrument; -use uuid::Uuid; use crate::{ cipher::{get_preferred_protocol_address, ServiceCipher}, @@ -119,8 +118,8 @@ pub enum MessageSenderError { #[error("Proof of type {options:?} required using token {token}")] ProofRequired { token: String, options: Vec }, - #[error("Recipient not found: {uuid}")] - NotFound { uuid: Uuid }, + #[error("Recipient not found: {addr:?}")] + NotFound { addr: ServiceAddress }, } impl MessageSender @@ -599,7 +598,7 @@ where Err(ServiceError::NotFoundError) => { tracing::debug!("Not found when sending a message"); return Err(MessageSenderError::NotFound { - uuid: recipient.uuid, + addr: recipient, }); }, Err(e) => { @@ -806,7 +805,7 @@ where }, Err(ServiceError::NotFoundError) => { return Err(MessageSenderError::NotFound { - uuid: recipient.uuid, + addr: *recipient, }); }, Err(e) => Err(e)?, From 9aefbf2ba1c0b50352f694a96d93b0a6cc524b12 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 25 May 2024 15:19:47 +0300 Subject: [PATCH 04/20] Clippy --- libsignal-service/examples/storage.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libsignal-service/examples/storage.rs b/libsignal-service/examples/storage.rs index 9f9998b57..6a0b1708e 100644 --- a/libsignal-service/examples/storage.rs +++ b/libsignal-service/examples/storage.rs @@ -9,6 +9,12 @@ use libsignal_service::protocol::{ #[derive(Default)] pub struct ExampleStore {} +impl Default for ExampleStore { + fn default() -> Self { + Self::new() + } +} + impl ExampleStore { pub fn new() -> Self { Self::default() From a0591867bce9a5c6f39f8aaf10daa3d5e57b1af9 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 25 May 2024 15:36:21 +0300 Subject: [PATCH 05/20] Use fewer unwraps Co-authored-by: Ruben De Smet --- libsignal-service/src/service_address.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 7fbc41822..673cc9096 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -69,9 +69,9 @@ impl TryFrom<&[u8]> for ServiceAddress { type Error = ParseServiceAddressError; fn try_from(value: &[u8]) -> Result { - if value.starts_with(b"PNI:") { + if let Some(pni) = value.strip_prefix(b"PNI:") { Ok(ServiceAddress { - uuid: Uuid::from_slice(value.strip_prefix(b"PNI:").unwrap())?, + uuid: Uuid::from_slice(pni)?, identity: ServiceIdType::PhoneNumberIdentity, }) } else { From c13ed1e30a0444606e11324f4a947c0797000924 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 25 May 2024 17:26:47 +0300 Subject: [PATCH 06/20] Implement TryFrom<&ProtocolAddress> for ServiceAddress --- libsignal-service/src/service_address.rs | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 673cc9096..43f5c0c88 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -47,6 +47,31 @@ impl From for ServiceAddress { } } +impl TryFrom<&ProtocolAddress> for ServiceAddress { + type Error = ParseServiceAddressError; + + fn try_from(addr: &ProtocolAddress) -> Result { + let value = addr.name(); + if value.starts_with("PNI:") { + Ok(ServiceAddress { + uuid: Uuid::parse_str(value.strip_prefix("PNI:").unwrap())?, + identity: ServiceIdType::PhoneNumberIdentity, + }) + } else { + match Uuid::parse_str(value) { + Ok(uuid) => Ok(ServiceAddress { + uuid, + identity: ServiceIdType::AccountIdentity, + }), + Err(e) => { + tracing::error!("Unknown identity format: {}", value); + Err(ParseServiceAddressError::InvalidUuid(e)) + }, + } + } + } +} + impl TryFrom<&str> for ServiceAddress { type Error = ParseServiceAddressError; From b7a0706fee327829e9e97226fb94e7143f70a0cd Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 25 May 2024 18:34:06 +0300 Subject: [PATCH 07/20] Remove unwrap() --- libsignal-service/src/service_address.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 43f5c0c88..ed2b0455b 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -52,9 +52,9 @@ impl TryFrom<&ProtocolAddress> for ServiceAddress { fn try_from(addr: &ProtocolAddress) -> Result { let value = addr.name(); - if value.starts_with("PNI:") { + if let Some(pni) = value.strip_prefix("PNI:") { Ok(ServiceAddress { - uuid: Uuid::parse_str(value.strip_prefix("PNI:").unwrap())?, + uuid: Uuid::parse_str(pni)?, identity: ServiceIdType::PhoneNumberIdentity, }) } else { From ecc1f98f9b7e0ce766a2d391851cf09f57d2dc3c Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sun, 26 May 2024 12:18:04 +0300 Subject: [PATCH 08/20] Add ServiceAddress to_service_id() --- libsignal-service/src/service_address.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index ed2b0455b..d9ffc7206 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -36,6 +36,15 @@ impl ServiceAddress { pub fn pni(&self) -> libsignal_protocol::Pni { libsignal_protocol::Pni::from_uuid_bytes(self.uuid.into_bytes()) } + + pub fn to_service_id(&self) -> String { + match self.identity { + ServiceIdType::AccountIdentity => self.uuid.to_string(), + ServiceIdType::PhoneNumberIdentity => { + format!("PNI:{}", self.uuid) + }, + } + } } impl From for ServiceAddress { From b4e5a7149e96d4086844ba2a9b920d8c406a5b10 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Thu, 30 May 2024 12:51:01 +0300 Subject: [PATCH 09/20] impl Hash for ServiceAddress --- libsignal-service/src/service_address.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index d9ffc7206..30b94ccaa 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -1,4 +1,4 @@ -use std::convert::TryFrom; +use std::{convert::TryFrom, hash::Hash, hash::Hasher}; use libsignal_protocol::{DeviceId, ProtocolAddress}; use serde::{Deserialize, Serialize}; @@ -138,3 +138,9 @@ impl TryFrom> for ServiceAddress { } } } + +impl Hash for ServiceAddress { + fn hash(&self, state: &mut H) { + self.to_service_id().hash(state); + } +} From 78bf513b2bea4f207b8930cc225abf89715b58fd Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Mon, 24 Jun 2024 06:12:45 +0300 Subject: [PATCH 10/20] Don't assume ACI in Envelope debug print --- libsignal-service/src/cipher.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/libsignal-service/src/cipher.rs b/libsignal-service/src/cipher.rs index f996bf26e..ea79eee73 100644 --- a/libsignal-service/src/cipher.rs +++ b/libsignal-service/src/cipher.rs @@ -55,17 +55,13 @@ fn debug_envelope(envelope: &Envelope) -> String { } else { format!( "Envelope {{ \ - source_address: {}, \ + source_address: {:?}, \ source_device: {:?}, \ server_guid: {:?}, \ timestamp: {:?}, \ content: {} bytes, \ }}", - if envelope.source_service_id.is_some() { - format!("{:?}", envelope.source_address()) - } else { - "unknown".to_string() - }, + envelope.source_service_id, envelope.source_device(), envelope.server_guid(), envelope.timestamp(), From 8ab8eb15f1a8e871c3949b05b3dee3631d029da8 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 13:09:08 +0300 Subject: [PATCH 11/20] Remove conflicting Default implementation Co-authored-by: Ruben De Smet --- libsignal-service/examples/storage.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libsignal-service/examples/storage.rs b/libsignal-service/examples/storage.rs index 6a0b1708e..9f9998b57 100644 --- a/libsignal-service/examples/storage.rs +++ b/libsignal-service/examples/storage.rs @@ -9,12 +9,6 @@ use libsignal_service::protocol::{ #[derive(Default)] pub struct ExampleStore {} -impl Default for ExampleStore { - fn default() -> Self { - Self::new() - } -} - impl ExampleStore { pub fn new() -> Self { Self::default() From eee22cbb38b5c00870ff833fc579627542fa613f Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 14:01:12 +0300 Subject: [PATCH 12/20] Return Option, fail cross-conversion Co-authored-by: Ruben De Smet --- libsignal-service/src/profile_service.rs | 2 +- libsignal-service/src/push_service.rs | 2 +- libsignal-service/src/service_address.rs | 20 ++++++++++++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/libsignal-service/src/profile_service.rs b/libsignal-service/src/profile_service.rs index ae17e0fc7..10061f84f 100644 --- a/libsignal-service/src/profile_service.rs +++ b/libsignal-service/src/profile_service.rs @@ -22,7 +22,7 @@ impl ProfileService { let endpoint = match profile_key { Some(key) => { let version = bincode::serialize( - &key.get_profile_key_version(address.aci()), + &key.get_profile_key_version(address.aci().unwrap()), )?; let version = std::str::from_utf8(&version) .expect("hex encoded profile key version"); diff --git a/libsignal-service/src/push_service.rs b/libsignal-service/src/push_service.rs index 5c626a1a9..8b0d169b1 100644 --- a/libsignal-service/src/push_service.rs +++ b/libsignal-service/src/push_service.rs @@ -903,7 +903,7 @@ pub trait PushService: MaybeSend { ) -> Result { let endpoint = if let Some(key) = profile_key { let version = bincode::serialize( - &key.get_profile_key_version(address.aci()), + &key.get_profile_key_version(address.aci().unwrap()), )?; let version = std::str::from_utf8(&version) .expect("hex encoded profile key version"); diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 30b94ccaa..cb777aaee 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -29,12 +29,24 @@ impl ServiceAddress { ProtocolAddress::new(self.uuid.to_string(), device_id.into()) } - pub fn aci(&self) -> libsignal_protocol::Aci { - libsignal_protocol::Aci::from_uuid_bytes(self.uuid.into_bytes()) + pub fn aci(&self) -> Option { + use libsignal_protocol::Aci; + match self.identity { + ServiceIdType::AccountIdentity => { + Some(Aci::from_uuid_bytes(self.uuid.into_bytes())) + }, + ServiceIdType::PhoneNumberIdentity => None, + } } - pub fn pni(&self) -> libsignal_protocol::Pni { - libsignal_protocol::Pni::from_uuid_bytes(self.uuid.into_bytes()) + pub fn pni(&self) -> Option { + use libsignal_protocol::Pni; + match self.identity { + ServiceIdType::AccountIdentity => None, + ServiceIdType::PhoneNumberIdentity => { + Some(Pni::from_uuid_bytes(self.uuid.into_bytes())) + }, + } } pub fn to_service_id(&self) -> String { From 0379807c5f428bb1cf20225a98da8f54424633b8 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 14:11:35 +0300 Subject: [PATCH 13/20] Match for both Aci or Pni self-recipient Co-authored-by: Ruben De Smet --- libsignal-service/src/sender.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/libsignal-service/src/sender.rs b/libsignal-service/src/sender.rs index f8dfb4211..c7e549f10 100644 --- a/libsignal-service/src/sender.rs +++ b/libsignal-service/src/sender.rs @@ -699,9 +699,22 @@ where devices.insert(DEFAULT_DEVICE_ID.into()); // never try to send messages to the sender device - if recipient.aci() == self.local_aci.aci() { - devices.remove(&self.device_id); - } + match recipient.identity { + ServiceIdType::AccountIdentity => { + if recipient.aci().is_some() + && recipient.aci() == self.local_aci.aci() + { + devices.remove(&self.device_id); + } + }, + ServiceIdType::PhoneNumberIdentity => { + if recipient.pni().is_some() + && recipient.pni() == self.local_aci.pni() + { + devices.remove(&self.device_id); + } + }, + }; for device_id in devices { trace!("sending message to device {}", device_id); From 729734c4f526f2c52964ce47dc0fb44c01622360 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 14:13:12 +0300 Subject: [PATCH 14/20] Don't accidentally convert PNI ServiceAddress to ACI ProtocolAddress Co-authored-by: Ruben De Smet --- libsignal-service/src/service_address.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index cb777aaee..a2e01a49b 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -26,7 +26,15 @@ impl ServiceAddress { &self, device_id: impl Into, ) -> ProtocolAddress { - ProtocolAddress::new(self.uuid.to_string(), device_id.into()) + match self.identity { + ServiceIdType::AccountIdentity => { + ProtocolAddress::new(self.uuid.to_string(), device_id.into()) + }, + ServiceIdType::PhoneNumberIdentity => ProtocolAddress::new( + format!("PNI:{}", self.uuid), + device_id.into(), + ), + } } pub fn aci(&self) -> Option { From d912e480a8b25e3cc4a592ff0a3df3c670d7b0ea Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 14:16:05 +0300 Subject: [PATCH 15/20] Remove unused code Co-authored-by: Ruben De Smet --- libsignal-service/src/service_address.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index a2e01a49b..94213f7d4 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -137,28 +137,6 @@ impl TryFrom<&[u8]> for ServiceAddress { } } -impl TryFrom> for ServiceAddress { - type Error = ParseServiceAddressError; - - fn try_from(value: Option<&str>) -> Result { - match value { - Some(uuid) => ServiceAddress::try_from(uuid), - None => Err(ParseServiceAddressError::NoUuid), - } - } -} - -impl TryFrom> for ServiceAddress { - type Error = ParseServiceAddressError; - - fn try_from(value: Option<&[u8]>) -> Result { - match value { - Some(uuid) => ServiceAddress::try_from(uuid), - None => Err(ParseServiceAddressError::NoUuid), - } - } -} - impl Hash for ServiceAddress { fn hash(&self, state: &mut H) { self.to_service_id().hash(state); From 187d86b04a44fed5c1cd8b51a86ef9f275eb659f Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Tue, 25 Jun 2024 14:19:43 +0300 Subject: [PATCH 16/20] Replace manual Hash implementation with derive Co-authored-by: Ruben De Smet --- libsignal-service/src/push_service.rs | 2 +- libsignal-service/src/service_address.rs | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/libsignal-service/src/push_service.rs b/libsignal-service/src/push_service.rs index 8b0d169b1..9cd10d12a 100644 --- a/libsignal-service/src/push_service.rs +++ b/libsignal-service/src/push_service.rs @@ -75,7 +75,7 @@ pub const STICKER_PATH: &str = "stickers/%s/full/%d"; pub const KEEPALIVE_TIMEOUT_SECONDS: Duration = Duration::from_secs(55); pub const DEFAULT_DEVICE_ID: u32 = 1; -#[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize, Hash)] pub enum ServiceIdType { /// Account Identity (ACI) /// diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 94213f7d4..caa22364d 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -1,4 +1,4 @@ -use std::{convert::TryFrom, hash::Hash, hash::Hasher}; +use std::convert::TryFrom; use libsignal_protocol::{DeviceId, ProtocolAddress}; use serde::{Deserialize, Serialize}; @@ -15,7 +15,7 @@ pub enum ParseServiceAddressError { NoUuid, } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize, Hash)] pub struct ServiceAddress { pub uuid: Uuid, pub identity: ServiceIdType, @@ -136,9 +136,3 @@ impl TryFrom<&[u8]> for ServiceAddress { } } } - -impl Hash for ServiceAddress { - fn hash(&self, state: &mut H) { - self.to_service_id().hash(state); - } -} From 1a5f25403bc0650e4b053e39aab4a86551c69b58 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 29 Jun 2024 15:09:19 +0300 Subject: [PATCH 17/20] Use `recipient` field with `destination` Co-authored-by: Ruben De Smet --- libsignal-service/src/push_service.rs | 4 ++-- libsignal-service/src/sender.rs | 4 ++-- libsignal-service/src/service_address.rs | 3 +-- libsignal-service/src/websocket/sender.rs | 4 ++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/libsignal-service/src/push_service.rs b/libsignal-service/src/push_service.rs index 9cd10d12a..a2383c843 100644 --- a/libsignal-service/src/push_service.rs +++ b/libsignal-service/src/push_service.rs @@ -75,7 +75,7 @@ pub const STICKER_PATH: &str = "stickers/%s/full/%d"; pub const KEEPALIVE_TIMEOUT_SECONDS: Duration = Duration::from_secs(55); pub const DEFAULT_DEVICE_ID: u32 = 1; -#[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize, Hash)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub enum ServiceIdType { /// Account Identity (ACI) /// @@ -811,7 +811,7 @@ pub trait PushService: MaybeSend { &mut self, messages: OutgoingPushMessages, ) -> Result { - let path = format!("/v1/messages/{}", messages.recipient.uuid); + let path = format!("/v1/messages/{}", messages.destination); self.put_json( Endpoint::Service, &path, diff --git a/libsignal-service/src/sender.rs b/libsignal-service/src/sender.rs index 47c408722..c2218c6fa 100644 --- a/libsignal-service/src/sender.rs +++ b/libsignal-service/src/sender.rs @@ -37,7 +37,7 @@ pub struct OutgoingPushMessage { #[derive(serde::Serialize, Debug)] pub struct OutgoingPushMessages { - pub recipient: ServiceAddress, + pub destination: uuid::Uuid, pub timestamp: u64, pub messages: Vec, pub online: bool, @@ -498,7 +498,7 @@ where .await?; let messages = OutgoingPushMessages { - recipient, + destination: recipient.uuid, timestamp, messages, online, diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index caa22364d..4da35634e 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -1,7 +1,6 @@ use std::convert::TryFrom; use libsignal_protocol::{DeviceId, ProtocolAddress}; -use serde::{Deserialize, Serialize}; use uuid::Uuid; pub use crate::push_service::ServiceIdType; @@ -15,7 +14,7 @@ pub enum ParseServiceAddressError { NoUuid, } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize, Hash)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] pub struct ServiceAddress { pub uuid: Uuid, pub identity: ServiceIdType, diff --git a/libsignal-service/src/websocket/sender.rs b/libsignal-service/src/websocket/sender.rs index 166d9c800..e9ea90ef6 100644 --- a/libsignal-service/src/websocket/sender.rs +++ b/libsignal-service/src/websocket/sender.rs @@ -12,7 +12,7 @@ impl SignalWebSocket { &mut self, messages: OutgoingPushMessages, ) -> Result { - let path = format!("/v1/messages/{}", messages.recipient.uuid); + let path = format!("/v1/messages/{}", messages.destination); self.put_json(&path, messages).await } @@ -21,7 +21,7 @@ impl SignalWebSocket { messages: OutgoingPushMessages, access: &UnidentifiedAccess, ) -> Result { - let path = format!("/v1/messages/{}", messages.recipient.uuid); + let path = format!("/v1/messages/{}", messages.destination); let header = format!( "Unidentified-Access-Key:{}", BASE64_RELAXED.encode(&access.key) From 17845a0dec8af3509f68aac250e381284b0c2e30 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 29 Jun 2024 17:23:46 +0300 Subject: [PATCH 18/20] Better explain ServiceAddress unwrap fails --- libsignal-service/src/envelope.rs | 3 ++- libsignal-service/src/profile_service.rs | 7 ++++--- libsignal-service/src/push_service.rs | 6 +++--- libsignal-service/src/service_address.rs | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/libsignal-service/src/envelope.rs b/libsignal-service/src/envelope.rs index 64d9c575d..75f0d6d89 100644 --- a/libsignal-service/src/envelope.rs +++ b/libsignal-service/src/envelope.rs @@ -135,7 +135,8 @@ impl Envelope { pub fn source_address(&self) -> ServiceAddress { match self.source_service_id.as_deref() { - Some(uuid) => ServiceAddress::try_from(uuid).unwrap(), + Some(service_id) => ServiceAddress::try_from(service_id) + .expect("invalid ProtocolAddress UUID or prefix"), None => panic!("source_service_id is set"), } } diff --git a/libsignal-service/src/profile_service.rs b/libsignal-service/src/profile_service.rs index 10061f84f..e23a4b51b 100644 --- a/libsignal-service/src/profile_service.rs +++ b/libsignal-service/src/profile_service.rs @@ -21,9 +21,10 @@ impl ProfileService { ) -> Result { let endpoint = match profile_key { Some(key) => { - let version = bincode::serialize( - &key.get_profile_key_version(address.aci().unwrap()), - )?; + let version = + bincode::serialize(&key.get_profile_key_version( + address.aci().expect("profile by ACI ProtocolAddress"), + ))?; let version = std::str::from_utf8(&version) .expect("hex encoded profile key version"); format!("/v1/profile/{}/{}", address.uuid, version) diff --git a/libsignal-service/src/push_service.rs b/libsignal-service/src/push_service.rs index a2383c843..073f40459 100644 --- a/libsignal-service/src/push_service.rs +++ b/libsignal-service/src/push_service.rs @@ -902,9 +902,9 @@ pub trait PushService: MaybeSend { profile_key: Option, ) -> Result { let endpoint = if let Some(key) = profile_key { - let version = bincode::serialize( - &key.get_profile_key_version(address.aci().unwrap()), - )?; + let version = bincode::serialize(&key.get_profile_key_version( + address.aci().expect("profile by ACI ProtocolAddress"), + ))?; let version = std::str::from_utf8(&version) .expect("hex encoded profile key version"); format!("/v1/profile/{}/{}", address.uuid, version) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 4da35634e..b5df2578c 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -106,7 +106,7 @@ impl TryFrom<&str> for ServiceAddress { fn try_from(value: &str) -> Result { if value.starts_with("PNI:") { Ok(ServiceAddress { - uuid: Uuid::parse_str(value.strip_prefix("PNI:").unwrap())?, + uuid: Uuid::parse_str(value.strip_prefix("PNI:").expect("invalid UUID in PNI ProtocolAddress"))?, identity: ServiceIdType::PhoneNumberIdentity, }) } else { From 7eadac0ca9fdaec73ab4f9aafb0aed691c319262 Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 29 Jun 2024 19:33:26 +0300 Subject: [PATCH 19/20] Disallow assuming ACI ServiceAddress from Uuid --- libsignal-service/src/service_address.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index b5df2578c..4cb54a533 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -36,6 +36,20 @@ impl ServiceAddress { } } + pub fn new_aci(uuid: Uuid) -> Self { + Self { + uuid, + identity: ServiceIdType::AccountIdentity, + } + } + + pub fn new_pni(uuid: Uuid) -> Self { + Self { + uuid, + identity: ServiceIdType::PhoneNumberIdentity, + } + } + pub fn aci(&self) -> Option { use libsignal_protocol::Aci; match self.identity { @@ -66,15 +80,6 @@ impl ServiceAddress { } } -impl From for ServiceAddress { - fn from(uuid: Uuid) -> Self { - Self { - uuid, - identity: ServiceIdType::AccountIdentity, - } - } -} - impl TryFrom<&ProtocolAddress> for ServiceAddress { type Error = ParseServiceAddressError; From 95e83e83b0d04675e1d9d43e6ea499fa496d333a Mon Sep 17 00:00:00 2001 From: Matti Viljanen Date: Sat, 29 Jun 2024 19:33:43 +0300 Subject: [PATCH 20/20] Clean up `try_from` blocks --- libsignal-service/src/service_address.rs | 50 +++++++++--------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/libsignal-service/src/service_address.rs b/libsignal-service/src/service_address.rs index 4cb54a533..912c06503 100644 --- a/libsignal-service/src/service_address.rs +++ b/libsignal-service/src/service_address.rs @@ -86,22 +86,14 @@ impl TryFrom<&ProtocolAddress> for ServiceAddress { fn try_from(addr: &ProtocolAddress) -> Result { let value = addr.name(); if let Some(pni) = value.strip_prefix("PNI:") { - Ok(ServiceAddress { - uuid: Uuid::parse_str(pni)?, - identity: ServiceIdType::PhoneNumberIdentity, - }) + Ok(ServiceAddress::new_pni(Uuid::parse_str(pni)?)) } else { - match Uuid::parse_str(value) { - Ok(uuid) => Ok(ServiceAddress { - uuid, - identity: ServiceIdType::AccountIdentity, - }), - Err(e) => { - tracing::error!("Unknown identity format: {}", value); - Err(ParseServiceAddressError::InvalidUuid(e)) - }, - } + Ok(ServiceAddress::new_aci(Uuid::parse_str(value)?)) } + .map_err(|e| { + tracing::error!("Parsing ServiceAddress from {:?}", addr); + ParseServiceAddressError::InvalidUuid(e) + }) } } @@ -109,17 +101,15 @@ impl TryFrom<&str> for ServiceAddress { type Error = ParseServiceAddressError; fn try_from(value: &str) -> Result { - if value.starts_with("PNI:") { - Ok(ServiceAddress { - uuid: Uuid::parse_str(value.strip_prefix("PNI:").expect("invalid UUID in PNI ProtocolAddress"))?, - identity: ServiceIdType::PhoneNumberIdentity, - }) + if let Some(pni) = value.strip_prefix("PNI:") { + Ok(ServiceAddress::new_pni(Uuid::parse_str(pni)?)) } else { - Ok(ServiceAddress { - uuid: Uuid::parse_str(value)?, - identity: ServiceIdType::AccountIdentity, - }) + Ok(ServiceAddress::new_aci(Uuid::parse_str(value)?)) } + .map_err(|e| { + tracing::error!("Parsing ServiceAddress from '{}'", value); + ParseServiceAddressError::InvalidUuid(e) + }) } } @@ -128,15 +118,13 @@ impl TryFrom<&[u8]> for ServiceAddress { fn try_from(value: &[u8]) -> Result { if let Some(pni) = value.strip_prefix(b"PNI:") { - Ok(ServiceAddress { - uuid: Uuid::from_slice(pni)?, - identity: ServiceIdType::PhoneNumberIdentity, - }) + Ok(ServiceAddress::new_pni(Uuid::from_slice(pni)?)) } else { - Ok(ServiceAddress { - uuid: Uuid::from_slice(value)?, - identity: ServiceIdType::AccountIdentity, - }) + Ok(ServiceAddress::new_aci(Uuid::from_slice(value)?)) } + .map_err(|e| { + tracing::error!("Parsing ServiceAddress from {:?}", value); + ParseServiceAddressError::InvalidUuid(e) + }) } }