From f73a657a23be2d3d5690069bd7cc32a4897ee881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20K=C3=B6sters?= Date: Fri, 11 Aug 2023 20:29:22 +0200 Subject: [PATCH 1/2] fix: ACL error shouldn't break the whole request --- src/api/server_server.rs | 5 ----- src/service/rooms/event_handler/mod.rs | 2 ++ 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/api/server_server.rs b/src/api/server_server.rs index 9bc9687c..f29de32b 100644 --- a/src/api/server_server.rs +++ b/src/api/server_server.rs @@ -735,11 +735,6 @@ pub async fn send_transaction_message_route( }; // We do not add the event_id field to the pdu here because of signature and hashes checks - services() - .rooms - .event_handler - .acl_check(sender_servername, &room_id)?; - let mutex = Arc::clone( services() .globals diff --git a/src/service/rooms/event_handler/mod.rs b/src/service/rooms/event_handler/mod.rs index 899f035a..1fa78b6c 100644 --- a/src/service/rooms/event_handler/mod.rs +++ b/src/service/rooms/event_handler/mod.rs @@ -92,6 +92,8 @@ impl Service { )); } + services().rooms.event_handler.acl_check(origin, &room_id)?; + // 1. Skip the PDU if we already have it as a timeline event if let Some(pdu_id) = services().rooms.timeline.get_pdu_id(event_id)? { return Ok(Some(pdu_id.to_vec())); From 9b55ce933a5cf62f445a061bdcd6b17e7bb58857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20K=C3=B6sters?= Date: Sat, 12 Aug 2023 09:53:32 +0200 Subject: [PATCH 2/2] Back off from more events, don't retry auth events --- src/service/rooms/event_handler/mod.rs | 110 ++++++++++++++++--------- 1 file changed, 71 insertions(+), 39 deletions(-) diff --git a/src/service/rooms/event_handler/mod.rs b/src/service/rooms/event_handler/mod.rs index 1fa78b6c..e93feaa5 100644 --- a/src/service/rooms/event_handler/mod.rs +++ b/src/service/rooms/event_handler/mod.rs @@ -119,7 +119,15 @@ impl Service { .ok_or_else(|| Error::bad_database("Failed to find first pdu in db."))?; let (incoming_pdu, val) = self - .handle_outlier_pdu(origin, &create_event, event_id, room_id, value, pub_key_map) + .handle_outlier_pdu( + origin, + &create_event, + event_id, + room_id, + value, + false, + pub_key_map, + ) .await?; self.check_room_id(room_id, &incoming_pdu)?; @@ -276,6 +284,7 @@ impl Service { event_id: &'a EventId, room_id: &'a RoomId, mut value: BTreeMap, + auth_events_known: bool, pub_key_map: &'a RwLock>>, ) -> AsyncRecursiveType<'a, Result<(Arc, BTreeMap)>> { Box::pin(async move { @@ -343,23 +352,25 @@ impl Service { self.check_room_id(room_id, &incoming_pdu)?; - // 4. fetch any missing auth events doing all checks listed here starting at 1. These are not timeline events - // 5. Reject "due to auth events" if can't get all the auth events or some of the auth events are also rejected "due to auth events" - // NOTE: Step 5 is not applied anymore because it failed too often - debug!(event_id = ?incoming_pdu.event_id, "Fetching auth events"); - self.fetch_and_handle_outliers( - origin, - &incoming_pdu - .auth_events - .iter() - .map(|x| Arc::from(&**x)) - .collect::>(), - create_event, - room_id, - room_version_id, - pub_key_map, - ) - .await; + if !auth_events_known { + // 4. fetch any missing auth events doing all checks listed here starting at 1. These are not timeline events + // 5. Reject "due to auth events" if can't get all the auth events or some of the auth events are also rejected "due to auth events" + // NOTE: Step 5 is not applied anymore because it failed too often + debug!(event_id = ?incoming_pdu.event_id, "Fetching auth events"); + self.fetch_and_handle_outliers( + origin, + &incoming_pdu + .auth_events + .iter() + .map(|x| Arc::from(&**x)) + .collect::>(), + create_event, + room_id, + room_version_id, + pub_key_map, + ) + .await; + } // 6. Reject "due to auth events" if the event doesn't pass auth based on the auth events debug!( @@ -1015,26 +1026,6 @@ impl Service { let mut pdus = vec![]; for id in events { - if let Some((time, tries)) = services() - .globals - .bad_event_ratelimiter - .read() - .unwrap() - .get(&**id) - { - // Exponential backoff - let mut min_elapsed_duration = - Duration::from_secs(5 * 60) * (*tries) * (*tries); - if min_elapsed_duration > Duration::from_secs(60 * 60 * 24) { - min_elapsed_duration = Duration::from_secs(60 * 60 * 24); - } - - if time.elapsed() < min_elapsed_duration { - info!("Backing off from {}", id); - continue; - } - } - // a. Look in the main timeline (pduid_pdu tree) // b. Look at outlier pdu tree // (get_pdu_json checks both) @@ -1052,6 +1043,26 @@ impl Service { let mut events_all = HashSet::new(); let mut i = 0; while let Some(next_id) = todo_auth_events.pop() { + if let Some((time, tries)) = services() + .globals + .bad_event_ratelimiter + .read() + .unwrap() + .get(&*next_id) + { + // Exponential backoff + let mut min_elapsed_duration = + Duration::from_secs(5 * 60) * (*tries) * (*tries); + if min_elapsed_duration > Duration::from_secs(60 * 60 * 24) { + min_elapsed_duration = Duration::from_secs(60 * 60 * 24); + } + + if time.elapsed() < min_elapsed_duration { + info!("Backing off from {}", next_id); + continue; + } + } + if events_all.contains(&next_id) { continue; } @@ -1062,7 +1073,7 @@ impl Service { } if let Ok(Some(_)) = services().rooms.timeline.get_pdu(&next_id) { - trace!("Found {} in db", id); + trace!("Found {} in db", next_id); continue; } @@ -1121,6 +1132,26 @@ impl Service { } for (next_id, value) in events_in_reverse_order.iter().rev() { + if let Some((time, tries)) = services() + .globals + .bad_event_ratelimiter + .read() + .unwrap() + .get(&**next_id) + { + // Exponential backoff + let mut min_elapsed_duration = + Duration::from_secs(5 * 60) * (*tries) * (*tries); + if min_elapsed_duration > Duration::from_secs(60 * 60 * 24) { + min_elapsed_duration = Duration::from_secs(60 * 60 * 24); + } + + if time.elapsed() < min_elapsed_duration { + info!("Backing off from {}", next_id); + continue; + } + } + match self .handle_outlier_pdu( origin, @@ -1128,6 +1159,7 @@ impl Service { next_id, room_id, value.clone(), + true, pub_key_map, ) .await