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

crypto: misc refactorings #3000

Merged
merged 6 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
20 changes: 11 additions & 9 deletions crates/matrix-sdk-crypto/src/gossiping/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl GossipMachine {
let device =
self.inner.store.get_device(&event.sender, &event.content.requesting_device_id).await?;

Ok(if let Some(device) = device {
if let Some(device) = device {
if device.user_id() == self.user_id() {
if device.is_verified() {
info!(
Expand All @@ -338,7 +338,7 @@ impl GossipMachine {
Ok(None)
}
Err(e) => Err(e),
}?
}
} else {
info!(
user_id = ?device.user_id(),
Expand All @@ -347,7 +347,7 @@ impl GossipMachine {
"Received a secret request that we won't serve, the device isn't trusted",
);

None
Ok(None)
}
} else {
info!(
Expand All @@ -357,7 +357,7 @@ impl GossipMachine {
"Received a secret request that we won't serve, the device doesn't belong to us",
);

None
Ok(None)
}
} else {
warn!(
Expand All @@ -374,8 +374,8 @@ impl GossipMachine {
.mark_user_as_changed(&event.sender)
.await?;

None
})
Ok(None)
}
}

/// Try to encrypt the given `InboundGroupSession` for the given `Device` as
Expand Down Expand Up @@ -1247,7 +1247,8 @@ mod tests {
.with_transaction(|mut btr| async {
let alice_account = atr.account().await?;
let bob_account = btr.account().await?;
let sessions = alice_account.create_session_for(bob_account).await;
let sessions =
alice_account.create_session_for_test_helper(bob_account).await;
Ok((btr, sessions))
})
.await?;
Expand Down Expand Up @@ -1845,7 +1846,7 @@ mod tests {
.with_transaction(|mut tr| async {
let alice_account = tr.account().await?;
let (alice_session, _) =
alice_account.create_session_for(&mut second_account).await;
alice_account.create_session_for_test_helper(&mut second_account).await;
Ok((tr, alice_session))
})
.await
Expand Down Expand Up @@ -2068,7 +2069,8 @@ mod tests {
.with_transaction(|mut btr| async {
let alice_account = atr.account().await?;
let bob_account = btr.account().await?;
let sessions = alice_account.create_session_for(bob_account).await;
let sessions =
alice_account.create_session_for_test_helper(bob_account).await;
Ok((btr, sessions))
})
.await?;
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-crypto/src/identities/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ impl ReadOnlyDevice {
}
} else {
warn!(
"Trying to find a Olm session of a device, but the device doesn't have a \
"Trying to find an Olm session of a device, but the device doesn't have a \
Curve25519 key",
);

Expand Down
21 changes: 10 additions & 11 deletions crates/matrix-sdk-crypto/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ impl OlmMachine {
transaction: &mut StoreTransaction,
changes: &mut Changes,
mut raw_event: Raw<AnyToDeviceEvent>,
) -> OlmResult<Raw<AnyToDeviceEvent>> {
) -> Raw<AnyToDeviceEvent> {
Self::record_message_id(&raw_event);

let event: ToDeviceEvents = match raw_event.deserialize_as() {
Expand All @@ -1110,7 +1110,7 @@ impl OlmMachine {
// Skip invalid events.
warn!("Received an invalid to-device event: {e}");

return Ok(raw_event);
return raw_event;
}
};

Expand All @@ -1135,7 +1135,7 @@ impl OlmMachine {
}
}

return Ok(raw_event);
return raw_event;
}
};

Expand Down Expand Up @@ -1172,7 +1172,7 @@ impl OlmMachine {
e => self.handle_to_device_event(changes, &e).await,
}

Ok(raw_event)
raw_event
}

/// Handle a to-device and one-time key counts from a sync response.
Expand Down Expand Up @@ -1247,8 +1247,7 @@ impl OlmMachine {

for raw_event in sync_changes.to_device_events {
let raw_event =
Box::pin(self.receive_to_device_event(transaction, &mut changes, raw_event))
.await?;
Box::pin(self.receive_to_device_event(transaction, &mut changes, raw_event)).await;
events.push(raw_event);
}

Expand Down Expand Up @@ -2245,7 +2244,7 @@ pub(crate) mod tests {
let account = tr.account().await.unwrap();
account.generate_fallback_key_helper();
account.update_uploaded_key_count(0);
account.generate_one_time_keys();
account.generate_one_time_keys_if_needed();
let request = machine
.keys_for_upload(account)
.await
Expand Down Expand Up @@ -2371,7 +2370,7 @@ pub(crate) mod tests {
.store()
.with_transaction(|mut tr| async {
let account = tr.account().await.unwrap();
assert!(account.generate_one_time_keys().is_some());
assert!(account.generate_one_time_keys_if_needed().is_some());
Ok((tr, ()))
})
.await
Expand All @@ -2385,7 +2384,7 @@ pub(crate) mod tests {
.store()
.with_transaction(|mut tr| async {
let account = tr.account().await.unwrap();
assert!(account.generate_one_time_keys().is_some());
assert!(account.generate_one_time_keys_if_needed().is_some());
Ok((tr, ()))
})
.await
Expand All @@ -2399,7 +2398,7 @@ pub(crate) mod tests {
.store()
.with_transaction(|mut tr| async {
let account = tr.account().await.unwrap();
assert!(account.generate_one_time_keys().is_none());
assert!(account.generate_one_time_keys_if_needed().is_none());

Ok((tr, ()))
})
Expand Down Expand Up @@ -2467,7 +2466,7 @@ pub(crate) mod tests {
fn test_one_time_key_signing() {
let mut account = Account::with_device_id(user_id(), alice_device_id());
account.update_uploaded_key_count(49);
account.generate_one_time_keys();
account.generate_one_time_keys_if_needed();

let mut one_time_keys = account.signed_one_time_keys();
let ed25519_key = account.identity_keys().ed25519;
Expand Down
55 changes: 29 additions & 26 deletions crates/matrix-sdk-crypto/src/olm/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ impl Account {
}

/// Generate count number of one-time keys.
pub fn generate_one_time_keys_helper(&mut self, count: usize) -> OneTimeKeyGenerationResult {
pub fn generate_one_time_keys(&mut self, count: usize) -> OneTimeKeyGenerationResult {
self.inner.generate_one_time_keys(count)
}

Expand Down Expand Up @@ -493,7 +493,7 @@ impl Account {
}

self.update_uploaded_key_count(count);
self.generate_one_time_keys();
self.generate_one_time_keys_if_needed();
}

if let Some(unused) = unused_fallback_keys {
Expand All @@ -513,34 +513,34 @@ impl Account {
/// Generally `Some` means that keys should be uploaded, while `None` means
/// that keys should not be uploaded.
#[instrument(skip_all)]
pub fn generate_one_time_keys(&mut self) -> Option<u64> {
pub fn generate_one_time_keys_if_needed(&mut self) -> Option<u64> {
// Only generate one-time keys if there aren't any, otherwise the caller
// might have failed to upload them the last time this method was
// called.
if self.one_time_keys().is_empty() {
let count = self.uploaded_key_count();
let max_keys = self.max_one_time_keys();
if !self.one_time_keys().is_empty() {
return Some(0);
}

if count >= max_keys as u64 {
return None;
}
let count = self.uploaded_key_count();
let max_keys = self.max_one_time_keys();

let key_count = (max_keys as u64) - count;
let key_count: usize = key_count.try_into().unwrap_or(max_keys);
if count >= max_keys as u64 {
return None;
}

let result = self.generate_one_time_keys_helper(key_count);
let key_count = (max_keys as u64) - count;
let key_count: usize = key_count.try_into().unwrap_or(max_keys);

debug!(
count = key_count,
discarded_keys = ?result.removed,
created_keys = ?result.created,
"Generated new one-time keys"
);
let result = self.generate_one_time_keys(key_count);

Some(key_count as u64)
} else {
Some(0)
}
debug!(
count = key_count,
discarded_keys = ?result.removed,
created_keys = ?result.created,
"Generated new one-time keys"
);

Some(key_count as u64)
}

pub(crate) fn generate_fallback_key_helper(&mut self) {
Expand Down Expand Up @@ -980,10 +980,13 @@ impl Account {
#[cfg(any(test, feature = "testing"))]
#[allow(dead_code)]
/// Testing only helper to create a session for the given Account
pub async fn create_session_for(&mut self, other: &mut Account) -> (Session, Session) {
pub async fn create_session_for_test_helper(
&mut self,
other: &mut Account,
) -> (Session, Session) {
use ruma::events::dummy::ToDeviceDummyEventContent;

other.generate_one_time_keys_helper(1);
other.generate_one_time_keys(1);
let one_time_map = other.signed_one_time_keys();
let device = ReadOnlyDevice::from_account(other);

Expand Down Expand Up @@ -1427,13 +1430,13 @@ mod tests {

account.mark_keys_as_published();
account.update_uploaded_key_count(50);
account.generate_one_time_keys();
account.generate_one_time_keys_if_needed();

let (_, third_one_time_keys, _) = account.keys_for_upload();
assert!(third_one_time_keys.is_empty());

account.update_uploaded_key_count(0);
account.generate_one_time_keys();
account.generate_one_time_keys_if_needed();

let (_, fourth_one_time_keys, _) = account.keys_for_upload();
assert!(!fourth_one_time_keys.is_empty());
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk-crypto/src/olm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub(crate) mod tests {
let alice = Account::with_device_id(alice_id(), alice_device_id());
let mut bob = Account::with_device_id(bob_id(), bob_device_id());

bob.generate_one_time_keys_helper(1);
bob.generate_one_time_keys(1);
let one_time_key = *bob.one_time_keys().values().next().unwrap();
let sender_key = bob.identity_keys().curve25519;
let session = alice.create_outbound_session_helper(
Expand Down Expand Up @@ -116,7 +116,7 @@ pub(crate) mod tests {
assert!(!one_time_keys.is_empty());
assert_ne!(account.max_one_time_keys(), 0);

account.generate_one_time_keys_helper(10);
account.generate_one_time_keys(10);
let one_time_keys = account.one_time_keys();

assert_ne!(one_time_keys.values().len(), 0);
Expand All @@ -133,7 +133,7 @@ pub(crate) mod tests {
let mut alice = Account::with_device_id(alice_id(), alice_device_id());
let bob = Account::with_device_id(bob_id(), bob_device_id());
let alice_keys = alice.identity_keys();
alice.generate_one_time_keys_helper(1);
alice.generate_one_time_keys(1);
let one_time_keys = alice.one_time_keys();
alice.mark_keys_as_published();

Expand Down
8 changes: 4 additions & 4 deletions crates/matrix-sdk-crypto/src/session_manager/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ mod tests {

assert!(request.one_time_keys.contains_key(bob.user_id()));

bob.generate_one_time_keys_helper(1);
bob.generate_one_time_keys(1);
let one_time = bob.signed_one_time_keys();
assert!(!one_time.is_empty());
bob.mark_keys_as_published();
Expand Down Expand Up @@ -854,7 +854,7 @@ mod tests {
.store
.with_transaction(|mut tr| async {
let manager_account = tr.account().await.unwrap();
let res = bob.create_session_for(manager_account).await;
let res = bob.create_session_for_test_helper(manager_account).await;
Ok((tr, res))
})
.await
Expand Down Expand Up @@ -882,7 +882,7 @@ mod tests {

assert!(request.one_time_keys.contains_key(bob.user_id()));

bob.generate_one_time_keys_helper(1);
bob.generate_one_time_keys(1);
let one_time = bob.signed_one_time_keys();
assert!(!one_time.is_empty());
bob.mark_keys_as_published();
Expand Down Expand Up @@ -1009,7 +1009,7 @@ mod tests {
// Since alice is timed out, we won't claim keys for her.
assert!(manager.get_missing_sessions(iter::once(alice)).await.unwrap().is_none());

alice_account.generate_one_time_keys_helper(1);
alice_account.generate_one_time_keys(1);
let one_time = alice_account.signed_one_time_keys();
assert!(!one_time.is_empty());

Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-crypto/src/store/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ macro_rules! cryptostore_integration_tests {
let alice = Account::with_device_id(alice_id(), alice_device_id());
let mut bob = Account::with_device_id(bob_id(), bob_device_id());

bob.generate_one_time_keys_helper(1);
bob.generate_one_time_keys(1);
let one_time_key = *bob.one_time_keys().values().next().unwrap();
let sender_key = bob.identity_keys().curve25519;
let session = alice
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/sliding_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ impl SlidingSync {
let client = self.inner.client.clone();
let e2ee_uploads = spawn(async move {
if let Err(error) = client.send_outgoing_requests().await {
error!(?error, "Error while sending outoging E2EE requests");
error!(?error, "Error while sending outgoing E2EE requests");
}
})
// Ensure that the task is not running in detached mode. It is aborted when it's
Expand Down