diff options
| -rw-r--r-- | CHANGELOG | 3 | ||||
| -rw-r--r-- | documentation/mumdrc.5 | 17 | ||||
| -rw-r--r-- | documentation/mumdrc.txt | 9 | ||||
| -rw-r--r-- | mumctl/src/main.rs | 64 | ||||
| -rw-r--r-- | mumd/src/command.rs | 34 | ||||
| -rw-r--r-- | mumd/src/network/tcp.rs | 58 | ||||
| -rw-r--r-- | mumd/src/state.rs | 47 | ||||
| -rw-r--r-- | mumlib/src/config.rs | 14 | ||||
| -rw-r--r-- | mumlib/src/error.rs | 2 |
9 files changed, 190 insertions, 58 deletions
@@ -23,6 +23,9 @@ Added * Server passwords. Thanks @rbran! * Added support for sending and receiving text messages. * See a list of occured events with +mumctl events+. + * Invalid server certificates are now rejected by default and need to be + explicitly allowed either when connecting or permanently per server or + globally. Changed ~~~~~~~ diff --git a/documentation/mumdrc.5 b/documentation/mumdrc.5 index ac0a2f5..00e8265 100644 --- a/documentation/mumdrc.5 +++ b/documentation/mumdrc.5 @@ -2,12 +2,12 @@ .\" Title: mumdrc .\" Author: [see the "AUTHOR(S)" section] .\" Generator: Asciidoctor 2.0.15 -.\" Date: 2021-04-10 +.\" Date: 2021-06-08 .\" Manual: \ \& .\" Source: \ \& .\" Language: English .\" -.TH "MUMDRC" "5" "2021-04-10" "\ \&" "\ \&" +.TH "MUMDRC" "5" "2021-06-08" "\ \&" "\ \&" .ie \n(.g .ds Aq \(aq .el .ds Aq ' .ss \n[.ss] 0 @@ -36,6 +36,12 @@ using mumctl(1). .sp The following configuration values are supported: .sp +accept_all_invalid_certs +.RS 4 +Whether to connect to a server that supplies an invalid server certificate. +This is overriden by server\-specific settings. Default false. +.RE +.sp audio.input_volume .RS 4 Default 1.0. @@ -73,6 +79,13 @@ password .RS 4 The password to supply to the server. (Optional) .RE +.sp +accept_invalid_cert +.RS 4 +Whether to connect to this server even if it supplies an invalid server +certificate. This overrides the global accept_all_invalid_certs if set. +Default false. +.RE .SH "AUTHORS" .sp Gustav Sörnäs and Eskil Queseth. diff --git a/documentation/mumdrc.txt b/documentation/mumdrc.txt index ed54b87..0579581 100644 --- a/documentation/mumdrc.txt +++ b/documentation/mumdrc.txt @@ -14,6 +14,10 @@ using mumctl(1). The following configuration values are supported: +accept_all_invalid_certs :: + Whether to connect to a server that supplies an invalid server certificate. + This is overriden by server-specific settings. Default false. + audio.input_volume :: Default 1.0. @@ -38,6 +42,11 @@ username :: password :: The password to supply to the server. (Optional) +accept_invalid_cert :: + Whether to connect to this server even if it supplies an invalid server + certificate. This overrides the global accept_all_invalid_certs if set. + Default false. + Authors ------- diff --git a/mumctl/src/main.rs b/mumctl/src/main.rs index 1fb2a45..5b2bc05 100644 --- a/mumctl/src/main.rs +++ b/mumctl/src/main.rs @@ -55,6 +55,8 @@ enum Command { password: Option<String>, #[structopt(short = "p", long = "port")] port: Option<u16>, + #[structopt(long = "accept-invalid-cert")] + accept_invalid_cert: bool, }, /// Disconnect from the currently connected server Disconnect, @@ -238,10 +240,12 @@ fn match_opt() -> Result<(), Error> { username, password, port, + accept_invalid_cert: cli_accept_invalid_cert, } => { let port = port.unwrap_or(mumlib::DEFAULT_PORT); - let (host, username, password, port) = + + let (host, username, password, port, server_accept_invalid_cert) = match config.servers.iter().find(|e| e.name == host) { Some(server) => ( &server.host, @@ -252,27 +256,46 @@ fn match_opt() -> Result<(), Error> { .ok_or(CliError::NoUsername)?, server.password.as_ref().or(password.as_ref()), server.port.unwrap_or(port), + server.accept_invalid_cert, ), None => ( &host, username.as_ref().ok_or(CliError::NoUsername)?, password.as_ref(), port, + None, ), }; + let config_accept_invalid_cert = server_accept_invalid_cert + .or(config.allow_invalid_server_cert); + let specified_accept_invalid_cert = cli_accept_invalid_cert || config_accept_invalid_cert.is_some(); + let response = send_command(MumCommand::ServerConnect { host: host.to_string(), port, username: username.to_string(), password: password.map(|x| x.to_string()), - accept_invalid_cert: true, //TODO - })??; - if let Some(CommandResponse::ServerConnect { welcome_message, server_state }) = response { - parse_state(&server_state); - if let Some(message) = welcome_message { - println!("\nWelcome: {}", message); + accept_invalid_cert: cli_accept_invalid_cert || config_accept_invalid_cert.unwrap_or(false), + })?; + match response { + Ok(Some(CommandResponse::ServerConnect { welcome_message , server_state })) => { + parse_state(&server_state); + if let Some(message) = welcome_message { + println!("\nWelcome: {}", message); + } } + Err(mumlib::error::Error::ServerCertReject) => { + error!("Connection rejected since the server supplied an invalid certificate."); + if !specified_accept_invalid_cert { + eprintln!("help: If you trust this server anyway, you can do any of the following to connect:"); + eprintln!(" 1. Temporarily trust this server by passing --accept-invalid-cert when connecting."); + eprintln!(" 2. Permanently trust this server by setting accept_invalid_cert=true in the server's config."); + eprintln!(" 3. Permantently trust all invalid certificates by setting accept_all_invalid_certs=true globally"); + } + } + Ok(other) => unreachable!("Response should only be a ServerConnect or ServerCertReject. Got {:?}", other), + Err(e) => return Err(e.into()), } } Command::Disconnect => { @@ -318,6 +341,11 @@ fn match_opt() -> Result<(), Error> { config.audio.output_volume = Some(volume); } } + "accept_all_invalid_certs" => { + if let Ok(b) = value.parse() { + config.allow_invalid_server_cert = Some(b); + } + } _ => { return Err(CliError::ConfigKeyNotFound(key).into()); } @@ -462,7 +490,7 @@ fn match_server_command(server_command: Server, config: &mut Config) -> Result<( match (key.as_deref(), value) { (None, _) => { print!( - "{}{}{}{}", + "{}{}{}{}{}", format!("host: {}\n", server.host.to_string()), server .port @@ -478,6 +506,10 @@ fn match_server_command(server_command: Server, config: &mut Config) -> Result<( .as_ref() .map(|s| format!("password: {}\n", s)) .unwrap_or_else(|| "".to_string()), + server + .accept_invalid_cert + .map(|b| format!("accept_invalid_cert: {}\n", if b { "true" } else { "false" })) + .unwrap_or_else(|| "".to_string()), ); } (Some("name"), None) => { @@ -510,6 +542,15 @@ fn match_server_command(server_command: Server, config: &mut Config) -> Result<( .ok_or(CliError::NotSet("password".to_string()))? ); } + (Some("accept_invalid_cert"), None) => { + println!( + "{}", + server + .accept_invalid_cert + .map(|b| b.to_string()) + .ok_or(CliError::NotSet("accept_invalid_cert".to_string()))? + ); + } (Some("name"), Some(_)) => { return Err(CliError::UseServerRename)?; } @@ -526,6 +567,12 @@ fn match_server_command(server_command: Server, config: &mut Config) -> Result<( server.password = Some(value); //TODO ask stdin if empty } + (Some("accept_invalid_cert"), Some(value)) => { + match value.parse() { + Ok(b) => server.accept_invalid_cert = Some(b), + Err(e) => warn!("{}", e) + } + } (Some(_), _) => { return Err(CliError::ConfigKeyNotFound(key.unwrap()))?; } @@ -555,6 +602,7 @@ fn match_server_command(server_command: Server, config: &mut Config) -> Result<( port, username, password, + accept_invalid_cert: None, }); } } diff --git a/mumd/src/command.rs b/mumd/src/command.rs index 410751a..2069178 100644 --- a/mumd/src/command.rs +++ b/mumd/src/command.rs @@ -4,10 +4,7 @@ use crate::state::{ExecutionContext, State}; use log::*; use mumble_protocol::{control::ControlPacket, Serverbound}; use mumlib::command::{Command, CommandResponse}; -use std::sync::{ - atomic::{AtomicU64, Ordering}, - Arc, RwLock, -}; +use std::{rc::Rc, sync::{Arc, RwLock, atomic::{AtomicBool, AtomicU64, Ordering}}}; use tokio::sync::{mpsc, watch}; pub async fn handle( @@ -32,16 +29,25 @@ pub async fn handle( &mut connection_info_sender, ); match event { - ExecutionContext::TcpEventCallback(event, generator) => { - tcp_event_queue.register_callback( - event, - Box::new(move |e| { - let response = generator(e); - for response in response { - response_sender.send(response).unwrap(); - } - }), - ); + ExecutionContext::TcpEventCallback(callbacks) => { + // A shared bool ensures that only one of the supplied callbacks is run. + let should_handle = Rc::new(AtomicBool::new(true)); + for (event, generator) in callbacks { + let should_handle = Rc::clone(&should_handle); + let response_sender = response_sender.clone(); + tcp_event_queue.register_callback( + event, + Box::new(move |e| { + // If should_handle == true no other callback has been run yet. + if should_handle.swap(false, Ordering::Relaxed) { + let response = generator(e); + for response in response { + response_sender.send(response).unwrap(); + } + } + }), + ); + } } ExecutionContext::TcpEventSubscriber(event, mut handler) => tcp_event_queue .register_subscriber( diff --git a/mumd/src/network/tcp.rs b/mumd/src/network/tcp.rs index 4a753bf..f620a32 100644 --- a/mumd/src/network/tcp.rs +++ b/mumd/src/network/tcp.rs @@ -1,14 +1,12 @@ +use crate::error::{ServerSendError, TcpError}; use crate::network::ConnectionInfo; +use crate::notifications; use crate::state::{State, StatePhase}; -use crate::{ - error::{ServerSendError, TcpError}, - notifications, -}; -use log::*; use futures_util::select; use futures_util::stream::{SplitSink, SplitStream, Stream}; use futures_util::{FutureExt, SinkExt, StreamExt}; +use log::*; use mumble_protocol::control::{msgs, ClientControlCodec, ControlCodec, ControlPacket}; use mumble_protocol::crypt::ClientCryptState; use mumble_protocol::voice::VoicePacket; @@ -36,17 +34,33 @@ type TcpReceiver = pub(crate) type TcpEventCallback = Box<dyn FnOnce(TcpEventData)>; pub(crate) type TcpEventSubscriber = Box<dyn FnMut(TcpEventData) -> bool>; //the bool indicates if it should be kept or not -#[derive(Debug, Clone, Hash, Eq, PartialEq)] +/// Why the TCP was disconnected. +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] +pub enum DisconnectedReason { + InvalidTls, + User, + TcpError, +} + +/// Something a callback can register to. Data is sent via a respective [TcpEventData]. +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub enum TcpEvent { Connected, //fires when the client has connected to a server - Disconnected, //fires when the client has disconnected from a server + Disconnected(DisconnectedReason), //fires when the client has disconnected from a server TextMessage, //fires when a text message comes in } +/// When a [TcpEvent] occurs, this contains the data for the event. +/// +/// The events are picked up by a [crate::state::ExecutionContext]. +/// +/// Having two different types might feel a bit confusing. Essentially, a +/// callback _registers_ to a [TcpEvent] but _takes_ a [TcpEventData] as +/// parameter. #[derive(Clone)] pub enum TcpEventData<'a> { Connected(Result<&'a msgs::ServerSync, mumlib::Error>), - Disconnected, + Disconnected(DisconnectedReason), TextMessage(&'a msgs::TextMessage), } @@ -54,7 +68,7 @@ impl<'a> From<&TcpEventData<'a>> for TcpEvent { fn from(t: &TcpEventData) -> Self { match t { TcpEventData::Connected(_) => TcpEvent::Connected, - TcpEventData::Disconnected => TcpEvent::Disconnected, + TcpEventData::Disconnected(reason) => TcpEvent::Disconnected(*reason), TcpEventData::TextMessage(_) => TcpEvent::TextMessage, } } @@ -142,12 +156,25 @@ pub async fn handle( } return Err(TcpError::NoConnectionInfoReceived); }; - let (mut sink, stream) = connect( + let connect_result = connect( connection_info.socket_addr, connection_info.hostname, connection_info.accept_invalid_cert, ) - .await?; + .await; + + let (mut sink, stream) = match connect_result { + Ok(ok) => ok, + Err(TcpError::TlsConnectError(_)) => { + warn!("Invalid TLS"); + state.read().unwrap().broadcast_phase(StatePhase::Disconnected); + event_queue.resolve(TcpEventData::Disconnected(DisconnectedReason::InvalidTls)); + continue; + } + Err(e) => { + return Err(e); + } + }; // Handshake (omitting `Version` message for brevity) let (username, password) = { @@ -170,7 +197,7 @@ pub async fn handle( let phase_watcher_inner = phase_watcher.clone(); - run_until( + let result = run_until( |phase| matches!(phase, StatePhase::Disconnected), async { select! { @@ -192,9 +219,12 @@ pub async fn handle( phase_watcher, ) .await - .unwrap_or(Ok(()))?; + .unwrap_or(Ok(())); - event_queue.resolve(TcpEventData::Disconnected); + match result { + Ok(()) => event_queue.resolve(TcpEventData::Disconnected(DisconnectedReason::User)), + Err(_) => event_queue.resolve(TcpEventData::Disconnected(DisconnectedReason::TcpError)), + } debug!("Fully disconnected TCP stream, waiting for new connection info"); } diff --git a/mumd/src/state.rs b/mumd/src/state.rs index d12b5b6..d2d77b1 100644 --- a/mumd/src/state.rs +++ b/mumd/src/state.rs @@ -2,7 +2,7 @@ pub mod channel; pub mod server; pub mod user; -use crate::audio::{AudioInput, AudioOutput, NotificationEvents}; +use crate::{audio::{AudioInput, AudioOutput, NotificationEvents}, network::tcp::DisconnectedReason}; use crate::error::StateError; use crate::network::tcp::{TcpEvent, TcpEventData}; use crate::network::{ConnectionInfo, VoiceStreamType}; @@ -27,8 +27,10 @@ use std::{ use tokio::sync::{mpsc, watch}; macro_rules! at { - ($event:expr, $generator:expr) => { - ExecutionContext::TcpEventCallback($event, Box::new($generator)) + ( $( $event:expr => $generator:expr ),+ $(,)? ) => { + ExecutionContext::TcpEventCallback(vec![ + $( ($event, Box::new($generator)), )* + ]) }; } @@ -42,7 +44,7 @@ type Responses = Box<dyn Iterator<Item = mumlib::error::Result<Option<CommandRes //TODO give me a better name pub enum ExecutionContext { - TcpEventCallback(TcpEvent, Box<dyn FnOnce(TcpEventData) -> Responses>), + TcpEventCallback(Vec<(TcpEvent, Box<dyn FnOnce(TcpEventData) -> Responses>)>), TcpEventSubscriber( TcpEvent, Box< @@ -606,23 +608,28 @@ pub fn handle_command( ))) .unwrap(); let state = Arc::clone(&og_state); - at!(TcpEvent::Connected, move |res| { - //runs the closure when the client is connected - if let TcpEventData::Connected(res) = res { - Box::new(iter::once(res.map(|msg| { - Some(CommandResponse::ServerConnect { - welcome_message: if msg.has_welcome_text() { - Some(msg.get_welcome_text().to_string()) - } else { - None - }, - server_state: state.read().unwrap().server.as_ref().unwrap().into(), - }) - }))) - } else { - unreachable!("callback should be provided with a TcpEventData::Connected"); + at!( + TcpEvent::Connected => move |res| { + //runs the closure when the client is connected + if let TcpEventData::Connected(res) = res { + Box::new(iter::once(res.map(|msg| { + Some(CommandResponse::ServerConnect { + welcome_message: if msg.has_welcome_text() { + Some(msg.get_welcome_text().to_string()) + } else { + None + }, + server_state: state.read().unwrap().server.as_ref().unwrap().into(), + }) + }))) + } else { + unreachable!("callback should be provided with a TcpEventData::Connected"); + } + }, + TcpEvent::Disconnected(DisconnectedReason::InvalidTls) => |_| { + Box::new(iter::once(Err(Error::ServerCertReject))) } - }) + ) } Command::ServerDisconnect => { if !matches!(*state.phase_receiver().borrow(), StatePhase::Connected(_)) { diff --git a/mumlib/src/config.rs b/mumlib/src/config.rs index 1bd3784..3edef37 100644 --- a/mumlib/src/config.rs +++ b/mumlib/src/config.rs @@ -10,8 +10,15 @@ use std::path::{Path, PathBuf}; use toml::value::Array; use toml::Value; +/// A TOML-friendly version of [Config]. +/// +/// Values need to be placed before tables due to how TOML works. #[derive(Debug, Deserialize, Serialize)] struct TOMLConfig { + // Values + accept_all_invalid_certs: Option<bool>, + + // Tables audio: Option<AudioConfig>, servers: Option<Array>, } @@ -20,6 +27,10 @@ struct TOMLConfig { pub struct Config { pub audio: AudioConfig, pub servers: Vec<ServerConfig>, + /// Whether we allow connecting to servers with invalid server certificates. + /// + /// None implies false but we can show a better message to the user. + pub allow_invalid_server_cert: Option<bool>, } impl Config { @@ -64,6 +75,7 @@ pub struct ServerConfig { pub port: Option<u16>, pub username: Option<String>, pub password: Option<String>, + pub accept_invalid_cert: Option<bool>, } impl ServerConfig { @@ -105,6 +117,7 @@ impl TryFrom<TOMLConfig> for Config { }) .transpose()? .unwrap_or_default(), + allow_invalid_server_cert: config.accept_all_invalid_certs, }) } } @@ -125,6 +138,7 @@ impl From<Config> for TOMLConfig { .map(|s| Value::try_from::<ServerConfig>(s).unwrap()) .collect(), ), + accept_all_invalid_certs: config.allow_invalid_server_cert, } } } diff --git a/mumlib/src/error.rs b/mumlib/src/error.rs index 2cb3927..30b61ee 100644 --- a/mumlib/src/error.rs +++ b/mumlib/src/error.rs @@ -13,6 +13,7 @@ pub enum Error { InvalidServerPassword, Unimplemented, NotConnectedToChannel, + ServerCertReject, } impl std::error::Error for Error {} @@ -30,6 +31,7 @@ impl fmt::Display for Error { Error::InvalidServerPassword => write!(f, "Invalid server password"), Error::Unimplemented => write!(f, "Unimplemented"), Error::NotConnectedToChannel => write!(f, "Not connected to a channel"), + Error::ServerCertReject => write!(f, "Invalid server certificate"), } } } |