diff --git a/src/api/server/invite.rs b/src/api/server/invite.rs index f9ad2dbb6..965fe33d2 100644 --- a/src/api/server/invite.rs +++ b/src/api/server/invite.rs @@ -401,7 +401,6 @@ async fn validate_invite_create_event( if !state_res::auth_check( room_version_rules, pdu, - None, |_, _| async { unreachable!("No state should be fetched when processing a lone create event"); }, diff --git a/src/core/matrix/state_res/event_auth.rs b/src/core/matrix/state_res/event_auth.rs index e5693cf78..b5d719742 100644 --- a/src/core/matrix/state_res/event_auth.rs +++ b/src/core/matrix/state_res/event_auth.rs @@ -5,17 +5,21 @@ use futures::{ future::{OptionFuture, join, join3}, }; use ruma::{ - Int, OwnedUserId, RoomVersionId, UserId, + AnyKeyName, CanonicalJsonObject, Int, OwnedUserId, RoomVersionId, SigningKeyId, UserId, events::room::{ join_rules::{JoinRule, RoomJoinRulesEventContent}, - member::{MembershipState, ThirdPartyInvite}, + member::MembershipState, power_levels::{RoomPowerLevelsEventContent, UserPowerLevel}, + third_party_invite::PublicKey, }, int, room_version_rules::{RoomIdFormatVersion, RoomVersionRules}, - serde::Raw, + serde::{Raw, base64, base64::Standard}, + signatures, + signatures::to_canonical_json_string_for_signing, + third_party_invite::IdentityServerBase64PublicKey, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue}; use super::{ @@ -25,7 +29,7 @@ use super::{ deserialize_power_levels_content_redact, }, }; -use crate::{debug, error, trace, warn}; +use crate::{debug, err, error, trace, warn}; // FIXME: field extracting could be bundled for `content` #[derive(Deserialize)] @@ -48,6 +52,40 @@ struct RoomCreateContentFields { federate: bool, } +// Workaround for https://github.com/matrix-org/matrix-spec/issues/2471 +#[derive(Deserialize, Debug)] +struct RoomThirdPartyInviteContentFields { + public_key: Option, + public_keys: Option>, +} + +// same here sort of (maintainers see https://forgejo.ellis.link/continuwuation/continuwuity-sec/issues/41) +#[derive(Deserialize, Debug)] +struct MemberThirdPartyInviteSignedContent { + signed: ThirdPartyInviteSignedObject, +} + +#[derive(Serialize, Deserialize, Debug)] +struct ThirdPartyInviteSignedObject(CanonicalJsonObject); + +impl ThirdPartyInviteSignedObject { + #[inline] + fn mxid(&self) -> Option { + self.0 + .get("mxid")? + .as_str() + .and_then(|raw| UserId::parse(raw).ok()) + } + + #[inline] + fn token(&self) -> Option<&str> { self.0.get("token")?.as_str() } + + #[inline] + fn signatures(&self) -> Option { + self.0.get("signatures")?.as_object().map(ToOwned::to_owned) + } +} + /// For the given event `kind` what are the relevant auth events that are needed /// to authenticate this `content`. /// @@ -61,7 +99,7 @@ pub fn auth_types_for_event( state_key: Option<&str>, content: &RawJsonValue, room_version: &RoomVersionRules, -) -> serde_json::Result> { +) -> crate::Result> { if kind == &TimelineEventType::RoomCreate { return Ok(vec![]); } @@ -84,7 +122,7 @@ pub fn auth_types_for_event( #[allow(unused)] struct RoomMemberContentFields { membership: Option>, - third_party_invite: Option>, + third_party_invite: Option>, join_authorised_via_users_server: Option>, } @@ -116,16 +154,24 @@ pub fn auth_types_for_event( auth_types.push(key); } - // TODO: restore this once 3pid support isn't broken - // if membership == MembershipState::Invite { - // if let Some(Ok(t_id)) = content.third_party_invite.map(|t| - // t.deserialize()) { let key = - // (StateEventType::RoomThirdPartyInvite, - // t_id.signed.token.into()); if !auth_types.contains(& - // key) { auth_types.push(key); - // } - // } - // } + if membership == MembershipState::Invite { + if let Some(Ok(t_id)) = content.third_party_invite.map(|t| t.deserialize()) { + let key = ( + StateEventType::RoomThirdPartyInvite, + t_id.signed + .token() + .ok_or_else(|| { + err!(Request(BadJson( + "Third party invite content missing a token" + ))) + })? + .into(), + ); + if !auth_types.contains(&key) { + auth_types.push(key); + } + } + } } } } @@ -179,12 +225,11 @@ fn get_user_power_level( pub async fn auth_check( room_version: &RoomVersionRules, incoming_event: &E, - current_third_party_invite: Option<&E>, fetch_state: F, create_event: &E, ) -> Result where - F: Fn(&StateEventType, &str) -> Fut + Send, + F: Fn(&StateEventType, &str) -> Fut + Send + Copy, Fut: Future> + Send, E: Event + Send + Sync, for<'a> &'a E: Event + Send, @@ -444,13 +489,15 @@ where sender, sender_member_event.as_ref(), incoming_event, - current_third_party_invite, + fetch_state, power_levels_event.as_ref(), join_rules_event.as_ref(), user_for_join_auth.as_deref(), &user_for_join_auth_membership, &room_create_event, - )? { + ) + .await? + { return Ok(false); } @@ -639,14 +686,14 @@ where /// event and the current State. #[allow(clippy::too_many_arguments)] #[allow(clippy::cognitive_complexity)] -fn valid_membership_change( +async fn valid_membership_change( room_version: &RoomVersionRules, target_user: &UserId, target_user_membership_event: Option<&E>, sender: &UserId, sender_membership_event: Option<&E>, current_event: &E, - current_third_party_invite: Option<&E>, + fetch_state: F, power_levels_event: Option<&E>, join_rules_event: Option<&E>, user_for_join_auth: Option<&UserId>, @@ -655,11 +702,13 @@ fn valid_membership_change( ) -> Result where E: Event + Send + Sync, + F: Fn(&StateEventType, &str) -> Fut + Send + Copy, + Fut: Future> + Send, for<'a> &'a E: Event + Send, { #[derive(Deserialize)] struct GetThirdPartyInvite { - third_party_invite: Option>, + third_party_invite: Option>, } let create_content = from_json_str::(create_room.content().get())?; let creator = if room_version.authorization.use_room_create_sender { @@ -901,22 +950,18 @@ where | MembershipState::Invite => { // If content has third_party_invite key trace!("starting target_membership=invite check"); - match third_party_invite.and_then(|i| i.deserialize().ok()) { - | Some(tp_id) => + match third_party_invite { + | Some(raw_tp_id) => if target_user_current_membership == MembershipState::Ban { warn!(?target_user_membership_event_id, "Can't invite banned user"); false } else { - let allow = verify_third_party_invite( - Some(target_user), - sender, - &tp_id, - current_third_party_invite, - ); - if !allow { - warn!("Third party invite invalid"); - } - allow + let Ok(tp_id) = raw_tp_id.deserialize() else { + warn!("Invite third party content is invalid"); + return Ok(false); + }; + verify_third_party_invite(Some(target_user), sender, &tp_id, fetch_state) + .await }, | _ => if !sender_is_joined { @@ -1435,14 +1480,133 @@ fn get_send_level( .unwrap_or_else(|| if state_key.is_some() { int!(50) } else { int!(0) }) } -fn verify_third_party_invite( - _target_user: Option<&UserId>, - _sender: &UserId, - _tp_id: &ThirdPartyInvite, - _current_third_party_invite: Option<&impl Event>, -) -> bool { - // TODO: implement proper verification here - true +async fn verify_third_party_invite( + target_user: Option<&UserId>, + sender: &UserId, + tp_id: &MemberThirdPartyInviteSignedContent, + fetch_state: F, +) -> bool +where + F: Fn(&StateEventType, &str) -> Fut + Send, + Fut: Future> + Send, + E: Event + Send + Sync, +{ + // If content.third_party_invite does not have a signed property, reject. + // ^ This would be a deserialization error before this function call, which + // equates to a rejection + let signed = &tp_id.signed; + // If signed does not have mxid and token properties, reject. + let Some(mxid) = signed.mxid() else { + warn!("Invite third party signed content is missing or has invalid `mxid` property"); + return false; + }; + let Some(token) = signed.token() else { + warn!("Invite third party signed content is missing or has invalid `token` property"); + return false; + }; + if let Err(e) = mxid.validate_historical() { + warn!(error=?e, ?mxid, "Invite third party signed content has invalid `mxid` property"); + return false; + } + // If mxid does not match state_key, reject. + // N.B. target_user is the state_key + if target_user.is_none_or(|u| u != mxid) { + warn!(?target_user, %mxid, "Invite third party mxid does not match membership state key"); + return false; + } + // If there is no m.room.third_party_invite event in the current room state + // with state_key matching token, reject. + let Some(current_third_party_invite) = + fetch_state(&StateEventType::RoomThirdPartyInvite, token).await + else { + warn!("No third party invite associated with the invite's third party invite found"); + return false; + }; + let Ok(tpi_content) = current_third_party_invite + .get_content::() + .inspect_err( + |e| warn!(error=?e, "Third party invite event content is invalid (cannot deserialize)"), + ) + else { + return false; + }; + if current_third_party_invite.state_key() != Some(token) { + warn!( + state_key=?current_third_party_invite.state_key(), + %token, + "Third party invite state key mismatch with signed invite token" + ); + return false; + } + // If sender does not match sender of the m.room.third_party_invite, reject. + if sender != current_third_party_invite.sender() { + warn!( + ?sender, + third_party_invite_sender=%current_third_party_invite.sender(), + "Third party invite sender does not match invite membership sender" + ); + return false; + } + // If any signature in signed matches any public key in the + // m.room.third_party_invite event, allow. The public keys are in content of + // m.room.third_party_invite as: + // + // A single public key in the public_key property. + // A list of public keys in the public_keys property. + + // NOTE: Technically the two below operations shouldn't be fallible at this + // point in execution, but if we can't de/serialize them now, we won't be + // able to in the future, so rejecting the event is the right way to go. + // We log an error though since this is unexpected. + let Ok(signed_json) = to_canonical_json_string_for_signing(&signed.0).inspect_err( + |e| error!(error=?e, "Canonical signed object cannot be cast to a signing string"), + ) else { + return false; + }; + let signatures = signed + .signatures() + .unwrap_or_default() // Empty sigs object will never pass so this is fine + .values() + .filter_map(|signatures| signatures.as_object()) + .flat_map(|signatures| { + signatures.iter().map(|(key, sig)| { + let key_id = SigningKeyId::::parse(key).ok()?; + base64::Base64::::parse(sig.as_str()?) + .map(|parsed| (key_id.algorithm(), parsed)) + .ok() + }) + }) + .flatten() + .collect::>(); + + for public_key_bytes in tpi_content + .public_key + .into_iter() + .chain( + tpi_content + .public_keys + .unwrap_or_default() + .iter() + .map(|pk| pk.public_key.clone()), + ) + .filter_map(|pk| pk.decode().ok()) + { + for (algorithm, signature) in &signatures { + if signatures::verify_canonical_json_bytes( + algorithm, + &public_key_bytes, + signature.as_bytes(), + signed_json.as_bytes(), + ) + .is_ok() + { + return true; + } + } + } + + warn!("Invite via third party was not signed by any associated public keys"); + false } #[cfg(test)] @@ -1461,7 +1625,7 @@ mod tests { use serde_json::value::to_raw_value as to_raw_json_value; use crate::{ - matrix::{Event, EventTypeExt, Pdu as PduEvent}, + matrix::{Event, EventTypeExt}, state_res::{ StateMap, event_auth::valid_membership_change, @@ -1472,8 +1636,8 @@ mod tests { }, }; - #[test] - fn test_ban_pass() { + #[tokio::test] + async fn test_ban_pass() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1495,6 +1659,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = charlie(); let sender = alice(); @@ -1506,19 +1672,20 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), None, &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } - #[test] - fn test_join_non_creator() { + #[tokio::test] + async fn test_join_non_creator() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1540,6 +1707,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = charlie(); let sender = charlie(); @@ -1551,19 +1720,20 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), None, &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } - #[test] - fn test_join_creator() { + #[tokio::test] + async fn test_join_creator() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1585,6 +1755,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = alice(); let sender = alice(); @@ -1596,19 +1768,20 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), None, &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } - #[test] - fn test_ban_fail() { + #[tokio::test] + async fn test_ban_fail() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1630,6 +1803,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = alice(); let sender = charlie(); @@ -1641,19 +1816,20 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), None, &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } - #[test] - fn test_restricted_join_rule() { + #[tokio::test] + async fn test_restricted_join_rule() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1692,6 +1868,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = ella(); let sender = ella(); @@ -1703,13 +1881,14 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), Some(alice()), &MembershipState::Join, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); @@ -1721,19 +1900,20 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), Some(ella()), &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } - #[test] - fn test_knock() { + #[tokio::test] + async fn test_knock() { let _ = tracing::subscriber::set_default( tracing_subscriber::fmt().with_test_writer().finish(), ); @@ -1764,6 +1944,8 @@ mod tests { ); let fetch_state = |ty, key| auth_events.get(&(ty, key)).cloned(); + let async_fetch_state = + |t: &StateEventType, k: &str| std::future::ready(fetch_state(t.to_owned(), k.into())); let target_user = ella(); let sender = ella(); @@ -1775,13 +1957,14 @@ mod tests { sender, fetch_state(StateEventType::RoomMember, sender.as_str().into()).as_ref(), &requester, - None::<&PduEvent>, + &async_fetch_state, fetch_state(StateEventType::RoomPowerLevels, "".into()).as_ref(), fetch_state(StateEventType::RoomJoinRules, "".into()).as_ref(), None, &MembershipState::Leave, &fetch_state(StateEventType::RoomCreate, "".into()).unwrap(), ) + .await .unwrap() ); } diff --git a/src/core/matrix/state_res/mod.rs b/src/core/matrix/state_res/mod.rs index 77948c323..3648b70a6 100644 --- a/src/core/matrix/state_res/mod.rs +++ b/src/core/matrix/state_res/mod.rs @@ -757,7 +757,8 @@ where Some(state_key), event.content(), room_version, - )?; + ) + .map_err(|e| Error::InvalidPdu(e.to_string()))?; trace!(list = ?auth_types, event_id = event.event_id().as_str(), "auth types for event"); let mut auth_state = StateMap::new(); @@ -830,12 +831,6 @@ where debug!(event_id = event.event_id().as_str(), "Running auth checks"); - // The key for this is (eventType + a state_key of the signed token not - // sender) so search for it - let current_third_party = auth_state.iter().find_map(|(_, pdu)| { - (*pdu.event_type() == TimelineEventType::RoomThirdPartyInvite).then_some(pdu) - }); - let fetch_state = |ty: &StateEventType, key: &str| { future::ready( auth_state @@ -847,7 +842,6 @@ where let auth_result = auth_check( room_version, &event, - current_third_party, fetch_state, &fetch_state(&StateEventType::RoomCreate, "") .await diff --git a/src/service/rooms/event_handler/pdu_checks.rs b/src/service/rooms/event_handler/pdu_checks.rs index 458ff4f3b..dc209f612 100644 --- a/src/service/rooms/event_handler/pdu_checks.rs +++ b/src/service/rooms/event_handler/pdu_checks.rs @@ -160,15 +160,9 @@ impl super::Service { ready(auth_events_by_key.get(&key).map(ToOwned::to_owned)) }; - state_res::event_auth::auth_check( - room_version_rules, - pdu, - None, // TODO: third party invite - state_fetch, - create_event, - ) - .await - .map_err(|e| err!("Event self-authentication failed: {e:?}")) + state_res::event_auth::auth_check(room_version_rules, pdu, state_fetch, create_event) + .await + .map_err(|e| err!("Event self-authentication failed: {e:?}")) } /// Checks that the event passes PDU check 5, which ensures that the event @@ -242,7 +236,6 @@ impl super::Service { let auth_check = state_res::event_auth::auth_check( room_version_rules, incoming_pdu, - None, // TODO: third party invite |ty, sk| state_fetch(ty.clone(), sk.into()), create_event.as_pdu(), ) @@ -292,7 +285,6 @@ impl super::Service { state_res::event_auth::auth_check( room_version_rules, incoming_pdu, - None, // third-party invite state_fetch, create_event.as_pdu(), ) diff --git a/src/service/rooms/membership/mod.rs b/src/service/rooms/membership/mod.rs index 31ba5d5fc..6bde92aba 100644 --- a/src/service/rooms/membership/mod.rs +++ b/src/service/rooms/membership/mod.rs @@ -582,7 +582,6 @@ impl Service { let auth_check = state_res::event_auth::auth_check( &room_version.rules().unwrap(), &parsed_join_pdu, - None, // TODO: third party invite |k, s| state_fetch(k.clone(), s.into()), &state_fetch(StateEventType::RoomCreate, "".into()) .await diff --git a/src/service/rooms/timeline/create.rs b/src/service/rooms/timeline/create.rs index 7d3ee9089..938fc3e41 100644 --- a/src/service/rooms/timeline/create.rs +++ b/src/service/rooms/timeline/create.rs @@ -252,15 +252,10 @@ impl super::Service { | _ => create_pdu.as_ref().unwrap().as_pdu(), }; - let auth_check = state_res::auth_check( - &room_version_rules, - &pdu, - None, // TODO: third_party_invite - auth_fetch, - create_event, - ) - .await - .map_err(|e| err!(Request(Forbidden(warn!("Auth check failed: {e:?}")))))?; + let auth_check = + state_res::auth_check(&room_version_rules, &pdu, auth_fetch, create_event) + .await + .map_err(|e| err!(Request(Forbidden(warn!("Auth check failed: {e:?}")))))?; if !auth_check { return Err!(Request(Forbidden("Event is not authorized."))); diff --git a/src/web/pages/account/register.rs b/src/web/pages/account/register.rs index a17bee043..f8389ceb4 100644 --- a/src/web/pages/account/register.rs +++ b/src/web/pages/account/register.rs @@ -314,6 +314,7 @@ async fn get_register_email_validate( Some(email), &client ) + .boxed() .await? ) } @@ -508,6 +509,7 @@ async fn begin_registration( // If email isn't required we can immediately complete registration Ok(response!( complete_registration(services, session_store, completed_registration, None, client) + .boxed() .await? )) }