Skip to content

Commit

Permalink
crypto: Verified identity changes - Update latch on identity update
Browse files Browse the repository at this point in the history
  • Loading branch information
BillCarsonFr committed Aug 2, 2024
1 parent e9f07eb commit 53b0484
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 14 deletions.
67 changes: 53 additions & 14 deletions crates/matrix-sdk-crypto/src/identities/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::{
Result as StoreResult, Store, StoreCache, UserKeyQueryResult,
},
types::{CrossSigningKey, DeviceKeys, MasterPubkey, SelfSigningPubkey, UserSigningPubkey},
CryptoStoreError, LocalTrust, SignatureError,
CryptoStoreError, LocalTrust, OwnUserIdentity, SignatureError, UserIdentities,
};

enum DeviceChange {
Expand Down Expand Up @@ -85,6 +85,13 @@ struct KeysQueryRequestDetails {
request_ids: HashSet<OwnedTransactionId>,
}

// Helper type to handle key query response
struct KeySetInfo {
user_id: OwnedUserId,
master_key: MasterPubkey,
self_signing: SelfSigningPubkey,
}

impl IdentityManager {
const MAX_KEY_QUERY_USERS: usize = 250;

Expand Down Expand Up @@ -444,6 +451,7 @@ impl IdentityManager {
async fn handle_changed_identity(
&self,
response: &KeysQueryResponse,
maybe_verified_own_identity: Option<&OwnUserIdentity>,
master_key: MasterPubkey,
self_signing: SelfSigningPubkey,
i: UserIdentityData,
Expand All @@ -461,7 +469,12 @@ impl IdentityManager {
}
}
UserIdentityData::Other(mut identity) => {
let has_changed = identity.update(master_key, self_signing)?;
let has_changed = identity.update(
master_key,
self_signing,
maybe_verified_own_identity.map(|o| o.user_signing_key()),
)?;

if has_changed {
Ok(IdentityUpdateResult::Updated(identity.into()))
} else {
Expand Down Expand Up @@ -502,18 +515,27 @@ impl IdentityManager {
async fn handle_new_identity(
&self,
response: &KeysQueryResponse,
maybe_verified_own_identity: Option<&OwnUserIdentity>,
master_key: MasterPubkey,
self_signing: SelfSigningPubkey,
changed_private_identity: &mut Option<PrivateCrossSigningIdentity>,
) -> Result<UserIdentityData, SignatureError> {
if master_key.user_id() == self.user_id() {
// Own identity
let user_signing = self.get_user_signing_key_from_response(response)?;
let identity = OwnUserIdentityData::new(master_key, self_signing, user_signing)?;
*changed_private_identity = self.check_private_identity(&identity).await;
Ok(identity.into())
} else {
// First time seen, create the identity. The current MSK will be pinned.
let identity = OtherUserIdentityData::new(master_key, self_signing)?;
let is_verified = maybe_verified_own_identity.map_or(false, |own_user_identity| {
own_user_identity.is_identity_signed(&identity).is_ok()
});
if is_verified {
identity.mark_as_verified_once();
}

Ok(identity.into())
}
}
Expand Down Expand Up @@ -616,28 +638,28 @@ impl IdentityManager {
/// * `changed_identity` - Output parameter: Unchanged if the identity is
/// that of another user. If it is our own, set to `None` or `Some`
/// depending on whether our stored private identity needs updating.
/// * `user_id` - The user id of the user whose identity is being processed.
/// * `master_key` - The public master cross-signing key for this user from
/// the `/keys/query` response.
/// * `self_signing` - The public self-signing key from the `/keys/query`
/// * `maybe_verified_own_identity` - Own verified identity if any to check
/// verification status of updated identity.
/// * `key_set_info` - The identity info as returned by the `/keys/query`
/// response.
#[instrument(skip_all, fields(user_id))]
async fn update_or_create_identity(
&self,
response: &KeysQueryResponse,
changes: &mut IdentityChanges,
changed_private_identity: &mut Option<PrivateCrossSigningIdentity>,
user_id: &UserId,
master_key: MasterPubkey,
self_signing: SelfSigningPubkey,
maybe_verified_own_identity: Option<&OwnUserIdentity>,
key_set_info: KeySetInfo,
) -> StoreResult<()> {
let KeySetInfo { user_id, master_key, self_signing } = key_set_info;
if master_key.user_id() != user_id || self_signing.user_id() != user_id {
warn!(?user_id, "User ID mismatch in one of the cross signing keys");
} else if let Some(i) = self.store.get_user_identity(user_id).await? {
} else if let Some(i) = self.store.get_user_identity(&user_id).await? {
// an identity we knew about before, which is being updated
match self
.handle_changed_identity(
response,
maybe_verified_own_identity,
master_key,
self_signing,
i,
Expand All @@ -660,7 +682,13 @@ impl IdentityManager {
} else {
// an identity we did not know about before
match self
.handle_new_identity(response, master_key, self_signing, changed_private_identity)
.handle_new_identity(
response,
maybe_verified_own_identity,
master_key,
self_signing,
changed_private_identity,
)
.await
{
Ok(identity) => {
Expand Down Expand Up @@ -698,6 +726,15 @@ impl IdentityManager {
let mut changes = IdentityChanges::default();
let mut changed_identity = None;

// We want to check if the updated/new other identities are trusted by us or
// not. This is based on the current verified state of the own identity.
let maybe_own_verified_identity = self
.store
.get_identity(self.user_id())
.await?
.and_then(UserIdentities::own)
.filter(|own| own.is_verified());

for (user_id, master_key) in &response.master_keys {
// Get the master and self-signing key for each identity; those are required for
// every user identity type. If we don't have those we skip over.
Expand All @@ -707,13 +744,14 @@ impl IdentityManager {
continue;
};

let key_set_info = KeySetInfo { user_id: user_id.clone(), master_key, self_signing };

self.update_or_create_identity(
response,
&mut changes,
&mut changed_identity,
user_id,
master_key,
self_signing,
maybe_own_verified_identity.as_ref(),
key_set_info,
)
.await?;
}
Expand Down Expand Up @@ -1353,6 +1391,7 @@ pub(crate) mod tests {
use crate::{
identities::manager::testing::{other_key_query_cross_signed, own_key_query},
olm::PrivateCrossSigningIdentity,
CrossSigningKeyExport, OlmMachine,
};

fn key_query_with_failures() -> KeysQueryResponse {
Expand Down
12 changes: 12 additions & 0 deletions crates/matrix-sdk-crypto/src/identities/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,13 +669,17 @@ impl OtherUserIdentityData {
///
/// * `self_signing_key` - The new self signing key of user identity.
///
/// * `maybe_verified_own_user_signing_key` - Our own user_signing_key if it
/// is verified to check the identity trust status after update.
///
/// Returns a `SignatureError` if we failed to update the identity.
/// Otherwise, returns `true` if there was a change to the identity and
/// `false` if the identity is unchanged.
pub(crate) fn update(
&mut self,
master_key: MasterPubkey,
self_signing_key: SelfSigningPubkey,
maybe_verified_own_user_signing_key: Option<&UserSigningPubkey>,
) -> Result<bool, SignatureError> {
master_key.verify_subkey(&self_signing_key)?;

Expand All @@ -685,11 +689,19 @@ impl OtherUserIdentityData {
// (see `has_pin_violation()`).
let pinned_master_key = self.pinned_master_key.read().unwrap().clone();

// Check if the new master_key is signed by our own **verified**
// user_signing_key. If the identity was verified we remember it.
let updated_is_verified = maybe_verified_own_user_signing_key
.map_or(false, |own_user_signing_key| {
own_user_signing_key.verify_master_key(&master_key).is_ok()
});

let new = Self {
user_id: master_key.user_id().into(),
master_key: master_key.clone().into(),
self_signing_key: self_signing_key.into(),
pinned_master_key: RwLock::new(pinned_master_key).into(),
verified_latch: Arc::new((self.is_verified_latch_set() || updated_is_verified).into()),
};
let changed = new != *self;

Expand Down

0 comments on commit 53b0484

Please sign in to comment.