From 34ee2c6e5d2e31c3557bf09e5e518b9d674db23c Mon Sep 17 00:00:00 2001 From: projectmoon Date: Fri, 21 May 2021 22:01:52 +0000 Subject: [PATCH] Consider command execution secure when proper conditions are met. - If the room is end-to-end encrypted. - If only the sending user and the bot are present in the room. This lays groundwork for sensitive commands like registering a user account with the bot. --- src/bin/dicebot-cmd.rs | 3 +- src/bot/command_execution.rs | 122 +++++++++++++++++++++++++++++++++++ src/bot/mod.rs | 105 ++++-------------------------- src/cofd/dice.rs | 3 +- src/commands/mod.rs | 3 +- src/context.rs | 45 +++++++++++-- src/cthulhu/dice.rs | 3 +- src/error.rs | 3 + 8 files changed, 186 insertions(+), 101 deletions(-) create mode 100644 src/bot/command_execution.rs diff --git a/src/bin/dicebot-cmd.rs b/src/bin/dicebot-cmd.rs index dc38d71..480b62c 100644 --- a/src/bin/dicebot-cmd.rs +++ b/src/bin/dicebot-cmd.rs @@ -30,7 +30,8 @@ async fn main() -> Result<(), BotError> { .expect("Could not create matrix client"), room: RoomContext { id: &room_id!("!fakeroomid:example.com"), - display_name: "fake room", + display_name: "fake room".to_owned(), + secure: false, }, username: "@localuser:example.com", message_body: &input, diff --git a/src/bot/command_execution.rs b/src/bot/command_execution.rs new file mode 100644 index 0000000..24da3e4 --- /dev/null +++ b/src/bot/command_execution.rs @@ -0,0 +1,122 @@ +use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor}; +use crate::context::{Context, RoomContext}; +use crate::db::sqlite::Database; +use crate::error::BotError; +use crate::matrix; +use futures::stream::{self, StreamExt}; +use log::{error, info}; +use matrix_sdk::{self, identifiers::EventId, room::Joined, Client}; +use std::clone::Clone; + +/// Handle responding to a single command being executed. Wil print +/// out the full result of that command. +pub(super) async fn handle_single_result( + client: &Client, + cmd_result: &ExecutionResult, + respond_to: &str, + room: &Joined, + event_id: EventId, +) { + if cmd_result.is_err() { + error!( + "Command execution error: {}", + cmd_result.as_ref().err().unwrap() + ); + } + + let html = cmd_result.message_html(respond_to); + matrix::send_message(client, room.room_id(), &html, Some(event_id)).await; +} + +/// Handle responding to multiple commands being executed. Will print +/// out how many commands succeeded and failed (if any failed). +pub(super) async fn handle_multiple_results( + client: &Client, + results: &[(String, ExecutionResult)], + respond_to: &str, + room: &Joined, +) { + let respond_to = format!( + "{}", + respond_to, respond_to + ); + + let errors: Vec<(&str, &ExecutionError)> = results + .into_iter() + .filter_map(|(cmd, result)| match result { + Err(e) => Some((cmd.as_ref(), e)), + _ => None, + }) + .collect(); + + for result in errors.iter() { + error!("Command execution error: '{}' - {}", result.0, result.1); + } + + let message = if errors.len() == 0 { + format!("{}: Executed {} commands", respond_to, results.len()) + } else { + let failures: Vec = errors + .iter() + .map(|&(cmd, err)| format!("{}: {}", cmd, err)) + .collect(); + + format!( + "{}: Executed {} commands ({} failed)\n\nFailures:\n{}", + respond_to, + results.len(), + errors.len(), + failures.join("\n") + ) + .replace("\n", "
") + }; + + matrix::send_message(client, room.room_id(), &message, None).await; +} + +/// Create a context for command execution. Can fai if the room +/// context creation fails. +async fn create_context<'a>( + db: &'a Database, + client: &'a Client, + room: &'a Joined, + sender: &'a str, + command: &'a str, +) -> Result, BotError> { + let room_ctx = RoomContext::new(room, sender).await?; + Ok(Context { + db: db.clone(), + matrix_client: client, + room: room_ctx, + username: &sender, + message_body: &command, + }) +} + +/// Attempt to execute all commands sent to the bot in a message. This +/// asynchronously executes all commands given to it. A Vec of all +/// commands and their execution results are returned. +pub(super) async fn execute( + commands: Vec<&str>, + db: &Database, + client: &Client, + room: &Joined, + sender: &str, +) -> Vec<(String, ExecutionResult)> { + stream::iter(commands) + .then(|command| async move { + match create_context(db, client, room, sender, command).await { + Err(e) => (command.to_owned(), Err(ExecutionError(e))), + Ok(ctx) => { + let cmd_result = execute_command(&ctx).await; + info!( + "[{}] {} executed: {}", + ctx.room.display_name, sender, command + ); + (command.to_owned(), cmd_result) + } + } + }) + .collect() + .await +} diff --git a/src/bot/mod.rs b/src/bot/mod.rs index 65def22..6a7d43b 100644 --- a/src/bot/mod.rs +++ b/src/bot/mod.rs @@ -1,20 +1,18 @@ -use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor}; +use crate::commands::{ExecutionError, ExecutionResult}; use crate::config::*; -use crate::context::{Context, RoomContext}; use crate::db::sqlite::Database; use crate::db::DbState; use crate::error::BotError; -use crate::matrix; use crate::state::DiceBotState; use dirs; -use futures::stream::{self, StreamExt}; -use log::{error, info}; +use log::info; use matrix_sdk::{self, identifiers::EventId, room::Joined, Client, ClientConfig, SyncSettings}; use std::clone::Clone; use std::path::PathBuf; use std::sync::{Arc, RwLock}; use url::Url; +mod command_execution; pub mod event_handlers; /// How many commands can be in one message. If the amount is higher @@ -53,72 +51,6 @@ fn create_client(config: &Config) -> Result { Ok(Client::new_with_config(homeserver_url, client_config)?) } -/// Handle responding to a single command being executed. Wil print -/// out the full result of that command. -async fn handle_single_result( - client: &Client, - cmd_result: &ExecutionResult, - respond_to: &str, - room: &Joined, - event_id: EventId, -) { - if cmd_result.is_err() { - error!( - "Command execution error: {}", - cmd_result.as_ref().err().unwrap() - ); - } - - let html = cmd_result.message_html(respond_to); - matrix::send_message(client, room.room_id(), &html, Some(event_id)).await; -} - -/// Handle responding to multiple commands being executed. Will print -/// out how many commands succeeded and failed (if any failed). -async fn handle_multiple_results( - client: &Client, - results: &[(String, ExecutionResult)], - respond_to: &str, - room: &Joined, -) { - let respond_to = format!( - "{}", - respond_to, respond_to - ); - - let errors: Vec<(&str, &ExecutionError)> = results - .into_iter() - .filter_map(|(cmd, result)| match result { - Err(e) => Some((cmd.as_ref(), e)), - _ => None, - }) - .collect(); - - for result in errors.iter() { - error!("Command execution error: '{}' - {}", result.0, result.1); - } - - let message = if errors.len() == 0 { - format!("{}: Executed {} commands", respond_to, results.len()) - } else { - let failures: Vec = errors - .iter() - .map(|&(cmd, err)| format!("{}: {}", cmd, err)) - .collect(); - - format!( - "{}: Executed {} commands ({} failed)\n\nFailures:\n{}", - respond_to, - results.len(), - errors.len(), - failures.join("\n") - ) - .replace("\n", "
") - }; - - matrix::send_message(client, room.room_id(), &message, None).await; -} - impl DiceBot { /// Create a new dicebot with the given configuration and state /// actor. This function returns a Result because it is possible @@ -184,11 +116,9 @@ impl DiceBot { async fn execute_commands( &self, room: &Joined, - sender_username: &str, + sender: &str, msg_body: &str, ) -> Vec<(String, ExecutionResult)> { - let room_name: &str = &room.display_name().await.ok().unwrap_or_default(); - let commands: Vec<&str> = msg_body .lines() .filter(|line| line.starts_with("!")) @@ -197,22 +127,7 @@ impl DiceBot { //Up to 50 commands allowed, otherwise we send back an error. let results: Vec<(String, ExecutionResult)> = if commands.len() < MAX_COMMANDS_PER_MESSAGE { - stream::iter(commands) - .then(|command| async move { - let ctx = Context { - db: self.db.clone(), - matrix_client: &self.client, - room: RoomContext::new_with_name(&room, room_name), - username: &sender_username, - message_body: &command, - }; - - let cmd_result = execute_command(&ctx).await; - info!("[{}] {} executed: {}", room_name, sender_username, command); - (command.to_owned(), cmd_result) - }) - .collect() - .await + command_execution::execute(commands, &self.db, &self.client, room, sender).await } else { vec![( "".to_owned(), @@ -232,7 +147,7 @@ impl DiceBot { ) { if results.len() >= 1 { if results.len() == 1 { - handle_single_result( + command_execution::handle_single_result( &self.client, &results[0].1, sender_username, @@ -241,7 +156,13 @@ impl DiceBot { ) .await; } else if results.len() > 1 { - handle_multiple_results(&self.client, &results, sender_username, &room).await; + command_execution::handle_multiple_results( + &self.client, + &results, + sender_username, + &room, + ) + .await; } } } diff --git a/src/cofd/dice.rs b/src/cofd/dice.rs index 998fcb3..6c6f4e7 100644 --- a/src/cofd/dice.rs +++ b/src/cofd/dice.rs @@ -333,7 +333,8 @@ mod tests { () => { crate::context::RoomContext { id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"), - display_name: "displayname", + display_name: "displayname".to_owned(), + secure: false, } }; } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fd2d50e..e226402 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -117,7 +117,8 @@ mod tests { () => { crate::context::RoomContext { id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"), - display_name: "displayname", + display_name: "displayname".to_owned(), + secure: false, } }; } diff --git a/src/context.rs b/src/context.rs index 04f2b40..da8484f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,7 +1,9 @@ use crate::db::sqlite::Database; -use matrix_sdk::identifiers::RoomId; +use crate::error::BotError; +use matrix_sdk::identifiers::{RoomId, UserId}; use matrix_sdk::room::Joined; use matrix_sdk::Client; +use std::convert::TryFrom; /// A context carried through the system providing access to things /// like the database. @@ -18,19 +20,52 @@ impl Context<'_> { pub fn room_id(&self) -> &RoomId { self.room.id } + + pub fn is_secure(&self) -> bool { + self.room.secure + } } #[derive(Clone)] pub struct RoomContext<'a> { pub id: &'a RoomId, - pub display_name: &'a str, + pub display_name: String, + pub secure: bool, } impl RoomContext<'_> { - pub fn new_with_name<'a>(room: &'a Joined, display_name: &'a str) -> RoomContext<'a> { - RoomContext { + pub async fn new_with_name<'a>( + room: &'a Joined, + display_name: String, + sending_user: &str, + ) -> Result, BotError> { + // TODO is_direct is a hack; should set rooms to Direct + // Message upon joining, if other contact has requested it. + // Waiting on SDK support. + let sending_user = UserId::try_from(sending_user)?; + let user_in_room = room.get_member(&sending_user).await.ok().is_some(); + let is_direct = room.joined_members().await?.len() == 2; + + Ok(RoomContext { id: room.room_id(), display_name, - } + secure: room.is_encrypted() && is_direct && user_in_room, + }) + } + + pub async fn new<'a>( + room: &'a Joined, + sending_user: &str, + ) -> Result, BotError> { + Self::new_with_name( + &room, + room.display_name() + .await + .ok() + .unwrap_or_default() + .to_string(), + sending_user, + ) + .await } } diff --git a/src/cthulhu/dice.rs b/src/cthulhu/dice.rs index 13444dd..37d194f 100644 --- a/src/cthulhu/dice.rs +++ b/src/cthulhu/dice.rs @@ -427,7 +427,8 @@ mod tests { () => { crate::context::RoomContext { id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"), - display_name: "displayname", + display_name: "displayname".to_owned(), + secure: false, } }; } diff --git a/src/error.rs b/src/error.rs index c180dd4..505ac3c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -75,6 +75,9 @@ pub enum BotError { #[error("could not convert to proper integer type")] TryFromIntError(#[from] std::num::TryFromIntError), + + #[error("identifier error: {0}")] + IdentifierError(#[from] matrix_sdk::identifiers::Error), } #[derive(Error, Debug)]