From c565e6ffbc098bdbb5463e03d80f5de726f9f330 Mon Sep 17 00:00:00 2001 From: Ginger Date: Tue, 30 Dec 2025 23:24:06 -0500 Subject: [PATCH] feat: Restrict where certain admin commands may be used --- src/admin/admin.rs | 18 +++- src/admin/context.rs | 21 +++- src/admin/debug/commands.rs | 29 ++--- src/admin/debug/mod.rs | 12 +-- src/admin/federation/commands.rs | 2 + src/admin/media/commands.rs | 8 ++ src/admin/processor.rs | 1 + src/admin/server/commands.rs | 8 ++ src/router/layers.rs | 5 +- src/service/admin/console.rs | 11 +- src/service/admin/execute.rs | 7 +- src/service/admin/mod.rs | 152 +++++++++++++++++---------- src/service/rooms/timeline/append.rs | 3 +- 13 files changed, 185 insertions(+), 92 deletions(-) diff --git a/src/admin/admin.rs b/src/admin/admin.rs index e6479960..bec02af0 100644 --- a/src/admin/admin.rs +++ b/src/admin/admin.rs @@ -53,14 +53,26 @@ pub(super) async fn process(command: AdminCommand, context: &Context<'_>) -> Res use AdminCommand::*; match command { - | Appservices(command) => appservice::process(command, context).await, + | Appservices(command) => { + // appservice commands are all restricted + context.bail_restricted()?; + appservice::process(command, context).await + }, | Media(command) => media::process(command, context).await, - | Users(command) => user::process(command, context).await, + | Users(command) => { + // user commands are all restricted + context.bail_restricted()?; + user::process(command, context).await + }, | Rooms(command) => room::process(command, context).await, | Federation(command) => federation::process(command, context).await, | Server(command) => server::process(command, context).await, | Debug(command) => debug::process(command, context).await, - | Query(command) => query::process(command, context).await, + | Query(command) => { + // query commands are all restricted + context.bail_restricted()?; + query::process(command, context).await + }, | Check(command) => check::process(command, context).await, } } diff --git a/src/admin/context.rs b/src/admin/context.rs index 3d3cffb7..5432acfa 100644 --- a/src/admin/context.rs +++ b/src/admin/context.rs @@ -1,6 +1,6 @@ use std::{fmt, time::SystemTime}; -use conduwuit::Result; +use conduwuit::{Err, Result}; use conduwuit_service::Services; use futures::{ Future, FutureExt, TryFutureExt, @@ -8,6 +8,7 @@ use futures::{ lock::Mutex, }; use ruma::{EventId, UserId}; +use service::admin::InvocationSource; pub(crate) struct Context<'a> { pub(crate) services: &'a Services, @@ -16,6 +17,7 @@ pub(crate) struct Context<'a> { pub(crate) reply_id: Option<&'a EventId>, pub(crate) sender: Option<&'a UserId>, pub(crate) output: Mutex>>, + pub(crate) source: InvocationSource, } impl Context<'_> { @@ -43,4 +45,21 @@ impl Context<'_> { self.sender .unwrap_or_else(|| self.services.globals.server_user.as_ref()) } + + /// Returns an Err if the [`Self::source`] of this context does not allow + /// restricted commands to be executed. + /// + /// This is intended to be placed at the start of restricted commands' + /// implementations, like so: ```ignore + /// self.bail_restricted()?; + /// // actual command impl + /// ``` + #[must_use] + pub(crate) fn bail_restricted(&self) -> Result { + if self.source.allows_restricted() { + Ok(()) + } else { + Err!("This command can only be used in the admin room.") + } + } } diff --git a/src/admin/debug/commands.rs b/src/admin/debug/commands.rs index 0dac50dc..0b6e45b2 100644 --- a/src/admin/debug/commands.rs +++ b/src/admin/debug/commands.rs @@ -291,6 +291,8 @@ pub(super) async fn get_remote_pdu( #[admin_command] pub(super) async fn get_room_state(&self, room: OwnedRoomOrAliasId) -> Result { + self.bail_restricted()?; + let room_id = self.services.rooms.alias.resolve(&room).await?; let room_state: Vec> = self .services @@ -417,27 +419,6 @@ pub(super) async fn change_log_level(&self, filter: Option, reset: bool) Err!("No log level was specified.") } -#[admin_command] -pub(super) async fn sign_json(&self) -> Result { - if self.body.len() < 2 - || !self.body[0].trim().starts_with("```") - || self.body.last().unwrap_or(&"").trim() != "```" - { - return Err!("Expected code block in command body. Add --help for details."); - } - - let string = self.body[1..self.body.len().checked_sub(1).unwrap()].join("\n"); - match serde_json::from_str(&string) { - | Err(e) => return Err!("Invalid json: {e}"), - | Ok(mut value) => { - self.services.server_keys.sign_json(&mut value)?; - let json_text = serde_json::to_string_pretty(&value)?; - write!(self, "{json_text}") - }, - } - .await -} - #[admin_command] pub(super) async fn verify_json(&self) -> Result { if self.body.len() < 2 @@ -477,6 +458,8 @@ pub(super) async fn verify_pdu(&self, event_id: OwnedEventId) -> Result { #[admin_command] #[tracing::instrument(skip(self))] pub(super) async fn first_pdu_in_room(&self, room_id: OwnedRoomId) -> Result { + self.bail_restricted()?; + if !self .services .rooms @@ -502,6 +485,8 @@ pub(super) async fn first_pdu_in_room(&self, room_id: OwnedRoomId) -> Result { #[admin_command] #[tracing::instrument(skip(self))] pub(super) async fn latest_pdu_in_room(&self, room_id: OwnedRoomId) -> Result { + self.bail_restricted()?; + if !self .services .rooms @@ -532,6 +517,8 @@ pub(super) async fn force_set_room_state_from_server( server_name: OwnedServerName, at_event: Option, ) -> Result { + self.bail_restricted()?; + if !self .services .rooms diff --git a/src/admin/debug/mod.rs b/src/admin/debug/mod.rs index 7a0769ab..883a629d 100644 --- a/src/admin/debug/mod.rs +++ b/src/admin/debug/mod.rs @@ -47,9 +47,9 @@ pub enum DebugCommand { shorteventid: ShortEventId, }, - /// - Attempts to retrieve a PDU from a remote server. Inserts it into our - /// database/timeline if found and we do not have this PDU already - /// (following normal event auth rules, handles it as an incoming PDU). + /// - Attempts to retrieve a PDU from a remote server. **Does not** insert + /// it into the database + /// or persist it anywhere. GetRemotePdu { /// An event ID (a $ followed by the base64 reference hash) event_id: OwnedEventId, @@ -125,12 +125,6 @@ pub enum DebugCommand { reset: bool, }, - /// - Sign JSON blob - /// - /// This command needs a JSON blob provided in a Markdown code block below - /// the command. - SignJson, - /// - Verify JSON signatures /// /// This command needs a JSON blob provided in a Markdown code block below diff --git a/src/admin/federation/commands.rs b/src/admin/federation/commands.rs index f77dadab..cb043653 100644 --- a/src/admin/federation/commands.rs +++ b/src/admin/federation/commands.rs @@ -8,12 +8,14 @@ use crate::{admin_command, get_room_info}; #[admin_command] pub(super) async fn disable_room(&self, room_id: OwnedRoomId) -> Result { + self.bail_restricted()?; self.services.rooms.metadata.disable_room(&room_id, true); self.write_str("Room disabled.").await } #[admin_command] pub(super) async fn enable_room(&self, room_id: OwnedRoomId) -> Result { + self.bail_restricted()?; self.services.rooms.metadata.disable_room(&room_id, false); self.write_str("Room enabled.").await } diff --git a/src/admin/media/commands.rs b/src/admin/media/commands.rs index ab857ccd..04269555 100644 --- a/src/admin/media/commands.rs +++ b/src/admin/media/commands.rs @@ -16,6 +16,8 @@ pub(super) async fn delete( mxc: Option, event_id: Option, ) -> Result { + self.bail_restricted()?; + if event_id.is_some() && mxc.is_some() { return Err!("Please specify either an MXC or an event ID, not both.",); } @@ -176,6 +178,8 @@ pub(super) async fn delete( #[admin_command] pub(super) async fn delete_list(&self) -> Result { + self.bail_restricted()?; + if self.body.len() < 2 || !self.body[0].trim().starts_with("```") || self.body.last().unwrap_or(&"").trim() != "```" @@ -231,6 +235,8 @@ pub(super) async fn delete_past_remote_media( after: bool, yes_i_want_to_delete_local_media: bool, ) -> Result { + self.bail_restricted()?; + if before && after { return Err!("Please only pick one argument, --before or --after.",); } @@ -273,6 +279,8 @@ pub(super) async fn delete_all_from_server( server_name: OwnedServerName, yes_i_want_to_delete_local_media: bool, ) -> Result { + self.bail_restricted()?; + if server_name == self.services.globals.server_name() && !yes_i_want_to_delete_local_media { return Err!("This command only works for remote media by default.",); } diff --git a/src/admin/processor.rs b/src/admin/processor.rs index 2c91efe1..77f5a35f 100644 --- a/src/admin/processor.rs +++ b/src/admin/processor.rs @@ -59,6 +59,7 @@ async fn process_command(services: Arc, input: &CommandInput) -> Proce reply_id: input.reply_id.as_deref(), sender: input.sender.as_deref(), output: BufWriter::new(Vec::new()).into(), + source: input.source, }; let (result, mut logs) = process(&context, command, &args).await; diff --git a/src/admin/server/commands.rs b/src/admin/server/commands.rs index 8c3573f0..443b259b 100644 --- a/src/admin/server/commands.rs +++ b/src/admin/server/commands.rs @@ -24,6 +24,8 @@ pub(super) async fn uptime(&self) -> Result { #[admin_command] pub(super) async fn show_config(&self) -> Result { + self.bail_restricted()?; + self.write_str(&format!("{}", *self.services.server.config)) .await } @@ -118,6 +120,8 @@ pub(super) async fn list_backups(&self) -> Result { #[admin_command] pub(super) async fn backup_database(&self) -> Result { + self.bail_restricted()?; + let db = Arc::clone(&self.services.db); let result = self .services @@ -144,6 +148,8 @@ pub(super) async fn admin_notice(&self, message: Vec) -> Result { #[admin_command] pub(super) async fn reload_mods(&self) -> Result { + self.bail_restricted()?; + self.services.server.reload()?; self.write_str("Reloading server...").await @@ -168,6 +174,8 @@ pub(super) async fn restart(&self, force: bool) -> Result { #[admin_command] pub(super) async fn shutdown(&self) -> Result { + self.bail_restricted()?; + warn!("shutdown command"); self.services.server.shutdown()?; diff --git a/src/router/layers.rs b/src/router/layers.rs index 7ef11331..59f9b96f 100644 --- a/src/router/layers.rs +++ b/src/router/layers.rs @@ -66,7 +66,10 @@ pub(crate) fn build(services: &Arc) -> Result<(Router, Guard)> { .layer(RequestBodyTimeoutLayer::new(Duration::from_secs( server.config.client_receive_timeout, ))) - .layer(TimeoutLayer::with_status_code(StatusCode::REQUEST_TIMEOUT, Duration::from_secs(server.config.client_request_timeout))) + .layer(TimeoutLayer::with_status_code( + StatusCode::REQUEST_TIMEOUT, + Duration::from_secs(server.config.client_request_timeout), + )) .layer(SetResponseHeaderLayer::if_not_present( HeaderName::from_static("origin-agent-cluster"), // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin-Agent-Cluster HeaderValue::from_static("?1"), diff --git a/src/service/admin/console.rs b/src/service/admin/console.rs index 931bb719..136e4956 100644 --- a/src/service/admin/console.rs +++ b/src/service/admin/console.rs @@ -9,7 +9,10 @@ use rustyline_async::{Readline, ReadlineError, ReadlineEvent}; use termimad::MadSkin; use tokio::task::JoinHandle; -use crate::{Dep, admin}; +use crate::{ + Dep, + admin::{self, InvocationSource}, +}; pub struct Console { server: Arc, @@ -160,7 +163,11 @@ impl Console { } async fn process(self: Arc, line: String) { - match self.admin.command_in_place(line, None).await { + match self + .admin + .command_in_place(line, None, InvocationSource::Console) + .await + { | Ok(Some(ref content)) => self.output(content), | Err(ref content) => self.output_err(content), | _ => unreachable!(), diff --git a/src/service/admin/execute.rs b/src/service/admin/execute.rs index e0d724bd..d297c565 100644 --- a/src/service/admin/execute.rs +++ b/src/service/admin/execute.rs @@ -2,6 +2,8 @@ use conduwuit::{Err, Result, debug, debug_info, error, implement, info}; use ruma::events::room::message::RoomMessageEventContent; use tokio::time::{Duration, sleep}; +use crate::admin::InvocationSource; + pub(super) const SIGNAL: &str = "SIGUSR2"; /// Possibly spawn the terminal console at startup if configured. @@ -88,7 +90,10 @@ pub(super) async fn signal_execute(&self) -> Result { async fn execute_command(&self, i: usize, command: String) -> Result { debug!("Execute command #{i}: executing {command:?}"); - match self.command_in_place(command, None).await { + match self + .command_in_place(command, None, InvocationSource::Console) + .await + { | Ok(Some(output)) => Self::execute_command_output(i, &output), | Err(output) => Self::execute_command_error(i, &output), | Ok(None) => { diff --git a/src/service/admin/mod.rs b/src/service/admin/mod.rs index 3d0ea461..ac7447b5 100644 --- a/src/service/admin/mod.rs +++ b/src/service/admin/mod.rs @@ -54,15 +54,36 @@ struct Services { media: Dep, } -/// Inputs to a command are a multi-line string, optional reply_id, and optional -/// sender. +/// Inputs to a command are a multi-line string, invocation source, optional +/// reply_id, and optional sender. #[derive(Debug)] pub struct CommandInput { pub command: String, pub reply_id: Option, + pub source: InvocationSource, pub sender: Option>, } +/// Where a command is being invoked from. +#[derive(Debug, Clone, Copy)] +pub enum InvocationSource { + /// The server's private admin room + AdminRoom, + /// An escaped `\!admin` command in a public room + EscapedCommand, + /// The server's admin console + Console, + /// Some other trusted internal source + Internal, +} + +impl InvocationSource { + /// Returns whether this invocation source allows "restricted" + /// commands, i.e. ones that could be potentially dangerous if executed by + /// an attacker or in a public room. + pub fn allows_restricted(&self) -> bool { !matches!(self, Self::EscapedCommand) } +} + /// Prototype of the tab-completer. The input is buffered text when tab /// asserted; the output will fully replace the input buffer. pub type Completer = fn(&str) -> String; @@ -276,10 +297,15 @@ impl Service { /// Posts a command to the command processor queue and returns. Processing /// will take place on the service worker's task asynchronously. Errors if /// the queue is full. - pub fn command(&self, command: String, reply_id: Option) -> Result<()> { + pub fn command( + &self, + command: String, + reply_id: Option, + source: InvocationSource, + ) -> Result<()> { self.channel .0 - .send(CommandInput { command, reply_id, sender: None }) + .send(CommandInput { command, reply_id, source, sender: None }) .map_err(|e| err!("Failed to enqueue admin command: {e:?}")) } @@ -290,11 +316,17 @@ impl Service { &self, command: String, reply_id: Option, + source: InvocationSource, sender: Box, ) -> Result<()> { self.channel .0 - .send(CommandInput { command, reply_id, sender: Some(sender) }) + .send(CommandInput { + command, + reply_id, + source, + sender: Some(sender), + }) .map_err(|e| err!("Failed to enqueue admin command: {e:?}")) } @@ -304,8 +336,9 @@ impl Service { &self, command: String, reply_id: Option, + source: InvocationSource, ) -> ProcessorResult { - self.process_command(CommandInput { command, reply_id, sender: None }) + self.process_command(CommandInput { command, reply_id, source, sender: None }) .await } @@ -372,7 +405,20 @@ impl Service { /// Checks whether a given user is an admin of this server pub async fn user_is_admin(&self, user_id: &UserId) -> bool { - self.get_admins().await.contains(&user_id.to_owned()) + if self.services.server.config.admins_list.contains(user_id) { + return true; + } + + if self.services.server.config.admins_from_room { + if let Ok(admin_room) = self.get_admin_room().await { + self.services + .state_cache + .is_joined(user_id, &admin_room) + .await + } + } + + false } /// Gets the room ID of the admin room @@ -473,59 +519,59 @@ impl Service { Ok(()) } - pub async fn is_admin_command(&self, event: &E, body: &str) -> bool + pub async fn is_admin_command(&self, event: &E, body: &str) -> Option where E: Event + Send + Sync, { - // Server-side command-escape with public echo - let is_escape = body.starts_with('\\'); - let is_public_escape = is_escape && body.trim_start_matches('\\').starts_with("!admin"); - - // Admin command with public echo (in admin room) - let server_user = &self.services.globals.server_user; - let is_public_prefix = - body.starts_with("!admin") || body.starts_with(server_user.as_str()); - - // Expected backward branch - if !is_public_escape && !is_public_prefix { - return false; - } - - let user_is_local = self.services.globals.user_is_local(event.sender()); - - // only allow public escaped commands by local admins - if is_public_escape && !user_is_local { - return false; - } - - // Check if server-side command-escape is disabled by configuration - if is_public_escape && !self.services.server.config.admin_escape_commands { - return false; - } - - // Prevent unescaped !admin from being used outside of the admin room - if event.room_id().is_some() - && is_public_prefix - && !self.is_admin_room(event.room_id().unwrap()).await - { - return false; - } - - // Only senders who are admin can proceed + // If the user isn't an admin they definitely can't run admin commands if !self.user_is_admin(event.sender()).await { - return false; + return None; } - // This will evaluate to false if the emergency password is set up so that - // the administrator can execute commands as the server user - let emergency_password_set = self.services.server.config.emergency_password.is_some(); - let from_server = event.sender() == server_user && !emergency_password_set; - if from_server && self.is_admin_room(event.room_id().unwrap()).await { - return false; - } + if let Some(room_id) = event.room_id() + && self.is_admin_room(room_id).await + { + // This is a message in the admin room - // Authentic admin command - true + // Ignore messages which aren't admin commands + let server_user = &self.services.globals.server_user; + if !(body.starts_with("!admin") || body.starts_with(server_user.as_str())) { + return None; + } + + // Ignore messages from the server user _unless_ the emergency password is set + let emergency_password_set = self.services.server.config.emergency_password.is_some(); + if event.sender() == server_user && !emergency_password_set { + return None; + } + + // Looks good + return Some(InvocationSource::AdminRoom); + } else { + // This is a message outside the admin room + + // Is it an escaped admin command? i.e. `\!admin --help` + let is_public_escape = + body.starts_with('\\') && body.trim_start_matches('\\').starts_with("!admin"); + + // Ignore the message if it's not + if !is_public_escape { + return None; + } + + // Only admin users belonging to this server can use escaped commands + if !self.services.globals.user_is_local(event.sender()) { + return None; + } + + // Check if escaped commands are disabled in the config + if !self.services.server.config.admin_escape_commands { + return None; + } + + // Looks good + return Some(InvocationSource::EscapedCommand); + } } #[must_use] diff --git a/src/service/rooms/timeline/append.rs b/src/service/rooms/timeline/append.rs index 2d405820..c568dd5b 100644 --- a/src/service/rooms/timeline/append.rs +++ b/src/service/rooms/timeline/append.rs @@ -335,10 +335,11 @@ where if let Some(body) = content.body { self.services.search.index_pdu(shortroomid, &pdu_id, &body); - if self.services.admin.is_admin_command(pdu, &body).await { + if let Some(source) = self.services.admin.is_admin_command(pdu, &body).await { self.services.admin.command_with_sender( body, Some((pdu.event_id()).into()), + source, pdu.sender.clone().into(), )?; }