Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove E164 phone numbers from ServiceAddress #198

Merged
merged 12 commits into from
Feb 8, 2023
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions libsignal-service-actix/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![recursion_limit = "256"]
#![allow(clippy::uninlined_format_args)]

pub mod push_service;
pub mod websocket;
Expand Down
1 change: 1 addition & 0 deletions libsignal-service-hyper/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![recursion_limit = "256"]
#![allow(clippy::uninlined_format_args)]

pub mod push_service;
pub mod websocket;
Expand Down
8 changes: 1 addition & 7 deletions libsignal-service/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -50,5 +45,4 @@ tokio = { version = "1.0", features = ["macros", "rt"] }
rustls = "0.20"

[features]
prefer-e164 = []
unsend-futures = []
42 changes: 10 additions & 32 deletions libsignal-service/src/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ where
Type::UnidentifiedSender => {
let SealedSenderDecryptionResult {
sender_uuid,
sender_e164,
sender_e164: _,
rubdos marked this conversation as resolved.
Show resolved Hide resolved
device_id,
mut message,
} = sealed_sender_decrypt(
Expand All @@ -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() {
Expand Down Expand Up @@ -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 = ProtocolAddress::new(address.to_string(), device_id);
if session_store.load_session(&address, None).await?.is_some() {
return Ok(address);
}

Ok(ProtocolAddress::new(address.identifier(), device_id))
Ok(ProtocolAddress::new(address.to_string(), device_id))
}

/// Decrypt a Sealed Sender message `ciphertext` in either the v1 or v2 format, validate its sender
Expand Down
5 changes: 1 addition & 4 deletions libsignal-service/src/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
ProtocolAddress::new(self.sender.to_string(), self.sender_device.into())
}
}

Expand Down
64 changes: 15 additions & 49 deletions libsignal-service/src/envelope.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::convert::{TryFrom, TryInto};

use prost::Message;
use uuid::Uuid;

Expand All @@ -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)),
}
}
}
Expand Down Expand Up @@ -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()
}
Expand All @@ -151,17 +121,13 @@ impl Envelope {
}

pub fn source_address(&self) -> ServiceAddress {
let uuid = self
let uuid: 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 }
}
}

Expand Down
2 changes: 2 additions & 0 deletions libsignal-service/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
7 changes: 3 additions & 4 deletions libsignal-service/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use bytes::Bytes;
use serde::{Deserialize, Serialize};
use thiserror::Error;

use std::convert::TryInto;

/// Attachment represents an attachment received from a peer
#[derive(Debug, Serialize, Deserialize)]
pub struct Attachment<R> {
Expand Down Expand Up @@ -48,10 +50,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(),
Expand Down
7 changes: 4 additions & 3 deletions libsignal-service/src/profile_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl ProfileService {
profile_key: Option<zkgroup::profiles::ProfileKey>,
) -> Result<SignalServiceProfile, ServiceError> {
let endpoint = match (profile_key, address.uuid) {
(Some(key), Some(uuid)) => {
(Some(key), uuid) => {
let uid_bytes = uuid.as_bytes();
let version = bincode::serialize(
&key.get_profile_key_version(*uid_bytes),
Expand All @@ -29,8 +29,9 @@ impl ProfileService {
.expect("hex encoded profile key version");
format!("/v1/profile/{}/{}", uuid, version)
},
(_, _) => {
format!("/v1/profile/{}", address.identifier())
(_, uuid) => {
// TODO: check if this is even possible?
format!("/v1/profile/{}", uuid)
},
};

Expand Down
40 changes: 19 additions & 21 deletions libsignal-service/src/push_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -414,6 +414,9 @@ pub enum ServiceError {

#[error("unsupported content")]
UnsupportedContent,

#[error(transparent)]
ParseServiceAddress(#[from] ParseServiceAddressError),
}

#[cfg_attr(feature = "unsend-futures", async_trait::async_trait(?Send))]
Expand Down Expand Up @@ -573,11 +576,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);
self.put_json(
Endpoint::Service,
&path,
Expand Down Expand Up @@ -659,19 +662,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, version)
} else {
format!("/v1/profile/{}", address)
};
// TODO: set locale to en_US
self.get_json(
Expand Down Expand Up @@ -701,8 +700,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, device_id);

let mut pre_key_response: PreKeyResponse = self
.get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)
Expand All @@ -720,9 +718,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)
} else {
format!("/v2/keys/{}/{}", destination.identifier(), device_id)
format!("/v2/keys/{}/{}", destination, device_id)
gferon marked this conversation as resolved.
Show resolved Hide resolved
};
let pre_key_response: PreKeyResponse = self
.get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)
Expand Down
13 changes: 2 additions & 11 deletions libsignal-service/src/receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?;
Expand All @@ -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)
Expand Down
Loading