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

Embed device keys in Olm-encrypted messages #3517

Merged
merged 22 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/matrix-sdk-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ testing = ["dep:http"]
[dependencies]
aes = "0.8.1"
as_variant = { workspace = true }
assert_matches2 = { workspace = true }
async-trait = { workspace = true }
bs58 = { version = "0.5.0" }
byteorder = { workspace = true }
Expand Down
13 changes: 13 additions & 0 deletions crates/matrix-sdk-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,19 @@ pub enum EventError {
MismatchedRoom(OwnedRoomId, Option<OwnedRoomId>),
}

/// Error type describing different errors that can happen when we create an
/// Olm session from a pickle.
#[derive(Error, Debug)]
pub enum SessionPickleError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SessionUnpickleError might be marginally better, feel free to disagree.

/// The device keys are missing the signing key
#[error("the device keys are missing the signing key")]
MissingSigningKey,

/// The device keys are missing the identity key
#[error("the device keys are missing the identity key")]
MissingIdentityKey,
}

/// Error type describing different errors that happen when we check or create
/// signatures for a Matrix JSON object.
#[derive(Error, Debug)]
Expand Down
7 changes: 7 additions & 0 deletions crates/matrix-sdk-crypto/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,14 @@ impl OlmMachine {
let static_account = account.static_data().clone();

let store = Arc::new(CryptoStoreWrapper::new(self.user_id(), MemoryStore::new()));
let device = ReadOnlyDevice::from_account(&account);
store.save_pending_changes(PendingChanges { account: Some(account) }).await?;
store
.save_changes(Changes {
devices: DeviceChanges { new: vec![device], ..Default::default() },
..Default::default()
})
.await?;

Ok(Self::new_helper(
device_id,
Expand Down
20 changes: 4 additions & 16 deletions crates/matrix-sdk-crypto/src/olm/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ impl Account {
inner: Arc::new(Mutex::new(session)),
session_id: session_id.into(),
sender_key: identity_key,
device_keys: our_device_keys,
our_device_keys,
created_using_fallback_key: fallback_used,
creation_time: now,
last_use_time: now,
Expand Down Expand Up @@ -1053,7 +1053,7 @@ impl Account {
inner: Arc::new(Mutex::new(result.session)),
session_id: session_id.into(),
sender_key: their_identity_key,
device_keys: our_device_keys,
our_device_keys,
created_using_fallback_key: false,
creation_time: now,
last_use_time: now,
Expand Down Expand Up @@ -1301,25 +1301,13 @@ impl Account {
);

return Err(OlmError::SessionWedged(
session.device_keys.user_id.to_owned(),
session.our_device_keys.user_id.to_owned(),
session.sender_key(),
));
}
}

// We didn't find a matching session; try to create a new session.
// Try to get our own stored device keys.
let device_keys = store
.get_device(&self.user_id, &self.device_id)
.await
.unwrap_or(None)
.map(|read_only_device| read_only_device.as_device_keys().clone());
// If we don't have our device keys stored, fall back to
// generating a fresh device keys from our own Account.
let device_keys = match device_keys {
Some(device_keys) => device_keys,
None => self.device_keys(),
};
let device_keys = store.get_own_device().await?.as_device_keys().clone();
let result =
match self.create_inbound_session(sender_key, device_keys, prekey_message) {
Ok(r) => r,
Expand Down
63 changes: 33 additions & 30 deletions crates/matrix-sdk-crypto/src/olm/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use vodozemac::{
#[cfg(feature = "experimental-algorithms")]
use crate::types::events::room::encrypted::OlmV2Curve25519AesSha2Content;
use crate::{
error::{EventError, OlmResult},
error::{EventError, OlmResult, SessionPickleError},
types::{
events::room::encrypted::{OlmV1Curve25519AesSha2Content, ToDeviceEncryptedEventContent},
DeviceKeys, EventEncryptionAlgorithm,
Expand All @@ -45,8 +45,8 @@ pub struct Session {
pub session_id: Arc<str>,
/// The Key of the sender
pub sender_key: Curve25519PublicKey,
/// The signed device keys
pub device_keys: DeviceKeys,
/// Our own signed device keys
pub our_device_keys: DeviceKeys,
/// Has this been created using the fallback key
pub created_using_fallback_key: bool,
/// When the session was created
Expand Down Expand Up @@ -151,12 +151,12 @@ impl Session {
recipient_device.ed25519_key().ok_or(EventError::MissingSigningKey)?;

let payload = json!({
"sender": &self.device_keys.user_id,
"sender_device": &self.device_keys.device_id,
"sender": &self.our_device_keys.user_id,
"sender_device": &self.our_device_keys.device_id,
"keys": {
"ed25519": self.device_keys.ed25519_key().unwrap().to_base64(),
"ed25519": self.our_device_keys.ed25519_key().expect("Device doesn't have ed25519 key").to_base64(),
},
"device_keys": self.device_keys,
"device_keys": self.our_device_keys,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this property need a different name:

Until this MSC is accepted, the new property should be named org.matrix.msc4147.device_keys.

We can do this in a follow up, so we don't risk more merge conflicts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"recipient": recipient_device.user_id(),
"recipient_keys": {
"ed25519": recipient_signing_key.to_base64(),
Expand All @@ -174,14 +174,20 @@ impl Session {
EventEncryptionAlgorithm::OlmV1Curve25519AesSha2 => OlmV1Curve25519AesSha2Content {
ciphertext,
recipient_key: self.sender_key,
sender_key: self.device_keys.curve25519_key().unwrap(),
sender_key: self
.our_device_keys
.curve25519_key()
.expect("Device doesn't have curve25519 key"),
message_id,
}
.into(),
#[cfg(feature = "experimental-algorithms")]
EventEncryptionAlgorithm::OlmV2Curve25519AesSha2 => OlmV2Curve25519AesSha2Content {
ciphertext,
sender_key: self.device_keys.curve25519_key().unwrap(),
sender_key: self
.device_keys
.curve25519_key()
.expect("Device doesn't have curve25519 key"),
message_id,
}
.into(),
Expand Down Expand Up @@ -223,30 +229,32 @@ impl Session {
///
/// # Arguments
///
/// * `user_id` - Our own user id that the session belongs to.
///
/// * `device_id` - Our own device ID that the session belongs to.
///
/// * `our_identity_keys` - An clone of the Arc to our own identity keys.
/// * `our_device_keys` - Our own signed device keys.
///
/// * `pickle` - The pickled version of the `Session`.
///
/// * `pickle_mode` - The mode that was used to pickle the session, either
/// an unencrypted mode or an encrypted using passphrase.
pub fn from_pickle(device_keys: DeviceKeys, pickle: PickledSession) -> Self {
// FIXME: assert that device_keys has curve25519 and ed25519 keys
pub fn from_pickle(
our_device_keys: DeviceKeys,
pickle: PickledSession,
) -> Result<Self, SessionPickleError> {
if our_device_keys.curve25519_key().is_none() {
return Err(SessionPickleError::MissingIdentityKey);
}
if our_device_keys.ed25519_key().is_none() {
return Err(SessionPickleError::MissingSigningKey);
}

let session: vodozemac::olm::Session = pickle.pickle.into();
let session_id = session.session_id();

Session {
Ok(Session {
inner: Arc::new(Mutex::new(session)),
session_id: session_id.into(),
created_using_fallback_key: pickle.created_using_fallback_key,
sender_key: pickle.sender_key,
device_keys,
our_device_keys,
creation_time: pickle.creation_time,
last_use_time: pickle.last_use_time,
}
})
}
}

Expand Down Expand Up @@ -278,6 +286,7 @@ pub struct PickledSession {

#[cfg(test)]
mod tests {
use assert_matches2::assert_let;
use matrix_sdk_test::async_test;
use ruma::{device_id, user_id};
use serde_json::{self, Value};
Expand Down Expand Up @@ -320,15 +329,9 @@ mod tests {
.unwrap();

#[cfg(feature = "experimental-algorithms")]
let ToDeviceEncryptedEventContent::OlmV2Curve25519AesSha2(content) = message
else {
panic!("Invalid encrypted event algorithm {}", message.algorithm());
};
assert_let!(ToDeviceEncryptedEventContent::OlmV2Curve25519AesSha2(content) = message);
#[cfg(not(feature = "experimental-algorithms"))]
let ToDeviceEncryptedEventContent::OlmV1Curve25519AesSha2(content) = message
else {
panic!("Invalid encrypted event algorithm {}", message.algorithm());
};
assert_let!(ToDeviceEncryptedEventContent::OlmV1Curve25519AesSha2(content) = message);

let prekey = if let OlmMessage::PreKey(m) = content.ciphertext {
m
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,8 +902,9 @@ impl_crypto_store! {
device_keys.clone(),
p,
)
.map_err(|_| IndexeddbCryptoStoreError::CryptoStoreError(CryptoStoreError::AccountUnset))
}))
.collect::<Vec<Session>>();
.collect::<Result<Vec<Session>>>()?;

self.session_cache.set_for_sender(sender_key, sessions);
}
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-sqlite/src/crypto_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,8 @@ impl CryptoStore for SqliteCryptoStore {
.into_iter()
.map(|bytes| {
let pickle = self.deserialize_value(&bytes)?;
Ok(Session::from_pickle(device_keys.clone(), pickle))
Session::from_pickle(device_keys.clone(), pickle)
.map_err(|_| Error::AccountUnset)
})
.collect::<Result<_>>()?;

Expand Down
Loading