Skip to content

Commit 3742bdc

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 b22bb3f commit 3742bdc

File tree

3 files changed

+207
-55
lines changed

3 files changed

+207
-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: 169 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,58 @@ 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 device_owner = sender_device.device_owner_identity.as_ref();
249+
let master_key = device_owner.and_then(|i| i.master_key().get_first_key());
250+
251+
match (device_owner, master_key) {
252+
(Some(device_owner), Some(master_key)) => {
253+
// We have user_id and master_key for the user sending the to-device message.
254+
let master_key = Box::new(master_key);
255+
let known_sender_data = KnownSenderData { user_id, device_id, master_key };
256+
if sender_device.is_cross_signing_trusted() {
257+
Self::SenderVerified(known_sender_data)
258+
} else if device_owner.was_previously_verified() {
259+
Self::VerificationViolation(known_sender_data)
260+
} else {
261+
Self::SenderUnverified(known_sender_data)
262+
}
263+
}
264+
265+
(_, _) => {
266+
// Surprisingly, there was no key in the MasterPubkey. We did not expect this:
267+
// treat it as if the device was not signed by this master key.
268+
//
269+
error!("MasterPubkey for user {user_id} does not contain any keys!");
270+
Self::device_info(sender_device.as_device_keys().clone())
271+
}
272+
}
273+
}
274+
219275
/// Returns `Greater` if this `SenderData` represents a greater level of
220276
/// trust than the supplied one, `Equal` if they have the same level, and
221277
/// `Less` if the supplied one has a greater level of trust.
@@ -344,10 +400,11 @@ pub enum SenderDataType {
344400

345401
#[cfg(test)]
346402
mod tests {
347-
use std::{cmp::Ordering, collections::BTreeMap};
403+
use std::{cmp::Ordering, collections::BTreeMap, ops::Deref};
348404

349405
use assert_matches2::assert_let;
350406
use insta::assert_json_snapshot;
407+
use matrix_sdk_test::async_test;
351408
use ruma::{
352409
device_id, owned_device_id, owned_user_id, user_id, DeviceKeyAlgorithm, DeviceKeyId,
353410
};
@@ -356,8 +413,13 @@ mod tests {
356413

357414
use super::SenderData;
358415
use crate::{
359-
olm::{KnownSenderData, PickledInboundGroupSession},
416+
machine::test_helpers::{
417+
create_signed_device_of_unverified_user, create_signed_device_of_verified_user,
418+
create_unsigned_device,
419+
},
420+
olm::{KnownSenderData, PickledInboundGroupSession, PrivateCrossSigningIdentity},
360421
types::{DeviceKey, DeviceKeys, EventEncryptionAlgorithm, Signatures},
422+
Account,
361423
};
362424

363425
#[test]
@@ -648,4 +710,108 @@ mod tests {
648710

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

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)