1
0
Fork 0
mirror of https://forgejo.ellis.link/continuwuation/continuwuity.git synced 2025-09-03 16:50:56 +00:00

fix(auth): prevent token collisions and optimise lookups

Ensures access tokens are unique across both user and appservice tables to
prevent authentication ambiguity and potential security issues.

Changes:
- On startup, automatically logout any user devices using tokens that
  conflict with appservice tokens (resolves in favour of appservices)
  and log a warning with affected user/device details
- When creating new user tokens, check for conflicts with appservice tokens
  and generate a new token if a collision would occur
- When registering new appservices, reject registration if the token is
  already in use by a user device
- Use futures::select_ok to race token lookups concurrently for better
  performance (adapted from tuwunel commit 066097a8)

This fix-forward approach resolves existing token collisions on startup
whilst preventing new ones from being created, without breaking existing
valid authentications.

The find_token optimisation is adapted from tuwunel (matrix-construct/tuwunel)
commit 066097a8: "Optimize user and appservice token queries" by Jason Volk.
This commit is contained in:
Tom Foster 2025-08-10 14:38:54 +01:00
parent e820551f62
commit d1ebcfaf0b
3 changed files with 59 additions and 17 deletions

View file

@ -5,6 +5,14 @@ use axum_extra::{
typed_header::TypedHeaderRejectionReason, typed_header::TypedHeaderRejectionReason,
}; };
use conduwuit::{Err, Error, Result, debug_error, err, warn}; use conduwuit::{Err, Error, Result, debug_error, err, warn};
use futures::{
TryFutureExt,
future::{
Either::{Left, Right},
select_ok,
},
pin_mut,
};
use ruma::{ use ruma::{
CanonicalJsonObject, CanonicalJsonValue, OwnedDeviceId, OwnedServerName, OwnedUserId, UserId, CanonicalJsonObject, CanonicalJsonValue, OwnedDeviceId, OwnedServerName, OwnedUserId, UserId,
api::{ api::{
@ -54,17 +62,7 @@ pub(super) async fn auth(
| None => request.query.access_token.as_deref(), | None => request.query.access_token.as_deref(),
}; };
let token = if let Some(token) = token { let token = find_token(services, token).await?;
match services.appservice.find_from_token(token).await {
| Some(reg_info) => Token::Appservice(Box::new(reg_info)),
| _ => match services.users.find_from_token(token).await {
| Ok((user_id, device_id)) => Token::User((user_id, device_id)),
| _ => Token::Invalid,
},
}
} else {
Token::None
};
if metadata.authentication == AuthScheme::None { if metadata.authentication == AuthScheme::None {
match metadata { match metadata {
@ -342,3 +340,24 @@ async fn parse_x_matrix(request: &mut Request) -> Result<XMatrix> {
Ok(x_matrix) Ok(x_matrix)
} }
async fn find_token(services: &Services, token: Option<&str>) -> Result<Token> {
let Some(token) = token else {
return Ok(Token::None);
};
let user_token = services.users.find_from_token(token).map_ok(Token::User);
let appservice_token = services
.appservice
.find_from_token(token)
.map_ok(Box::new)
.map_ok(Token::Appservice);
pin_mut!(user_token, appservice_token);
match select_ok([Left(user_token), Right(appservice_token)]).await {
| Err(e) if !e.is_not_found() => Err(e),
| Ok((token, _)) => Ok(token),
| _ => Ok(Token::Invalid),
}
}

View file

@ -133,10 +133,10 @@ impl Service {
.await .await
.is_ok() .is_ok()
{ {
return err!(Request(InvalidParam( return Err(err!(Request(InvalidParam(
"Cannot register appservice: The provided token is already in use by a user \ "Cannot register appservice: The provided token is already in use by a user \
device. Please generate a different token for the appservice." device. Please generate a different token for the appservice."
))); ))));
} }
self.db self.db
@ -182,12 +182,13 @@ impl Service {
.map(|info| info.registration) .map(|info| info.registration)
} }
pub async fn find_from_token(&self, token: &str) -> Option<RegistrationInfo> { pub async fn find_from_token(&self, token: &str) -> Result<RegistrationInfo> {
self.read() self.read()
.await .await
.values() .values()
.find(|info| info.registration.as_token == token) .find(|info| info.registration.as_token == token)
.cloned() .cloned()
.ok_or_else(|| err!(Request(NotFound("Appservice token not found"))))
} }
/// Checks if a given user id matches any exclusive appservice regex /// Checks if a given user id matches any exclusive appservice regex

View file

@ -19,7 +19,7 @@ use ruma::{
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_json::json; use serde_json::json;
use crate::{Dep, account_data, admin, globals, rooms}; use crate::{Dep, account_data, admin, appservice, globals, rooms};
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct UserSuspension { pub struct UserSuspension {
@ -40,6 +40,7 @@ struct Services {
server: Arc<Server>, server: Arc<Server>,
account_data: Dep<account_data::Service>, account_data: Dep<account_data::Service>,
admin: Dep<admin::Service>, admin: Dep<admin::Service>,
appservice: Dep<appservice::Service>,
globals: Dep<globals::Service>, globals: Dep<globals::Service>,
state_accessor: Dep<rooms::state_accessor::Service>, state_accessor: Dep<rooms::state_accessor::Service>,
state_cache: Dep<rooms::state_cache::Service>, state_cache: Dep<rooms::state_cache::Service>,
@ -76,6 +77,7 @@ impl crate::Service for Service {
server: args.server.clone(), server: args.server.clone(),
account_data: args.depend::<account_data::Service>("account_data"), account_data: args.depend::<account_data::Service>("account_data"),
admin: args.depend::<admin::Service>("admin"), admin: args.depend::<admin::Service>("admin"),
appservice: args.depend::<appservice::Service>("appservice"),
globals: args.depend::<globals::Service>("globals"), globals: args.depend::<globals::Service>("globals"),
state_accessor: args state_accessor: args
.depend::<rooms::state_accessor::Service>("rooms::state_accessor"), .depend::<rooms::state_accessor::Service>("rooms::state_accessor"),
@ -407,6 +409,26 @@ impl Service {
))); )));
} }
// Prevent token collisions with appservice tokens
let final_token = if self
.services
.appservice
.find_from_token(token)
.await
.is_ok()
{
let new_token = utils::random_string(32);
conduwuit::debug_warn!(
"Token collision prevented: Generated new token for user '{}' device '{}' \
(original token conflicted with an appservice)",
user_id.localpart(),
device_id
);
new_token
} else {
token.to_owned()
};
// Remove old token // Remove old token
if let Ok(old_token) = self.db.userdeviceid_token.qry(&key).await { if let Ok(old_token) = self.db.userdeviceid_token.qry(&key).await {
self.db.token_userdeviceid.remove(&old_token); self.db.token_userdeviceid.remove(&old_token);
@ -414,8 +436,8 @@ impl Service {
} }
// Assign token to user device combination // Assign token to user device combination
self.db.userdeviceid_token.put_raw(key, token); self.db.userdeviceid_token.put_raw(key, &final_token);
self.db.token_userdeviceid.raw_put(token, key); self.db.token_userdeviceid.raw_put(&final_token, key);
Ok(()) Ok(())
} }