Skip to content

Commit

Permalink
crypto: key sharing error for verified user with unverified devices
Browse files Browse the repository at this point in the history
  • Loading branch information
BillCarsonFr committed Aug 7, 2024
1 parent bd7711e commit 6e6f71d
Show file tree
Hide file tree
Showing 9 changed files with 647 additions and 40 deletions.
4 changes: 3 additions & 1 deletion bindings/matrix-sdk-crypto-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,8 @@ pub struct EncryptionSettings {
/// Should untrusted devices receive the room key, or should they be
/// excluded from the conversation.
pub only_allow_trusted_devices: bool,
/// Should fail to send when a verified user has unverified devices.
pub error_on_unsigned_device_of_verified_users: bool,
}

impl From<EncryptionSettings> for RustEncryptionSettings {
Expand All @@ -677,7 +679,7 @@ impl From<EncryptionSettings> for RustEncryptionSettings {
rotation_period: Duration::from_secs(v.rotation_period),
rotation_period_msgs: v.rotation_period_msgs,
history_visibility: v.history_visibility.into(),
sharing_strategy: CollectStrategy::new_device_based(v.only_allow_trusted_devices),
sharing_strategy: CollectStrategy::new_device_based(v.only_allow_trusted_devices, v.error_on_unsigned_device_of_verified_users),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@ impl BaseClient {
let members = self.store.get_user_ids(room_id, filter).await?;

let settings = settings.ok_or(Error::EncryptionNotEnabled)?;
let settings = EncryptionSettings::new(settings, history_visibility, false);
let settings = EncryptionSettings::new(settings, history_visibility, false, false);

Ok(o.share_room_key(room_id, members.iter().map(Deref::deref), settings).await?)
}
Expand Down
20 changes: 20 additions & 0 deletions crates/matrix-sdk-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::BTreeMap;

use ruma::{CanonicalJsonError, IdParseError, OwnedDeviceId, OwnedRoomId, OwnedUserId};
use serde::{ser::SerializeMap, Serializer};
use serde_json::Error as SerdeError;
Expand Down Expand Up @@ -70,8 +72,26 @@ pub enum OlmError {
have a valid Olm session with us"
)]
MissingSession,

#[error(transparent)]
/// The room key that was to be shared was not shared because the sharing
/// strategy could not be fulfilled.
RoomKeySharingStrategyError(DeviceCollectError),
}

/// Depending on the sharing strategy for room keys, the distribution of the
/// room key could fail.
#[derive(Error, Debug)]
pub enum DeviceCollectError {
/// When a verified user has some unsigned devices.
/// Happens only with `CollectStrategy::DeviceBasedStrategy` when
/// `error_on_unsigned_device_of_verified_users` is true.
///
/// In order to resolve this the caller has to ignore or blacklist the given
/// devices (see [`Device::set_local_trust`]).
#[error("Encryption failed because one or several verified users have unsigned devices")]
DeviceBasedVerifiedUserHasUnsignedDevice(BTreeMap<OwnedUserId, Vec<OwnedDeviceId>>),
}
/// Error representing a failure during a group encryption operation.
#[derive(Error, Debug)]
pub enum MegolmError {
Expand Down
8 changes: 8 additions & 0 deletions crates/matrix-sdk-crypto/src/identities/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,14 @@ impl DeviceData {
self.local_trust_state() == LocalTrust::BlackListed
}

/// Is the device locally marked as whitelisted.
///
/// Whitelisted devices will receive group sessions regardless of their
/// verification status.
pub fn is_whitelisted(&self) -> bool {
self.local_trust_state() == LocalTrust::Ignored
}

/// Set the trust state of the device to the given state.
///
/// Note: This should only done in the crypto store where the trust state
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-crypto/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3314,7 +3314,7 @@ pub(crate) mod tests {

let encryption_settings = EncryptionSettings::default();
let encryption_settings = EncryptionSettings {
sharing_strategy: CollectStrategy::new_device_based(true),
sharing_strategy: CollectStrategy::new_device_based(true, false),
..encryption_settings
};

Expand Down
11 changes: 8 additions & 3 deletions crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ impl EncryptionSettings {
content: RoomEncryptionEventContent,
history_visibility: HistoryVisibility,
only_allow_trusted_devices: bool,
error_on_unsigned_device_of_verified_users: bool,
) -> Self {
let rotation_period: Duration =
content.rotation_period_ms.map_or(ROTATION_PERIOD, |r| Duration::from_millis(r.into()));
Expand All @@ -133,7 +134,10 @@ impl EncryptionSettings {
rotation_period,
rotation_period_msgs,
history_visibility,
sharing_strategy: CollectStrategy::new_device_based(only_allow_trusted_devices),
sharing_strategy: CollectStrategy::new_device_based(
only_allow_trusted_devices,
error_on_unsigned_device_of_verified_users,
),
}
}
}
Expand Down Expand Up @@ -787,15 +791,16 @@ mod tests {
fn test_encryption_settings_conversion() {
let mut content =
RoomEncryptionEventContent::new(EventEncryptionAlgorithm::MegolmV1AesSha2);
let settings = EncryptionSettings::new(content.clone(), HistoryVisibility::Joined, false);
let settings =
EncryptionSettings::new(content.clone(), HistoryVisibility::Joined, false, false);

assert_eq!(settings.rotation_period, ROTATION_PERIOD);
assert_eq!(settings.rotation_period_msgs, ROTATION_MESSAGES);

content.rotation_period_ms = Some(uint!(3600));
content.rotation_period_msgs = Some(uint!(500));

let settings = EncryptionSettings::new(content, HistoryVisibility::Shared, false);
let settings = EncryptionSettings::new(content, HistoryVisibility::Shared, false, false);

assert_eq!(settings.rotation_period, Duration::from_millis(3600));
assert_eq!(settings.rotation_period_msgs, 500);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ mod tests {
.any(|d| d.user_id() == user_id && d.device_id() == device_id));

let settings = EncryptionSettings {
sharing_strategy: CollectStrategy::new_device_based(true),
sharing_strategy: CollectStrategy::new_device_based(true, false),
..Default::default()
};
let users = [user_id].into_iter();
Expand Down Expand Up @@ -1235,7 +1235,7 @@ mod tests {

let users = keys_claim.one_time_keys.keys().map(Deref::deref);
let settings = EncryptionSettings {
sharing_strategy: CollectStrategy::new_device_based(true),
sharing_strategy: CollectStrategy::new_device_based(true, false),
..Default::default()
};

Expand Down
Loading

0 comments on commit 6e6f71d

Please sign in to comment.