improvement: check signatures on join

src/client_server/membership.rs

@@ -2,7 +2,7 @@ use super::State;
 use crate::{
     client_server,
     pdu::{PduBuilder, PduEvent},
-    utils, ConduitResult, Database, Error, Result, Ruma,
+    server_server, utils, ConduitResult, Database, Error, Result, Ruma,
 };
 use log::{error, warn};
 use ruma::{
@@ -21,7 +21,7 @@ use ruma::{
     serde::{to_canonical_value, CanonicalJsonObject, Raw},
     EventId, RoomId, RoomVersionId, ServerName, UserId,
 };
-use std::{collections::BTreeMap, convert::TryFrom, sync::Arc};
+use std::{collections::BTreeMap, convert::TryFrom};
 
 #[cfg(feature = "conduit_bin")]
 use rocket::{get, post};
@@ -515,27 +515,6 @@ async fn join_room_by_id_helper(
             )
             .await?;
 
-        let add_event_id = |pdu: &Raw<Pdu>| -> Result<(EventId, CanonicalJsonObject)> {
-            let mut value = serde_json::from_str(pdu.json().get()).map_err(|e| {
-                error!("{:?}: {:?}", pdu, e);
-                Error::BadServerResponse("Invalid PDU in server response")
-            })?;
-            let event_id = EventId::try_from(&*format!(
-                "${}",
-                ruma::signatures::reference_hash(&value, &room_version)
-                    .expect("ruma can calculate reference hashes")
-            ))
-            .expect("ruma's reference hashes are valid event ids");
-
-            value.insert(
-                "event_id".to_owned(),
-                to_canonical_value(&event_id)
-                    .expect("a valid EventId can be converted to CanonicalJsonValue"),
-            );
-
-            Ok((event_id, value))
-        };
-
         let count = db.globals.next_count()?;
 
         let mut pdu_id = room_id.as_bytes().to_vec();
@@ -546,23 +525,15 @@ async fn join_room_by_id_helper(
             .map_err(|_| Error::BadServerResponse("Invalid PDU in send_join response."))?;
 
         let mut state = BTreeMap::new();
+        let mut pub_key_map = BTreeMap::new();
+
+        for pdu in send_join_response.room_state.state.iter() {
+            let (event_id, value) = validate_and_add_event_id(pdu, &room_version, &mut pub_key_map, &db).await?;
+            let pdu = PduEvent::from_id_val(&event_id, value.clone()).map_err(|e| {
+                warn!("{:?}: {}", value, e);
+                Error::BadServerResponse("Invalid PDU in send_join response.")
+            })?;
 
-        for pdu in send_join_response
-            .room_state
-            .state
-            .iter()
-            .map(add_event_id)
-            .map(|r| {
-                let (event_id, value) = r?;
-                PduEvent::from_id_val(&event_id, value.clone())
-                    .map(|ev| (event_id, Arc::new(ev)))
-                    .map_err(|e| {
-                        warn!("{:?}: {}", value, e);
-                        Error::BadServerResponse("Invalid PDU in send_join response.")
-                    })
-            })
-        {
-            let (_id, pdu) = pdu?;
             db.rooms.add_pdu_outlier(&pdu)?;
             if let Some(state_key) = &pdu.state_key {
                 if pdu.kind == EventType::RoomMember {
@@ -612,22 +583,12 @@ async fn join_room_by_id_helper(
 
         db.rooms.force_state(room_id, state, &db.globals)?;
 
-        for pdu in send_join_response
+        for pdu in send_join_response.room_state.auth_chain.iter() {
-            .room_state
+            let (event_id, value) = validate_and_add_event_id(pdu, &room_version, &mut pub_key_map, &db).await?;
-            .auth_chain
+            let pdu = PduEvent::from_id_val(&event_id, value.clone()).map_err(|e| {
-            .iter()
+                warn!("{:?}: {}", value, e);
-            .map(add_event_id)
+                Error::BadServerResponse("Invalid PDU in send_join response.")
-            .map(|r| {
+            })?;
-                let (event_id, value) = r?;
-                PduEvent::from_id_val(&event_id, value.clone())
-                    .map(|ev| (event_id, Arc::new(ev)))
-                    .map_err(|e| {
-                        warn!("{:?}: {}", value, e);
-                        Error::BadServerResponse("Invalid PDU in send_join response.")
-                    })
-            })
-        {
-            let (_id, pdu) = pdu?;
             db.rooms.add_pdu_outlier(&pdu)?;
         }
 
@@ -674,3 +635,32 @@ async fn join_room_by_id_helper(
 
     Ok(join_room_by_id::Response::new(room_id.clone()).into())
 }
+
+async fn validate_and_add_event_id(
+    pdu: &Raw<Pdu>,
+    room_version: &RoomVersionId,
+    pub_key_map: &mut BTreeMap<String, BTreeMap<String, String>>,
+    db: &Database,
+) -> Result<(EventId, CanonicalJsonObject)> {
+    let mut value = serde_json::from_str::<CanonicalJsonObject>(pdu.json().get()).map_err(|e| {
+        error!("{:?}: {:?}", pdu, e);
+        Error::BadServerResponse("Invalid PDU in server response")
+    })?;
+
+    server_server::fetch_required_signing_keys(&value, pub_key_map, db).await?;
+
+    let event_id = EventId::try_from(&*format!(
+        "${}",
+        ruma::signatures::reference_hash(&value, &room_version)
+            .expect("ruma can calculate reference hashes")
+    ))
+    .expect("ruma's reference hashes are valid event ids");
+
+    value.insert(
+        "event_id".to_owned(),
+        to_canonical_value(&event_id)
+            .expect("a valid EventId can be converted to CanonicalJsonValue"),
+    );
+
+    Ok((event_id, value))
+}

src/server_server.rs

@@ -658,44 +658,7 @@ fn handle_incoming_pdu<'a>(
 
         // We go through all the signatures we see on the value and fetch the corresponding signing
         // keys
-        for (signature_server, signature) in match value
+        fetch_required_signing_keys(&value, pub_key_map, db).await.map_err(|e| e.to_string())?;
-            .get("signatures")
-            .ok_or_else(|| "No signatures in server response pdu.".to_string())?
-        {
-            CanonicalJsonValue::Object(map) => map,
-            _ => return Err("Invalid signatures object in server response pdu.".to_string()),
-        } {
-            let signature_object = match signature {
-                CanonicalJsonValue::Object(map) => map,
-                _ => {
-                    return Err(
-                        "Invalid signatures content object in server response pdu.".to_string()
-                    )
-                }
-            };
-
-            let signature_ids = signature_object.keys().collect::<Vec<_>>();
-
-            debug!("Fetching signing keys for {}", signature_server);
-            let keys = match fetch_signing_keys(
-                &db,
-                &Box::<ServerName>::try_from(&**signature_server).map_err(|_| {
-                    "Invalid servername in signatures of server response pdu.".to_string()
-                })?,
-                signature_ids,
-            )
-            .await
-            {
-                Ok(keys) => keys,
-                Err(_) => {
-                    return Err(
-                        "Signature verification failed: Could not fetch signing key.".to_string(),
-                    );
-                }
-            };
-
-            pub_key_map.insert(signature_server.clone(), keys);
-        }
 
         // 2. Check signatures, otherwise drop
         // 3. check content hash, redact if doesn't match
@@ -1639,38 +1602,58 @@ pub fn get_profile_information_route<'a>(
     .into())
 }
 
-/*
+pub async fn fetch_required_signing_keys(
-#[cfg_attr(
+    event: &BTreeMap<String, CanonicalJsonValue>,
-    feature = "conduit_bin",
+    pub_key_map: &mut BTreeMap<String, BTreeMap<String, String>>,
-    get("/_matrix/federation/v2/invite/<_>/<_>", data = "<body>")
+    db: &Database,
-)]
+) -> Result<()> {
-pub fn get_user_devices_route<'a>(
+    // We go through all the signatures we see on the value and fetch the corresponding signing
-    db: State<'a, Database>,
+    // keys
-    body: Ruma<membership::v1::Request<'_>>,
+    for (signature_server, signature) in match event
-) -> ConduitResult<get_profile_information::v1::Response> {
+        .get("signatures")
-    if !db.globals.allow_federation() {
+        .ok_or_else(|| Error::BadServerResponse("No signatures in server response pdu."))?
-        return Err(Error::bad_config("Federation is disabled."));
+    {
-    }
+        CanonicalJsonValue::Object(map) => map,
-
+        _ => {
-    let mut displayname = None;
+            return Err(Error::BadServerResponse(
-    let mut avatar_url = None;
+                "Invalid signatures object in server response pdu.",
-
+            ))
-    match body.field {
-        Some(ProfileField::DisplayName) => displayname = db.users.displayname(&body.user_id)?,
-        Some(ProfileField::AvatarUrl) => avatar_url = db.users.avatar_url(&body.user_id)?,
-        None => {
-            displayname = db.users.displayname(&body.user_id)?;
-            avatar_url = db.users.avatar_url(&body.user_id)?;
         }
+    } {
+        let signature_object = match signature {
+            CanonicalJsonValue::Object(map) => map,
+            _ => {
+                return Err(Error::BadServerResponse(
+                    "Invalid signatures content object in server response pdu.",
+                ))
+            }
+        };
+
+        let signature_ids = signature_object.keys().collect::<Vec<_>>();
+
+        debug!("Fetching signing keys for {}", signature_server);
+        let keys = match fetch_signing_keys(
+            db,
+            &Box::<ServerName>::try_from(&**signature_server).map_err(|_| {
+                Error::BadServerResponse("Invalid servername in signatures of server response pdu.")
+            })?,
+            signature_ids,
+        )
+        .await
+        {
+            Ok(keys) => keys,
+            Err(_) => {
+                return Err(Error::BadServerResponse(
+                    "Signature verification failed: Could not fetch signing key.",
+                ));
+            }
+        };
+
+        pub_key_map.insert(signature_server.clone(), keys);
     }
 
-    Ok(get_profile_information::v1::Response {
+    Ok(())
-        displayname,
-        avatar_url,
-    }
-    .into())
 }
-*/
 
 #[cfg(test)]
 mod tests {