From 25f4245a3b82c8f8ace093ec181bdb55263b461f Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 17 Dec 2024 15:49:55 +0000 Subject: [PATCH 1/5] task(crypto): Accept old PreviouslyVerified value of SenderData when deserializing --- .../src/olm/group_sessions/sender_data.rs | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs index df54390e580..0be07403e00 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs @@ -215,6 +215,7 @@ enum SenderDataReader { legacy_session: bool, }, + #[serde(alias = "SenderUnverifiedButPreviouslyVerified")] VerificationViolation(KnownSenderData), SenderUnverified(KnownSenderData), @@ -286,7 +287,10 @@ mod tests { use vodozemac::Ed25519PublicKey; use super::SenderData; - use crate::types::{DeviceKeys, Signatures}; + use crate::{ + olm::KnownSenderData, + types::{DeviceKeys, Signatures}, + }; #[test] fn serializing_unknown_device_correctly_preserves_owner_check_failed_if_true() { @@ -360,6 +364,47 @@ mod tests { assert_let!(SenderData::SenderVerified { .. } = end); } + #[test] + fn deserializing_sender_unverified_but_previously_verified_migrates_to_verification_violation() + { + let json = r#" + { + "SenderUnverifiedButPreviouslyVerified":{ + "user_id":"@u:s.co", + "master_key":[ + 150,140,249,139,141,29,63,230,179,14,213,175,176,61,11,255, + 26,103,10,51,100,154,183,47,181,117,87,204,33,215,241,92 + ], + "master_key_verified":true + } + } + "#; + + let end: SenderData = serde_json::from_str(json).expect("Failed to parse!"); + assert_let!(SenderData::VerificationViolation(KnownSenderData { user_id, .. }) = end); + assert_eq!(user_id, owned_user_id!("@u:s.co")); + } + + #[test] + fn deserializing_verification_violation() { + let json = r#" + { + "VerificationViolation":{ + "user_id":"@u:s.co", + "master_key":[ + 150,140,249,139,141,29,63,230,179,14,213,175,176,61,11,255, + 26,103,10,51,100,154,183,47,181,117,87,204,33,215,241,92 + ], + "master_key_verified":true + } + } + "#; + + let end: SenderData = serde_json::from_str(json).expect("Failed to parse!"); + assert_let!(SenderData::VerificationViolation(KnownSenderData { user_id, .. }) = end); + assert_eq!(user_id, owned_user_id!("@u:s.co")); + } + #[test] fn equal_sessions_have_same_trust_level() { let unknown = SenderData::unknown(); From 04e3fbe56dd0120d1f6c3758254df900c420b93a Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 18 Dec 2024 09:42:49 +0000 Subject: [PATCH 2/5] task(crypto): Accept old PreviouslyVerified value of VerificationLevel when deserializing --- .../src/deserialized_responses.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 65dcb153196..e7f1ec2d78a 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -176,6 +176,7 @@ pub enum VerificationLevel { /// The message was sent by a user identity we have not verified, but the /// user was previously verified. + #[serde(alias = "PreviouslyVerified")] VerificationViolation, /// The message was sent by a device not linked to (signed by) any user @@ -889,6 +890,40 @@ mod tests { ); } + #[test] + fn test_verification_level_deserializes() { + // Given a JSON VerificationLevel + #[derive(Deserialize)] + struct Container { + verification_level: VerificationLevel, + } + let container = json!({ "verification_level": "VerificationViolation" }); + + // When we deserialize it + let deserialized: Container = serde_json::from_value(container) + .expect("We can deserialize the old PreviouslyVerified value"); + + // Then it is populated correctly + assert_eq!(deserialized.verification_level, VerificationLevel::VerificationViolation); + } + + #[test] + fn test_verification_level_deserializes_from_old_previously_verified_value() { + // Given a JSON VerificationLevel with the old value PreviouslyVerified + #[derive(Deserialize)] + struct Container { + verification_level: VerificationLevel, + } + let container = json!({ "verification_level": "PreviouslyVerified" }); + + // When we deserialize it + let deserialized: Container = serde_json::from_value(container) + .expect("We can deserialize the old PreviouslyVerified value"); + + // Then it is migrated to the new value + assert_eq!(deserialized.verification_level, VerificationLevel::VerificationViolation); + } + #[test] fn sync_timeline_event_serialisation() { let room_event = SyncTimelineEvent { From 901003b09b1483d09c80143ffe8e13c6d5c492e2 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 18 Dec 2024 09:49:17 +0000 Subject: [PATCH 3/5] task(crypto): Accept old PreviouslyVerified value of ShieldStateCode when deserializing --- .../src/deserialized_responses.rs | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index e7f1ec2d78a..626fe86d302 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -260,6 +260,7 @@ pub enum ShieldStateCode { /// An unencrypted event in an encrypted room. SentInClear, /// The sender was previously verified but changed their identity. + #[serde(alias = "PreviouslyVerified")] VerificationViolation, } @@ -815,7 +816,7 @@ mod tests { TimelineEventKind, UnableToDecryptInfo, UnableToDecryptReason, UnsignedDecryptionResult, UnsignedEventLocation, VerificationState, }; - use crate::deserialized_responses::{DeviceLinkProblem, VerificationLevel}; + use crate::deserialized_responses::{DeviceLinkProblem, ShieldStateCode, VerificationLevel}; fn example_event() -> serde_json::Value { json!({ @@ -924,6 +925,40 @@ mod tests { assert_eq!(deserialized.verification_level, VerificationLevel::VerificationViolation); } + #[test] + fn test_shield_state_code_deserializes() { + // Given a JSON ShieldStateCode with value VerificationViolation + #[derive(Deserialize)] + struct Container { + shield_state_code: ShieldStateCode, + } + let container = json!({ "shield_state_code": "VerificationViolation" }); + + // When we deserialize it + let deserialized: Container = serde_json::from_value(container) + .expect("We can deserialize the old PreviouslyVerified value"); + + // Then it is populated correctly + assert_eq!(deserialized.shield_state_code, ShieldStateCode::VerificationViolation); + } + + #[test] + fn test_shield_state_code_deserializes_from_old_previously_verified_value() { + // Given a JSON ShieldStateCode with the old value PreviouslyVerified + #[derive(Deserialize)] + struct Container { + shield_state_code: ShieldStateCode, + } + let container = json!({ "shield_state_code": "PreviouslyVerified" }); + + // When we deserialize it + let deserialized: Container = serde_json::from_value(container) + .expect("We can deserialize the old PreviouslyVerified value"); + + // Then it is migrated to the new value + assert_eq!(deserialized.shield_state_code, ShieldStateCode::VerificationViolation); + } + #[test] fn sync_timeline_event_serialisation() { let room_event = SyncTimelineEvent { From 67ed6178c91e60209ee1b87a61d4cfc6d3c41995 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 18 Dec 2024 10:16:38 +0000 Subject: [PATCH 4/5] fix(crypto): Fix error when reading VerifiedStateOrBool with old PreviouslyVerifiedButNoLonger value --- .../matrix-sdk-crypto/src/identities/user.rs | 77 ++++++++++++++----- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs index 45436c29128..0bb651aa9db 100644 --- a/crates/matrix-sdk-crypto/src/identities/user.rs +++ b/crates/matrix-sdk-crypto/src/identities/user.rs @@ -920,6 +920,7 @@ enum OwnUserIdentityVerifiedState { NeverVerified, /// We previously verified this identity, but it has changed. + #[serde(alias = "PreviouslyVerifiedButNoLonger")] VerificationViolation, /// We have verified the current identity. @@ -1539,26 +1540,10 @@ pub(crate) mod tests { /// that we can deserialize boolean values. #[test] fn test_deserialize_own_user_identity_bool_verified() { - let mut json = json!({ - "user_id": "@example:localhost", - "master_key": { - "user_id":"@example:localhost", - "usage":["master"], - "keys":{"ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0":"rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0"}, - }, - "self_signing_key": { - "user_id":"@example:localhost", - "usage":["self_signing"], - "keys":{"ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210":"0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210"} - }, - "user_signing_key": { - "user_id":"@example:localhost", - "usage":["user_signing"], - "keys":{"ed25519:DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo":"DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo"} - }, - "verified": false - }); + let mut json = own_user_identity_data(); + // Set `"verified": false` + *json.get_mut("verified").unwrap() = false.into(); let id: OwnUserIdentityData = serde_json::from_value(json.clone()).unwrap(); assert_eq!(*id.verified.read().unwrap(), OwnUserIdentityVerifiedState::NeverVerified); @@ -1568,6 +1553,38 @@ pub(crate) mod tests { assert_eq!(*id.verified.read().unwrap(), OwnUserIdentityVerifiedState::Verified); } + #[test] + fn test_own_user_identity_verified_state_verification_violation_deserializes() { + // Given data containing verified: VerificationViolation + let mut json = own_user_identity_data(); + *json.get_mut("verified").unwrap() = "VerificationViolation".into(); + + // When we deserialize + let id: OwnUserIdentityData = serde_json::from_value(json.clone()).unwrap(); + + // Then the value is correctly populated + assert_eq!( + *id.verified.read().unwrap(), + OwnUserIdentityVerifiedState::VerificationViolation + ); + } + + #[test] + fn test_own_user_identity_verified_state_previously_verified_deserializes() { + // Given data containing verified: PreviouslyVerifiedButNoLonger + let mut json = own_user_identity_data(); + *json.get_mut("verified").unwrap() = "PreviouslyVerifiedButNoLonger".into(); + + // When we deserialize + let id: OwnUserIdentityData = serde_json::from_value(json.clone()).unwrap(); + + // Then the old value is re-interpreted as VerificationViolation + assert_eq!( + *id.verified.read().unwrap(), + OwnUserIdentityVerifiedState::VerificationViolation + ); + } + #[test] fn own_identity_check_signatures() { let response = own_key_query(); @@ -1943,4 +1960,26 @@ pub(crate) mod tests { assert!(!own_identity.was_previously_verified()); assert!(!own_identity.has_verification_violation()); } + + fn own_user_identity_data() -> Value { + json!({ + "user_id": "@example:localhost", + "master_key": { + "user_id":"@example:localhost", + "usage":["master"], + "keys":{"ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0":"rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0"}, + }, + "self_signing_key": { + "user_id":"@example:localhost", + "usage":["self_signing"], + "keys":{"ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210":"0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210"} + }, + "user_signing_key": { + "user_id":"@example:localhost", + "usage":["user_signing"], + "keys":{"ed25519:DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo":"DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo"} + }, + "verified": false + }) + } } From 49ed922845de8c3dae3d9768b176ece7e9a5d42c Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 18 Dec 2024 13:27:07 +0000 Subject: [PATCH 5/5] doc(crypto): Add changelog entry for #4424 --- crates/matrix-sdk-crypto/CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 9809ea75244..b5568ec2bbe 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to this project will be documented in this file. +## Unreleased changes + +### Bug fixes + +- Fix [#4424](https://github.com/matrix-org/matrix-rust-sdk/issues/4424) Failed + storage upgrade for "PreviouslyVerifiedButNoLonger". This bug caused errors to + occur when loading crypto information from storage, which typically prevented + apps from starting correctly. + ([#4430](https://github.com/matrix-org/matrix-rust-sdk/pull/4430)) + ## [0.8.0] - 2024-11-19 ### Features