Skip to content

Commit c8dfc81

Browse files
committed
crypto: move some logic from SenderDataFinder to SenderData
create a new method `SenderData::from_device` which does the last few steps of `SenderDataFinder`: turns out we want it elsewhere. Add some tests to test that functionality in isolation.
1 parent 8d7c6e9 commit c8dfc81

File tree

3 files changed

+213
-55
lines changed

3 files changed

+213
-55
lines changed

crates/matrix-sdk-crypto/src/machine/test_helpers.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,36 @@ pub async fn create_signed_device_of_unverified_user(
332332
device
333333
}
334334

335+
/// Sign the given [`DeviceKeys`] with a cross-signing identity, then sign the
336+
/// identity with our own identity, and wrap both up as a [`Device`].
337+
pub async fn create_signed_device_of_verified_user(
338+
mut device_keys: DeviceKeys,
339+
device_owner_identity: &PrivateCrossSigningIdentity,
340+
own_identity: &PrivateCrossSigningIdentity,
341+
) -> Device {
342+
// Sign the device keys with the owner's identity.
343+
{
344+
let self_signing = device_owner_identity.self_signing_key.lock().await;
345+
let self_signing = self_signing.as_ref().unwrap();
346+
self_signing.sign_device(&mut device_keys).unwrap();
347+
}
348+
349+
let mut public_identity = OtherUserIdentityData::from_private(device_owner_identity).await;
350+
351+
// Sign the public identity of the owner with our own identity.
352+
sign_user_identity_data(own_identity, &mut public_identity).await;
353+
354+
let device = Device {
355+
inner: DeviceData::try_from(&device_keys).unwrap(),
356+
verification_machine: dummy_verification_machine(),
357+
own_identity: Some(own_identity.to_public_identity().await.unwrap()),
358+
device_owner_identity: Some(public_identity.into()),
359+
};
360+
assert!(device.is_cross_signed_by_owner());
361+
assert!(device.is_cross_signing_trusted());
362+
device
363+
}
364+
335365
/// Sign a public user identity with our own user-signing key.
336366
pub async fn sign_user_identity_data(
337367
signer_private_identity: &PrivateCrossSigningIdentity,

crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs

Lines changed: 175 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@ use std::{cmp::Ordering, fmt};
1616

1717
use ruma::{DeviceId, OwnedDeviceId, OwnedUserId, UserId};
1818
use serde::{de, de::Visitor, Deserialize, Deserializer, Serialize};
19+
use tracing::error;
1920
use vodozemac::Ed25519PublicKey;
2021

21-
use crate::types::{serialize_ed25519_key, DeviceKeys};
22+
use crate::{
23+
types::{serialize_ed25519_key, DeviceKeys},
24+
Device,
25+
};
2226

2327
/// Information about the sender of a megolm session where we know the
2428
/// cross-signing identity of the sender.
@@ -216,6 +220,61 @@ impl SenderData {
216220
Self::UnknownDevice { legacy_session: true, owner_check_failed: false }
217221
}
218222

223+
/// Create a [`SenderData`] representing the current verification state of
224+
/// the given device.
225+
///
226+
/// Depending on whether the device is correctly cross-signed or not, and
227+
/// whether the user has been verified or not, this can return
228+
/// [`SenderData::DeviceInfo`], [`SenderData::VerificationViolation`],
229+
/// [`SenderData::SenderUnverified`] or [`SenderData::SenderVerified`]
230+
pub fn from_device(sender_device: &Device) -> Self {
231+
// Is the device cross-signed?
232+
// Does the cross-signing key match that used to sign the device?
233+
// And is the signature in the device valid?
234+
let cross_signed = sender_device.is_cross_signed_by_owner();
235+
236+
if cross_signed {
237+
Self::from_cross_signed_device(sender_device)
238+
} else {
239+
// We have device keys, but they are not signed by the sender
240+
SenderData::device_info(sender_device.as_device_keys().clone())
241+
}
242+
}
243+
244+
fn from_cross_signed_device(sender_device: &Device) -> Self {
245+
let user_id = sender_device.user_id().to_owned();
246+
let device_id = Some(sender_device.device_id().to_owned());
247+
248+
let master_key = sender_device
249+
.device_owner_identity
250+
.as_ref()
251+
.and_then(|i| i.master_key().get_first_key());
252+
253+
if let Some(master_key) = master_key {
254+
// We have user_id and master_key for the user sending the to-device message.
255+
let master_key = Box::new(master_key);
256+
let known_sender_data = KnownSenderData { user_id, device_id, master_key };
257+
if sender_device.is_cross_signing_trusted() {
258+
Self::SenderVerified(known_sender_data)
259+
} else if sender_device
260+
.device_owner_identity
261+
.as_ref()
262+
.expect("User with master key must have identity")
263+
.was_previously_verified()
264+
{
265+
Self::VerificationViolation(known_sender_data)
266+
} else {
267+
Self::SenderUnverified(known_sender_data)
268+
}
269+
} else {
270+
// Surprisingly, there was no key in the MasterPubkey. We did not expect this:
271+
// treat it as if the device was not signed by this master key.
272+
//
273+
error!("MasterPubkey for user {user_id} does not contain any keys!");
274+
Self::device_info(sender_device.as_device_keys().clone())
275+
}
276+
}
277+
219278
/// Returns `Greater` if this `SenderData` represents a greater level of
220279
/// trust than the supplied one, `Equal` if they have the same level, and
221280
/// `Less` if the supplied one has a greater level of trust.
@@ -344,10 +403,11 @@ pub enum SenderDataType {
344403

345404
#[cfg(test)]
346405
mod tests {
347-
use std::{cmp::Ordering, collections::BTreeMap};
406+
use std::{cmp::Ordering, collections::BTreeMap, ops::Deref};
348407

349408
use assert_matches2::assert_let;
350409
use insta::assert_json_snapshot;
410+
use matrix_sdk_test::async_test;
351411
use ruma::{
352412
device_id, owned_device_id, owned_user_id, user_id, DeviceKeyAlgorithm, DeviceKeyId,
353413
};
@@ -356,8 +416,13 @@ mod tests {
356416

357417
use super::SenderData;
358418
use crate::{
359-
olm::{KnownSenderData, PickledInboundGroupSession},
419+
machine::test_helpers::{
420+
create_signed_device_of_unverified_user, create_signed_device_of_verified_user,
421+
create_unsigned_device,
422+
},
423+
olm::{KnownSenderData, PickledInboundGroupSession, PrivateCrossSigningIdentity},
360424
types::{DeviceKey, DeviceKeys, EventEncryptionAlgorithm, Signatures},
425+
Account,
361426
};
362427

363428
#[test]
@@ -648,4 +713,111 @@ mod tests {
648713

649714
assert_eq!(master_key.to_base64(), "kOp9s4ClyQujYD7rRZA8xgE6kvYlqKSNnMrQNmSrcuE");
650715
}
716+
717+
#[async_test]
718+
async fn test_from_device_for_unsigned_device() {
719+
let bob_account =
720+
Account::with_device_id(user_id!("@bob:example.com"), device_id!("BOB_DEVICE"));
721+
let bob_device = create_unsigned_device(bob_account.device_keys().clone());
722+
723+
let sender_data = SenderData::from_device(&bob_device);
724+
725+
assert_eq!(
726+
sender_data,
727+
SenderData::DeviceInfo {
728+
device_keys: bob_device.device_keys.deref().clone(),
729+
legacy_session: false
730+
}
731+
);
732+
}
733+
734+
#[async_test]
735+
async fn test_from_device_for_unverified_user() {
736+
let bob_identity =
737+
PrivateCrossSigningIdentity::new(user_id!("@bob:example.com").to_owned());
738+
let bob_account =
739+
Account::with_device_id(user_id!("@bob:example.com"), device_id!("BOB_DEVICE"));
740+
let bob_device = create_signed_device_of_unverified_user(
741+
bob_account.device_keys().clone(),
742+
&bob_identity,
743+
)
744+
.await;
745+
746+
let sender_data = SenderData::from_device(&bob_device);
747+
748+
assert_eq!(
749+
sender_data,
750+
SenderData::SenderUnverified(KnownSenderData {
751+
user_id: bob_account.user_id().to_owned(),
752+
device_id: Some(bob_account.device_id().to_owned()),
753+
master_key: Box::new(
754+
bob_identity.master_public_key().await.unwrap().get_first_key().unwrap()
755+
),
756+
})
757+
);
758+
}
759+
760+
#[async_test]
761+
async fn test_from_device_for_verified_user() {
762+
let alice_account =
763+
Account::with_device_id(user_id!("@alice:example.com"), device_id!("ALICE_DEVICE"));
764+
let alice_identity = PrivateCrossSigningIdentity::with_account(&alice_account).await.0;
765+
766+
let bob_identity =
767+
PrivateCrossSigningIdentity::new(user_id!("@bob:example.com").to_owned());
768+
let bob_account =
769+
Account::with_device_id(user_id!("@bob:example.com"), device_id!("BOB_DEVICE"));
770+
let bob_device = create_signed_device_of_verified_user(
771+
bob_account.device_keys().clone(),
772+
&bob_identity,
773+
&alice_identity,
774+
)
775+
.await;
776+
777+
let sender_data = SenderData::from_device(&bob_device);
778+
779+
assert_eq!(
780+
sender_data,
781+
SenderData::SenderVerified(KnownSenderData {
782+
user_id: bob_account.user_id().to_owned(),
783+
device_id: Some(bob_account.device_id().to_owned()),
784+
master_key: Box::new(
785+
bob_identity.master_public_key().await.unwrap().get_first_key().unwrap()
786+
),
787+
})
788+
);
789+
}
790+
791+
#[async_test]
792+
async fn test_from_device_for_verification_violation_user() {
793+
let bob_identity =
794+
PrivateCrossSigningIdentity::new(user_id!("@bob:example.com").to_owned());
795+
let bob_account =
796+
Account::with_device_id(user_id!("@bob:example.com"), device_id!("BOB_DEVICE"));
797+
let bob_device = create_signed_device_of_unverified_user(
798+
bob_account.device_keys().clone(),
799+
&bob_identity,
800+
)
801+
.await;
802+
bob_device
803+
.device_owner_identity
804+
.as_ref()
805+
.unwrap()
806+
.other()
807+
.unwrap()
808+
.mark_as_previously_verified();
809+
810+
let sender_data = SenderData::from_device(&bob_device);
811+
812+
assert_eq!(
813+
sender_data,
814+
SenderData::VerificationViolation(KnownSenderData {
815+
user_id: bob_account.user_id().to_owned(),
816+
device_id: Some(bob_account.device_id().to_owned()),
817+
master_key: Box::new(
818+
bob_identity.master_public_key().await.unwrap().get_first_key().unwrap()
819+
),
820+
})
821+
);
822+
}
651823
}

crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs

Lines changed: 8 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@
1313
// limitations under the License.
1414

1515
use ruma::UserId;
16-
use tracing::error;
1716
use vodozemac::Curve25519PublicKey;
1817

19-
use super::{InboundGroupSession, KnownSenderData, SenderData};
18+
use super::{InboundGroupSession, SenderData};
2019
use crate::{
2120
error::MismatchedIdentityKeysError, store::Store, types::events::olm_v1::DecryptedRoomKeyEvent,
2221
CryptoStoreError, Device, DeviceData, MegolmError, OlmError, SignatureError,
@@ -229,57 +228,14 @@ impl<'a> SenderDataFinder<'a> {
229228
// Is the session owned by the device?
230229
let device_is_owner = sender_device.is_owner_of_session(self.session)?;
231230

232-
// Is the device cross-signed?
233-
// Does the cross-signing key match that used to sign the device?
234-
// And is the signature in the device valid?
235-
let cross_signed = sender_device.is_cross_signed_by_owner();
236-
237-
Ok(match (device_is_owner, cross_signed) {
238-
(true, true) => self.device_is_cross_signed_by_sender(sender_device),
239-
(true, false) => {
240-
// F (we have device keys, but they are not signed by the sender)
241-
SenderData::device_info(sender_device.as_device_keys().clone())
242-
}
243-
(false, _) => {
244-
// Step E (the device does not own the session)
245-
// Give up: something is wrong with the session.
246-
SenderData::UnknownDevice { legacy_session: false, owner_check_failed: true }
247-
}
248-
})
249-
}
250-
251-
/// Step G (device is cross-signed by the sender)
252-
fn device_is_cross_signed_by_sender(&self, sender_device: Device) -> SenderData {
253-
// H (cross-signing key matches that used to sign the device!)
254-
let user_id = sender_device.user_id().to_owned();
255-
let device_id = Some(sender_device.device_id().to_owned());
256-
257-
let master_key = sender_device
258-
.device_owner_identity
259-
.as_ref()
260-
.and_then(|i| i.master_key().get_first_key());
261-
262-
if let Some(master_key) = master_key {
263-
// We have user_id and master_key for the user sending the to-device message.
264-
let master_key = Box::new(master_key);
265-
let known_sender_data = KnownSenderData { user_id, device_id, master_key };
266-
if sender_device.is_cross_signing_trusted() {
267-
SenderData::SenderVerified(known_sender_data)
268-
} else if sender_device
269-
.device_owner_identity
270-
.expect("User with master key must have identity")
271-
.was_previously_verified()
272-
{
273-
SenderData::VerificationViolation(known_sender_data)
274-
} else {
275-
SenderData::SenderUnverified(known_sender_data)
276-
}
231+
if !device_is_owner {
232+
// Step E (the device does not own the session)
233+
// Give up: something is wrong with the session.
234+
Ok(SenderData::UnknownDevice { legacy_session: false, owner_check_failed: true })
277235
} else {
278-
// Surprisingly, there was no key in the MasterPubkey. We did not expect this:
279-
// treat it as if the device was not signed by this master key.
280-
//
281-
error!("MasterPubkey for user {user_id} does not contain any keys!",);
282-
SenderData::device_info(sender_device.as_device_keys().clone())
236+
// Steps F, G, and H: we have a device, which may or may not be signed by the
237+
// sender.
238+
Ok(SenderData::from_device(&sender_device))
283239
}
284240
}
285241
}

0 commit comments

Comments
 (0)