Option to exclude insecure devices for Olm traffic, to be consistent with megolm sharing?

[BillCarsonFr]

Following the work on invisible crypto it is now possible to exclude devices not signed by their owner from participating in encrypted room:

• When encrypting via CollectStrategy/IdentityBased
• When decrypting via TrustRequirement

We also have an API to encrypt a message directly to a device (using olm):

matrix-rust-sdk/crates/matrix-sdk-crypto/src/identities/device.rs

Lines 407 to 412 in bdfe641

	/// Encrypt the given content for this `Device`.
	///
	/// # Arguments
	///
	/// * `content` - The content of the event that should be encrypted.
	pub(crate) async fn encrypt(

This can lead to a strange situation when the exclude insecure devices option is enabled and:

• Sharing a room key won't work for a given device
• Encrypting directly to this device would work

Maybe we would want to use some EncryptionSetting/DecryptionSetting for olm traffic?
Notice that we also don't look at the device trust status BlackListed/Ignored when encrypting in olm

[fkwp]

@toger5

[dkasak]

I had a short discussion about this with @poljar now.

Ideally we would reject all encrypted to-device message types if the sender device is insecure. However, there are some caveats with some standardised message types:

• Message keys, i.e. m.room_key.
  • If there's a delay in securing the device and you receive a message key in the meantime, we would reject it. But suppose we learn that the device is secured a short time afterwards—now we'd accept the message key we've rejected and the consequence of the rejection is that we'll now get ~100 UTDs from this user.
  • So we accept room keys from insecure devices but keep them in a staged area.
• There are to-device message types we also accept in cleartext. It might be silly to accept them in cleartext but reject them when they're encrypted just because they're coming from an insecure device.
  • These include m.key.verification, m.secret.request, m.room_key.withheld.
  • We accept these from insecure devices.

An improvement we could do is to reject all to-device message types from insecure devices, including custom to-device message types, except for these standardised exceptions. This would amount to a whitelist approach of some standard types.

[andybalaam]

* There are to-device message types we also accept in cleartext...
  
  * These include `m.key.verification`, `m.secret.request`, `m.room_key.withheld`.

I had a look and I can't find where we enforce this in matrix-rust-sdk. I have 2 questions:

1. Is that a complete list of what we should accept in plaintext?
2. Do we explicitly reject other types if we receive them?

@dkasak or cc @poljar

[dkasak]

I believe that is the complete list (but would like @poljar to confirm) and yes, I would like us to reject cleartext messages unless the type is an exception that needs to be cleartext.

[poljar]

Here's the list of events that are accepted in the clear:

matrix-rust-sdk/crates/matrix-sdk-crypto/src/machine/mod.rs

Lines 1331 to 1351 in 737e06b

	async fn handle_to_device_event(&self, changes: &mut Changes, event: &ToDeviceEvents) {
	use crate::types::events::ToDeviceEvents::*;
	match event {
	RoomKeyRequest(e) => self.inner.key_request_machine.receive_incoming_key_request(e),
	SecretRequest(e) => self.inner.key_request_machine.receive_incoming_secret_request(e),
	RoomKeyWithheld(e) => self.add_withheld_info(changes, e),
	KeyVerificationAccept(..)
	| KeyVerificationCancel(..)
	| KeyVerificationKey(..)
	| KeyVerificationMac(..)
	| KeyVerificationRequest(..)
	| KeyVerificationReady(..)
	| KeyVerificationDone(..)
	| KeyVerificationStart(..) => {
	self.handle_verification_event(event).await;
	}
	Dummy(_) | RoomKey(_) | ForwardedRoomKey(_) | RoomEncrypted(_) => {}
	_ => {}
	}
	}

Or rather, anything that has an empty branch isn't handled if it's in the clear.