refactor(crypto): Remove the forwarding chains

These aren't really useful since they can be easily spoofed by any room
key forwarder.
This commit is contained in:
Damir Jelić
2022-09-12 13:00:09 +02:00
parent d1b2bfcaa4
commit aac41fc82a
10 changed files with 22 additions and 73 deletions

View File

@@ -251,8 +251,6 @@ pub fn migrate(
InboundGroupSession::from_libolm_pickle(&session.pickle, &data.pickle_key)?.pickle();
let sender_key = Curve25519PublicKey::from_base64(&session.sender_key)?;
let forwarding_chains: Result<Vec<Curve25519PublicKey>, _> =
session.forwarding_chains.iter().map(|k| Curve25519PublicKey::from_base64(k)).collect();
let pickle = matrix_sdk_crypto::olm::PickledInboundGroupSession {
pickle,
@@ -268,7 +266,6 @@ pub fn migrate(
})
.collect::<anyhow::Result<_>>()?,
room_id: RoomId::parse(session.room_id)?,
forwarding_chains: forwarding_chains?,
imported: session.imported,
backed_up: session.backed_up,
history_visibility: None,

View File

@@ -610,16 +610,16 @@ impl OlmMachine {
let event_json: Event<'_> = serde_json::from_str(decrypted.event.json().get())?;
Ok(match &encryption_info.algorithm_info {
AlgorithmInfo::MegolmV1AesSha2 {
curve25519_key,
sender_claimed_keys,
forwarding_curve25519_key_chain,
} => DecryptedEvent {
clear_event: serde_json::to_string(&event_json)?,
sender_curve25519_key: curve25519_key.to_owned(),
claimed_ed25519_key: sender_claimed_keys.get(&DeviceKeyAlgorithm::Ed25519).cloned(),
forwarding_curve25519_chain: forwarding_curve25519_key_chain.to_owned(),
},
AlgorithmInfo::MegolmV1AesSha2 { curve25519_key, sender_claimed_keys } => {
DecryptedEvent {
clear_event: serde_json::to_string(&event_json)?,
sender_curve25519_key: curve25519_key.to_owned(),
claimed_ed25519_key: sender_claimed_keys
.get(&DeviceKeyAlgorithm::Ed25519)
.cloned(),
forwarding_curve25519_chain: vec![],
}
}
})
}

View File

@@ -182,12 +182,8 @@ impl DecryptedRoomEvent {
/// Chain of Curve25519 keys through which this session was
/// forwarded, via `m.forwarded_room_key` events.
#[wasm_bindgen(getter, js_name = "forwardingCurve25519KeyChain")]
pub fn forwarding_curve25519_key_chain(&self) -> Option<Array> {
Some(match &self.encryption_info.as_ref()?.algorithm_info {
AlgorithmInfo::MegolmV1AesSha2 { forwarding_curve25519_key_chain, .. } => {
forwarding_curve25519_key_chain.iter().map(JsValue::from).collect()
}
})
pub fn forwarding_curve25519_key_chain(&self) -> Array {
Array::new()
}
/// The verification state of the device that sent us the event,

View File

@@ -178,12 +178,8 @@ impl DecryptedRoomEvent {
/// Chain of Curve25519 keys through which this session was
/// forwarded, via `m.forwarded_room_key` events.
#[napi(getter)]
pub fn forwarding_curve25519_key_chain(&self) -> Option<Vec<String>> {
Some(match &self.encryption_info.as_ref()?.algorithm_info {
AlgorithmInfo::MegolmV1AesSha2 { forwarding_curve25519_key_chain, .. } => {
forwarding_curve25519_key_chain.clone()
}
})
pub fn forwarding_curve25519_key_chain(&self) -> Vec<String> {
vec![]
}
/// The verification state of the device that sent us the event,

View File

@@ -68,9 +68,6 @@ pub enum AlgorithmInfo {
/// decrypt this session. This map will usually contain a single ed25519
/// key.
sender_claimed_keys: BTreeMap<DeviceKeyAlgorithm, String>,
/// Chain of curve25519 keys through which this session was forwarded,
/// via m.forwarded_room_key events.
forwarding_curve25519_key_chain: Vec<String>,
},
}

View File

@@ -112,9 +112,9 @@ impl MegolmV1BackupKey {
pub(crate) async fn encrypt(&self, session: InboundGroupSession) -> KeyBackupData {
let pk = OlmPkEncryption::new(&self.to_base64());
// It's ok to truncate here, there's a semantic difference only between
// 0 and 1+ anyways.
let forwarded_count = (session.forwarding_key_chain().len() as u32).into();
// The forwarding chains don't mean much, we only care whether we received the
// session directly from the creator of the session or not.
let forwarded_count = (session.has_been_imported() as u8).into();
let first_message_index = session.first_known_index().into();
// Convert our key to the backup representation.

View File

@@ -879,7 +879,7 @@ impl GossipMachine {
algorithm: EventEncryptionAlgorithm,
content: &ForwardedMegolmV1AesSha2Content,
) -> Result<Option<InboundGroupSession>, CryptoStoreError> {
match InboundGroupSession::from_forwarded_key(sender_key, &algorithm, content) {
match InboundGroupSession::from_forwarded_key(&algorithm, content) {
Ok(session) => {
let old_session = self
.store

View File

@@ -1068,11 +1068,6 @@ impl OlmMachine {
.iter()
.map(|(k, v)| (k.to_owned(), v.to_base64()))
.collect(),
forwarding_curve25519_key_chain: session
.forwarding_key_chain()
.iter()
.map(|k| k.to_base64())
.collect(),
},
verification_state,
})

View File

@@ -42,12 +42,12 @@ use super::{
use crate::{
error::{EventError, MegolmResult},
types::{
deserialize_curve_key, deserialize_curve_key_vec,
deserialize_curve_key,
events::{
forwarded_room_key::ForwardedMegolmV1AesSha2Content,
room::encrypted::{EncryptedEvent, RoomEventEncryptionScheme},
},
serialize_curve_key, serialize_curve_key_vec, EventEncryptionAlgorithm, SigningKeys,
serialize_curve_key, EventEncryptionAlgorithm, SigningKeys,
},
};
@@ -72,7 +72,6 @@ pub struct InboundGroupSession {
pub signing_keys: Arc<SigningKeys<DeviceKeyAlgorithm>>,
/// The Room this GroupSession belongs to
pub room_id: Arc<RoomId>,
forwarding_chains: Arc<Vec<Curve25519PublicKey>>,
imported: bool,
algorithm: Arc<EventEncryptionAlgorithm>,
backed_up: Arc<AtomicBool>,
@@ -120,7 +119,6 @@ impl InboundGroupSession {
sender_key,
signing_keys: keys.into(),
room_id: room_id.into(),
forwarding_chains: Vec::new().into(),
imported: false,
algorithm: encryption_algorithm.into(),
backed_up: AtomicBool::new(false).into(),
@@ -153,9 +151,9 @@ impl InboundGroupSession {
room_id: room_id.to_owned(),
sender_key: backup.sender_key,
session_id,
forwarding_curve25519_key_chain: vec![],
session_key: backup.session_key,
sender_claimed_keys: backup.sender_claimed_keys,
forwarding_curve25519_key_chain: backup.forwarding_curve25519_key_chain,
})
}
@@ -169,7 +167,6 @@ impl InboundGroupSession {
/// * `content` - A forwarded room key content that contains the session key
/// to create the `InboundGroupSession`.
pub fn from_forwarded_key(
sender_key: Curve25519PublicKey,
algorithm: &EventEncryptionAlgorithm,
content: &ForwardedMegolmV1AesSha2Content,
) -> Result<Self, SessionCreationError> {
@@ -178,8 +175,6 @@ impl InboundGroupSession {
let session = InnerSession::import(&content.session_key, config);
let first_known_index = session.first_known_index();
let mut forwarding_chains = content.forwarding_curve25519_key_chain.clone();
forwarding_chains.push(sender_key);
let mut sender_claimed_key = SigningKeys::new();
sender_claimed_key.insert(DeviceKeyAlgorithm::Ed25519, content.claimed_ed25519_key.into());
@@ -192,7 +187,6 @@ impl InboundGroupSession {
history_visibility: None.into(),
signing_keys: sender_claimed_key.into(),
room_id: (*content.room_id).into(),
forwarding_chains: forwarding_chains.into(),
imported: true,
backed_up: AtomicBool::new(false).into(),
algorithm: algorithm.to_owned().into(),
@@ -213,7 +207,6 @@ impl InboundGroupSession {
sender_key: self.sender_key,
signing_key: (*self.signing_keys).clone(),
room_id: (*self.room_id).to_owned(),
forwarding_chains: self.forwarding_key_chain().to_vec(),
imported: self.imported,
backed_up: self.backed_up(),
history_visibility: self.history_visibility.as_ref().clone(),
@@ -255,15 +248,6 @@ impl InboundGroupSession {
&self.signing_keys
}
/// Get the list of ed25519 keys that this session was forwarded through.
///
/// Each ed25519 key represents a single device. If device A forwards the
/// session to device B and device B to C this list will contain the ed25519
/// keys of A and B.
pub fn forwarding_key_chain(&self) -> &[Curve25519PublicKey] {
&self.forwarding_chains
}
/// Export this session at the given message index.
pub async fn export_at_index(&self, message_index: u32) -> ExportedRoomKey {
let message_index = std::cmp::max(self.first_known_index(), message_index);
@@ -276,7 +260,7 @@ impl InboundGroupSession {
room_id: (*self.room_id).to_owned(),
sender_key: self.sender_key,
session_id: self.session_id().to_owned(),
forwarding_curve25519_key_chain: self.forwarding_key_chain().to_vec(),
forwarding_curve25519_key_chain: vec![],
sender_claimed_keys: (*self.signing_keys).clone(),
session_key,
}
@@ -306,7 +290,6 @@ impl InboundGroupSession {
first_known_index,
signing_keys: pickle.signing_key.into(),
room_id: (*pickle.room_id).into(),
forwarding_chains: pickle.forwarding_chains.into(),
backed_up: AtomicBool::from(pickle.backed_up).into(),
algorithm: pickle.algorithm.into(),
imported: pickle.imported,
@@ -463,14 +446,6 @@ pub struct PickledInboundGroupSession {
pub signing_key: SigningKeys<DeviceKeyAlgorithm>,
/// The id of the room that the session is used in.
pub room_id: OwnedRoomId,
/// The list of claimed Curve25519 keys that forwarded us this key. Will be
/// empty if we directly received this session.
#[serde(
default,
deserialize_with = "deserialize_curve_key_vec",
serialize_with = "serialize_curve_key_vec"
)]
pub forwarding_chains: Vec<Curve25519PublicKey>,
/// Flag remembering if the session was directly sent to us by the sender
/// or if it was imported.
pub imported: bool,
@@ -504,7 +479,6 @@ impl TryFrom<&ExportedRoomKey> for InboundGroupSession {
first_known_index,
signing_keys: key.sender_claimed_keys.to_owned().into(),
room_id: key.room_id.to_owned().into(),
forwarding_chains: key.forwarding_curve25519_key_chain.to_owned().into(),
imported: true,
algorithm: key.algorithm.to_owned().into(),
backed_up: AtomicBool::from(false).into(),

View File

@@ -303,11 +303,6 @@ macro_rules! cryptostore_integration_tests {
let mut export = session.export().await;
let curve_key =
Curve25519PublicKey::from_base64("Nn0L2hkcCMFKqynTjyGsJbth7QrVmX3lbrksMkrGOAw")
.unwrap();
export.forwarding_curve25519_key_chain = vec![curve_key];
let session = InboundGroupSession::from_export(&export).unwrap();
let changes =
@@ -332,7 +327,6 @@ macro_rules! cryptostore_integration_tests {
.unwrap();
assert_eq!(session, loaded_session);
let export = loaded_session.export().await;
assert!(!export.forwarding_curve25519_key_chain.is_empty());
assert_eq!(store.get_inbound_group_sessions().await.unwrap().len(), 1);
assert_eq!(store.inbound_group_session_counts().await.unwrap().total, 1);