Remove E164 phone numbers from ServiceAddress (#198)

README.md

@@ -47,7 +47,6 @@ If you're looking to contribute or want to ask a question, you're more than welc
 | Feature flag     | Description                                                                                                                                                                             |
 |------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `unsend-futures` | This feature removes the `Send` requirement on returned futures. Enabling this flag may be necessary for interoperability with other libraries that don't support `Send` such as actix. |
-| `prefer-e164`    | This is a legacy feature that should not be used in new applications.                                                                                                                   |
 
 ## License
 

libsignal-service-actix/src/lib.rs

@@ -1,4 +1,5 @@
 #![recursion_limit = "256"]
+#![allow(clippy::uninlined_format_args)]
 
 pub mod push_service;
 pub mod websocket;

libsignal-service-hyper/src/lib.rs

@@ -1,4 +1,5 @@
 #![recursion_limit = "256"]
+#![allow(clippy::uninlined_format_args)]
 
 pub mod push_service;
 pub mod websocket;

libsignal-service/Cargo.toml

@@ -1,12 +1,7 @@
 [package]
 name = "libsignal-service"
 version = "0.1.0"
-authors = [
-    "Ruben De Smet <ruben.de.smet@rubdos.be>",
-    "Gabriel Féron <g@leirbag.net>",
-    "Michael Bryan <michaelfbryan@gmail.com>",
-    "Shady Khalifa <shekohex@gmail.com>",
-]
+authors = ["Ruben De Smet <ruben.de.smet@rubdos.be>", "Gabriel Féron <g@leirbag.net>", "Michael Bryan <michaelfbryan@gmail.com>", "Shady Khalifa <shekohex@gmail.com>"]
 edition = "2018"
 license = "GPLv3"
 readme = "../README.md"
@@ -50,5 +45,4 @@ tokio = { version = "1.0", features = ["macros", "rt"] }
 rustls = "0.20"
 
 [features]
-prefer-e164 = []
 unsend-futures = []

libsignal-service/src/cipher.rs

@@ -206,7 +206,7 @@ where
             Type::UnidentifiedSender => {
                 let SealedSenderDecryptionResult {
                     sender_uuid,
-                    sender_e164,
+                    sender_e164: _,
                     device_id,
                     mut message,
                 } = sealed_sender_decrypt(
@@ -226,15 +226,11 @@ where
                 .await?;
 
                 let sender = ServiceAddress {
-                    phonenumber: sender_e164
-                        .and_then(|n| phonenumber::parse(None, n).ok()),
-                    uuid: Some(Uuid::parse_str(&sender_uuid).map_err(
-                        |_| {
-                            SignalProtocolError::InvalidSealedSenderMessage(
-                                "invalid sender UUID".to_string(),
-                            )
-                        },
-                    )?),
+                    uuid: Uuid::parse_str(&sender_uuid).map_err(|_| {
+                        SignalProtocolError::InvalidSealedSenderMessage(
+                            "invalid sender UUID".to_string(),
+                        )
+                    })?,
                 };
 
                 let needs_receipt = if envelope.source_uuid.is_some() {
@@ -418,30 +414,12 @@ pub async fn get_preferred_protocol_address<S: SessionStore>(
     address: &ServiceAddress,
     device_id: DeviceId,
 ) -> Result<ProtocolAddress, libsignal_protocol::error::SignalProtocolError> {
-    if let Some(ref uuid) = address.uuid {
-        let address = ProtocolAddress::new(uuid.to_string(), device_id);
-        if session_store.load_session(&address, None).await?.is_some() {
-            return Ok(address);
-        }
-    }
-    if let Some(e164) = address.e164() {
-        let address = ProtocolAddress::new(e164, device_id);
-        if session_store.load_session(&address, None).await?.is_some() {
-            return Ok(address);
-        }
-        if cfg!(feature = "prefer-e164") {
-            log::warn!("prefer-e164 triggered.  This is a legacy feature and shouldn't be used for new applications.");
-            return Ok(address);
-        }
-    }
-    if cfg!(feature = "prefer-e164") {
-        panic!(
-            "{:?}:{} does not have a e164 associated, falling back to UUID.",
-            address, device_id
-        );
+    let address = address.to_protocol_address(device_id);
+    if session_store.load_session(&address, None).await?.is_some() {
+        return Ok(address);
     }
 
-    Ok(ProtocolAddress::new(address.identifier(), device_id))
+    Ok(address)
 }
 
 /// Decrypt a Sealed Sender message `ciphertext` in either the v1 or v2 format, validate its sender

libsignal-service/src/content.rs

@@ -22,10 +22,7 @@ pub struct Metadata {
 
 impl Metadata {
     pub(crate) fn protocol_address(&self) -> ProtocolAddress {
-        ProtocolAddress::new(
-            self.sender.identifier(),
-            self.sender_device.into(),
-        )
+        self.sender.to_protocol_address(self.sender_device)
     }
 }
 

libsignal-service/src/envelope.rs

@@ -1,3 +1,5 @@
+use std::convert::{TryFrom, TryInto};
+
 use prost::Message;
 use uuid::Uuid;
 
@@ -8,48 +10,16 @@ use crate::{
 
 pub use crate::proto::Envelope;
 
-#[derive(thiserror::Error, Debug, Clone)]
-pub enum EnvelopeParseError {
-    #[error("Supplied phone number could not be parsed in E164 format")]
-    InvalidPhoneNumber(#[from] phonenumber::ParseError),
-
-    #[error("Supplied uuid could not be parsed")]
-    InvalidUuidError(#[from] uuid::Error),
-
-    #[error("Envelope with neither Uuid or E164")]
-    NoSenderError,
-}
-
-impl std::convert::TryFrom<EnvelopeEntity> for Envelope {
-    type Error = EnvelopeParseError;
+impl TryFrom<EnvelopeEntity> for Envelope {
+    type Error = ParseServiceAddressError;
 
-    fn try_from(
-        entity: EnvelopeEntity,
-    ) -> Result<Envelope, EnvelopeParseError> {
-        use ParseServiceAddressError::*;
-        if entity.source.is_none() && entity.source_uuid.is_none() {
-            return Err(EnvelopeParseError::NoSenderError);
-        }
-
-        // XXX: throwing allocations like it's Java.
-        let source = ServiceAddress::parse(
-            entity.source.as_deref(),
-            entity.source_uuid.as_deref(),
-        );
-        match source {
-            // Valid source
-            Ok(source) if entity.source_device > 0 => {
-                Ok(Envelope::new_with_source(entity, source))
-            },
-            // No source
-            Ok(_) | Err(NoSenderError) => Ok(Envelope::new_from_entity(entity)),
-            // Source specified, but unparsable
-            Err(InvalidPhoneNumber(e)) => {
-                Err(EnvelopeParseError::InvalidPhoneNumber(e))
-            },
-            Err(InvalidUuidError(e)) => {
-                Err(EnvelopeParseError::InvalidUuidError(e))
+    fn try_from(entity: EnvelopeEntity) -> Result<Self, Self::Error> {
+        match entity.source_uuid.as_deref() {
+            Some(uuid) => {
+                let address = uuid.try_into()?;
+                Ok(Envelope::new_with_source(entity, address))
             },
+            None => Ok(Envelope::new_from_entity(entity)),
         }
     }
 }
@@ -128,7 +98,7 @@ impl Envelope {
             source_device: Some(entity.source_device),
             timestamp: Some(entity.timestamp),
             server_timestamp: Some(entity.server_timestamp),
-            source_uuid: source.uuid.as_ref().map(|s| s.to_string()),
+            source_uuid: Some(source.uuid.to_string()),
             content: entity.content,
             ..Default::default()
         }
@@ -154,14 +124,10 @@ impl Envelope {
         let uuid = self
             .source_uuid
             .as_deref()
-            .map(Uuid::parse_str)
-            .transpose()
-            .expect("valid e164 checked in constructor");
+            .and_then(|u| Uuid::parse_str(u).ok())
+            .expect("valid uuid checked in constructor");
 
-        ServiceAddress {
-            phonenumber: None,
-            uuid,
-        }
+        ServiceAddress { uuid }
     }
 }
 

libsignal-service/src/lib.rs

@@ -1,5 +1,7 @@
 #![recursion_limit = "256"]
 #![deny(clippy::dbg_macro)]
+// TODO: we cannot use this until whisperfish builds with a newer Rust version
+#![allow(clippy::uninlined_format_args)]
 
 mod account_manager;
 pub mod attachment_cipher;

libsignal-service/src/models.rs

@@ -7,6 +7,8 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 use zkgroup::profiles::ProfileKey;
 
+use std::convert::TryInto;
+
 /// Attachment represents an attachment received from a peer
 #[derive(Debug, Serialize, Deserialize)]
 pub struct Attachment<R> {
@@ -51,10 +53,7 @@ impl Contact {
         avatar_data: Option<Bytes>,
     ) -> Result<Self, ParseContactError> {
         Ok(Self {
-            address: ServiceAddress::parse(
-                contact_details.number.as_deref(),
-                contact_details.uuid.as_deref(),
-            )?,
+            address: contact_details.uuid.as_deref().try_into()?,
             name: contact_details.name().into(),
             color: contact_details.color.clone(),
             verified: contact_details.verified.clone().unwrap_or_default(),

libsignal-service/src/profile_service.rs

@@ -19,18 +19,18 @@ impl ProfileService {
         address: ServiceAddress,
         profile_key: Option<zkgroup::profiles::ProfileKey>,
     ) -> Result<SignalServiceProfile, ServiceError> {
-        let endpoint = match (profile_key, address.uuid) {
-            (Some(key), Some(uuid)) => {
-                let uid_bytes = uuid.as_bytes();
+        let endpoint = match profile_key {
+            Some(key) => {
+                let uid_bytes = address.uuid.as_bytes();
                 let version = bincode::serialize(
                     &key.get_profile_key_version(*uid_bytes),
                 )?;
                 let version = std::str::from_utf8(&version)
                     .expect("hex encoded profile key version");
-                format!("/v1/profile/{}/{}", uuid, version)
+                format!("/v1/profile/{}/{}", address.uuid, version)
             },
-            (_, _) => {
-                format!("/v1/profile/{}", address.identifier())
+            None => {
+                format!("/v1/profile/{}", address.uuid)
             },
         };
 

libsignal-service/src/push_service.rs

@@ -10,7 +10,7 @@ use crate::{
     sender::{OutgoingPushMessages, SendMessageResponse},
     utils::{serde_base64, serde_optional_base64},
     websocket::SignalWebSocket,
-    MaybeSend, Profile, ServiceAddress,
+    MaybeSend, ParseServiceAddressError, Profile, ServiceAddress,
 };
 
 use aes_gcm::{
@@ -379,6 +379,9 @@ pub enum ServiceError {
 
     #[error("unsupported content")]
     UnsupportedContent,
+
+    #[error(transparent)]
+    ParseServiceAddress(#[from] ParseServiceAddressError),
 }
 
 #[cfg_attr(feature = "unsend-futures", async_trait::async_trait(?Send))]
@@ -538,11 +541,11 @@ pub trait PushService: MaybeSend {
         }
     }
 
-    async fn send_messages<'a>(
+    async fn send_messages(
         &mut self,
-        messages: OutgoingPushMessages<'a>,
+        messages: OutgoingPushMessages,
     ) -> Result<SendMessageResponse, ServiceError> {
-        let path = format!("/v1/messages/{}", messages.destination);
+        let path = format!("/v1/messages/{}", messages.recipient.uuid);
         self.put_json(
             Endpoint::Service,
             &path,
@@ -624,19 +627,15 @@ pub trait PushService: MaybeSend {
         address: ServiceAddress,
         profile_key: Option<zkgroup::profiles::ProfileKey>,
     ) -> Result<SignalServiceProfile, ServiceError> {
-        let endpoint = match (profile_key, address.uuid) {
-            (Some(key), Some(uuid)) => {
-                let uid_bytes = uuid.as_bytes();
-                let version = bincode::serialize(
-                    &key.get_profile_key_version(*uid_bytes),
-                )?;
-                let version = std::str::from_utf8(&version)
-                    .expect("hex encoded profile key version");
-                format!("/v1/profile/{}/{}", uuid, version)
-            },
-            (_, _) => {
-                format!("/v1/profile/{}", address.identifier())
-            },
+        let endpoint = if let Some(key) = profile_key {
+            let uid_bytes = address.uuid.as_bytes();
+            let version =
+                bincode::serialize(&key.get_profile_key_version(*uid_bytes))?;
+            let version = std::str::from_utf8(&version)
+                .expect("hex encoded profile key version");
+            format!("/v1/profile/{}/{}", address.uuid, version)
+        } else {
+            format!("/v1/profile/{}", address.uuid)
         };
         // TODO: set locale to en_US
         self.get_json(
@@ -666,8 +665,7 @@ pub trait PushService: MaybeSend {
         destination: &ServiceAddress,
         device_id: u32,
     ) -> Result<PreKeyBundle, ServiceError> {
-        let path =
-            format!("/v2/keys/{}/{}", destination.identifier(), device_id);
+        let path = format!("/v2/keys/{}/{}", destination.uuid, device_id);
 
         let mut pre_key_response: PreKeyResponse = self
             .get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)
@@ -685,9 +683,9 @@ pub trait PushService: MaybeSend {
         device_id: u32,
     ) -> Result<Vec<PreKeyBundle>, ServiceError> {
         let path = if device_id == 1 {
-            format!("/v2/keys/{}/*", destination.identifier())
+            format!("/v2/keys/{}/*", destination.uuid)
         } else {
-            format!("/v2/keys/{}/{}", destination.identifier(), device_id)
+            format!("/v2/keys/{}/{}", destination.uuid, device_id)
         };
         let pre_key_response: PreKeyResponse = self
             .get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)

libsignal-service/src/receiver.rs

@@ -15,15 +15,6 @@ pub struct MessageReceiver<Service> {
     service: Service,
 }
 
-#[derive(thiserror::Error, Debug)]
-pub enum MessageReceiverError {
-    #[error("ServiceError")]
-    ServiceError(#[from] ServiceError),
-
-    #[error("Envelope parsing error")]
-    EnvelopeParseError(#[from] crate::envelope::EnvelopeParseError),
-}
-
 impl<Service: PushService> MessageReceiver<Service> {
     // TODO: to avoid providing the wrong service/wrong credentials
     // change it like LinkingManager or ProvisioningManager
@@ -39,7 +30,7 @@ impl<Service: PushService> MessageReceiver<Service> {
     /// [`MessageReceiver::create_message_pipe()`].
     pub async fn retrieve_messages(
         &mut self,
-    ) -> Result<Vec<Envelope>, MessageReceiverError> {
+    ) -> Result<Vec<Envelope>, ServiceError> {
         use std::convert::TryFrom;
 
         let entities = self.service.get_messages().await?;
@@ -53,7 +44,7 @@ impl<Service: PushService> MessageReceiver<Service> {
     pub async fn create_message_pipe(
         &mut self,
         credentials: ServiceCredentials,
-    ) -> Result<MessagePipe, MessageReceiverError> {
+    ) -> Result<MessagePipe, ServiceError> {
         let ws = self
             .service
             .ws("/v1/websocket/", Some(credentials.clone()), true)