crypto: Check that the other device had a valid MAC.

matrix_sdk_crypto/src/verification/sas/helpers.rs

@@ -142,9 +142,9 @@ pub fn receive_mac_event(
     ids: &SasIds,
     flow_id: &str,
     event: &ToDeviceEvent<MacEventContent>,
-) -> (Vec<Box<DeviceId>>, Vec<String>) {
+) -> (Vec<Device>, Vec<String>) {
     // TODO check the event and cancel if it isn't ok (sender, transaction id)
-    let mut verified_devices: Vec<Box<DeviceId>> = Vec::new();
+    let mut verified_devices = Vec::new();
 
     let info = extra_mac_info_receive(&ids, flow_id);
 
@@ -180,7 +180,7 @@ pub fn receive_mac_event(
                     .calculate_mac(key, &format!("{}{}", info, key_id))
                     .expect("Can't calculate SAS MAC")
             {
-                verified_devices.push(ids.other_device.device_id().into());
+                verified_devices.push(ids.other_device.clone());
             } else {
                 // TODO cancel here
             }

matrix_sdk_crypto/src/verification/sas/mod.rs

@@ -182,16 +182,30 @@ impl Sas {
 
         if let Some(device) = device {
             if device.keys() == self.other_device.keys() {
-                trace!(
-                    "Marking device {} {} as verified.",
-                    device.user_id(),
-                    device.device_id()
-                );
+                if self
+                    .verified_devices()
+                    .map_or(false, |v| v.contains(&device))
+                {
+                    trace!(
+                        "Marking device {} {} as verified.",
+                        device.user_id(),
+                        device.device_id()
+                    );
 
-                device.set_trust_state(TrustState::Verified);
-                self.store.read().await.save_devices(&[device]).await?;
+                    device.set_trust_state(TrustState::Verified);
+                    self.store.read().await.save_devices(&[device]).await?;
 
-                Ok(true)
+                    Ok(true)
+                } else {
+                    info!(
+                        "The interactive verification process didn't contain a \
+                        MAC for the device {} {}",
+                        device.user_id(),
+                        device.device_id()
+                    );
+
+                    Ok(false)
+                }
             } else {
                 warn!(
                     "The device keys of {} {} have changed while an interactive \
@@ -273,7 +287,7 @@ impl Sas {
         content
     }
 
-    pub(crate) fn verified_devices(&self) -> Option<Arc<Vec<Box<DeviceId>>>> {
+    pub(crate) fn verified_devices(&self) -> Option<Arc<Vec<Device>>> {
         self.inner.lock().unwrap().verified_devices()
     }
 
@@ -472,7 +486,7 @@ impl InnerSas {
         }
     }
 
-    fn verified_devices(&self) -> Option<Arc<Vec<Box<DeviceId>>>> {
+    fn verified_devices(&self) -> Option<Arc<Vec<Device>>> {
         if let InnerSas::Done(s) = self {
             Some(s.verified_devices())
         } else {
@@ -609,8 +623,8 @@ mod test {
         let event = wrap_to_device_event(alice.user_id(), alice.as_content());
         let bob = bob.into_done(&event).unwrap();
 
-        assert!(bob.verified_devices().contains(&alice.device_id().into()));
-        assert!(alice.verified_devices().contains(&bob.device_id().into()));
+        assert!(bob.verified_devices().contains(&bob.other_device()));
+        assert!(alice.verified_devices().contains(&alice.other_device()));
     }
 
     #[tokio::test]
@@ -674,10 +688,10 @@ mod test {
         assert!(alice
             .verified_devices()
             .unwrap()
-            .contains(&bob.device_id().into()));
+            .contains(&alice.other_device()));
         assert!(bob
             .verified_devices()
             .unwrap()
-            .contains(&alice.device_id().into()));
+            .contains(&bob.other_device()));
     }
 }

matrix_sdk_crypto/src/verification/sas/sas_state.rs

@@ -165,7 +165,7 @@ pub struct Confirmed {
 #[derive(Clone, Debug)]
 pub struct MacReceived {
     we_started: bool,
-    verified_devices: Arc<Vec<Box<DeviceId>>>,
+    verified_devices: Arc<Vec<Device>>,
     verified_master_keys: Arc<Vec<String>>,
 }
 
@@ -175,7 +175,7 @@ pub struct MacReceived {
 /// the master keys in the verified devices list.
 #[derive(Clone, Debug)]
 pub struct Done {
-    verified_devices: Arc<Vec<Box<DeviceId>>>,
+    verified_devices: Arc<Vec<Device>>,
     verified_master_keys: Arc<Vec<String>>,
 }
 
@@ -196,6 +196,10 @@ impl<S: Clone> SasState<S> {
         &self.ids.account.device_id()
     }
 
+    pub fn other_device(&self) -> Device {
+        self.ids.other_device.clone()
+    }
+
     pub fn cancel(self, cancel_code: CancelCode) -> SasState<Canceled> {
         SasState {
             inner: self.inner,
@@ -691,7 +695,7 @@ impl SasState<Done> {
     }
 
     /// Get the list of verified devices.
-    pub fn verified_devices(&self) -> Arc<Vec<Box<DeviceId>>> {
+    pub fn verified_devices(&self) -> Arc<Vec<Device>> {
         self.state.verified_devices.clone()
     }
 
@@ -853,7 +857,7 @@ mod test {
         let event = wrap_to_device_event(alice.user_id(), alice.as_content());
         let bob = bob.into_done(&event).unwrap();
 
-        assert!(bob.verified_devices().contains(&alice.device_id().into()));
-        assert!(alice.verified_devices().contains(&bob.device_id().into()));
+        assert!(bob.verified_devices().contains(&bob.other_device()));
+        assert!(alice.verified_devices().contains(&alice.other_device()));
     }
 }