diff --git a/nix/module.nix b/nix/module.nix index 3bd6bda..4d6c996 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -38,7 +38,7 @@ in socket_path = lib.mkOption { type = lib.types.path; default = "/run/mysqladm/mysqladm.sock"; - description = "Path to the MySQL socket"; + description = "Path to the mysqladm socket"; }; }; @@ -96,15 +96,20 @@ in } ]; - systemd.services."mysqladm@" = { + systemd.services."mysqladm" = { description = "MySQL administration tool for non-admin users"; restartTriggers = [ config.environment.etc."mysqladm/config.toml".source ]; + requires = [ "mysqladm.socket" ]; serviceConfig = { Type = "notify"; ExecStart = "${lib.getExe cfg.package} ${cfg.logLevel} server --systemd socket-activate"; WatchdogSec = 15; + # Although this is a multi-instance unit, the constant `User` field is needed + # for authentication via mysql's auth_socket plugin to work. + User = "mysqladm"; + Group = "mysqladm"; DynamicUser = true; ConfigurationDirectory = "mysqladm"; @@ -117,8 +122,12 @@ in PrivateNetwork = false; PrivateIPC = false; - IPAddressDeny = - lib.optionals (lib.elem cfg.settings.mysql.host [ null "localhost" "127.0.0.1" ]) [ "any" ]; + IPAddressDeny = "any"; + IPAddressAllow = [ + "127.0.0.0/8" + ] ++ lib.optionals (cfg.settings.mysql.host != null) [ + cfg.settings.mysql.host + ]; RestrictAddressFamilies = [ "AF_UNIX" ] ++ (lib.optionals (cfg.settings.mysql.host != null) [ "AF_INET" "AF_INET6" ]); @@ -162,7 +171,7 @@ in wantedBy = [ "sockets.target" ]; socketConfig = { ListenStream = cfg.settings.server.socket_path; - Accept = "yes"; + Accept = "no"; PassCredentials = true; }; }; diff --git a/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs b/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs index 7ebf152..5167cb1 100644 --- a/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs +++ b/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs @@ -151,8 +151,11 @@ pub fn main() -> anyhow::Result<()> { return Ok(()); } - let server_connection = - bootstrap_server_connection_and_drop_privileges(args.server_socket_path, args.config)?; + let server_connection = bootstrap_server_connection_and_drop_privileges( + args.server_socket_path, + args.config, + Default::default(), + )?; let command = match args.command { Some(command) => command, diff --git a/src/cli/mysql_admutils_compatibility/mysql_useradm.rs b/src/cli/mysql_admutils_compatibility/mysql_useradm.rs index 3cca747..1618e09 100644 --- a/src/cli/mysql_admutils_compatibility/mysql_useradm.rs +++ b/src/cli/mysql_admutils_compatibility/mysql_useradm.rs @@ -124,8 +124,11 @@ pub fn main() -> anyhow::Result<()> { } }; - let server_connection = - bootstrap_server_connection_and_drop_privileges(args.server_socket_path, args.config)?; + let server_connection = bootstrap_server_connection_and_drop_privileges( + args.server_socket_path, + args.config, + Default::default(), + )?; tokio_run_command(command, server_connection)?; diff --git a/src/core/bootstrap.rs b/src/core/bootstrap.rs index ff18b55..ed1595f 100644 --- a/src/core/bootstrap.rs +++ b/src/core/bootstrap.rs @@ -1,33 +1,43 @@ use std::{fs, path::PathBuf}; use anyhow::Context; +use clap_verbosity_flag::Verbosity; use nix::libc::{EXIT_SUCCESS, exit}; use std::os::unix::net::UnixStream as StdUnixStream; use tokio::net::UnixStream as TokioUnixStream; use crate::{ - core::common::{DEFAULT_CONFIG_PATH, DEFAULT_SOCKET_PATH, UnixUser}, + core::common::{ + DEFAULT_CONFIG_PATH, DEFAULT_SOCKET_PATH, UnixUser, executable_is_suid_or_sgid, + }, server::{config::read_config_from_path, server_loop::handle_requests_for_single_session}, }; -// TODO: this function is security critical, it should be integration tested -// in isolation. -/// Drop privileges to the real user and group of the process. -/// If the process is not running with elevated privileges, this function -/// is a no-op. -pub fn drop_privs() -> anyhow::Result<()> { - log::debug!("Dropping privileges"); - let real_uid = nix::unistd::getuid(); - let real_gid = nix::unistd::getgid(); +/// Determine whether we will make a connection to an external server +/// or start an internal server with elevated privileges. +/// +/// If neither is feasible, an error is returned. +fn will_connect_to_external_server( + server_socket_path: Option<&PathBuf>, + config_path: Option<&PathBuf>, +) -> anyhow::Result { + if server_socket_path.is_some() { + return Ok(true); + } - nix::unistd::setuid(real_uid).context("Failed to drop privileges")?; - nix::unistd::setgid(real_gid).context("Failed to drop privileges")?; + if config_path.is_some() { + return Ok(false); + } - debug_assert_eq!(nix::unistd::getuid(), real_uid); - debug_assert_eq!(nix::unistd::getgid(), real_gid); + if fs::metadata(DEFAULT_SOCKET_PATH).is_ok() { + return Ok(true); + } - log::debug!("Privileges dropped successfully"); - Ok(()) + if fs::metadata(DEFAULT_CONFIG_PATH).is_ok() { + return Ok(false); + } + + anyhow::bail!("No socket path or config path provided, and no default socket or config found"); } /// This function is used to bootstrap the connection to the server. @@ -45,31 +55,44 @@ pub fn drop_privs() -> anyhow::Result<()> { /// with the socket for the server. /// /// If neither of these options are available, the function will fail. +/// +/// Note that this function is also responsible for setting up logging, +/// because in the case of an internal server, we need to drop privileges +/// before we can initialize logging. pub fn bootstrap_server_connection_and_drop_privileges( server_socket_path: Option, - config_path: Option, + config: Option, + verbose: Verbosity, ) -> anyhow::Result { - if server_socket_path.is_some() && config_path.is_some() { - anyhow::bail!("Cannot provide both a socket path and a config path"); + if will_connect_to_external_server(server_socket_path.as_ref(), config.as_ref())? { + assert!( + !executable_is_suid_or_sgid()?, + "The executable should not be SUID or SGID when connecting to an external server" + ); + + env_logger::Builder::new() + .filter_level(verbose.log_level_filter()) + .init(); + + connect_to_external_server(server_socket_path) + } else { + // NOTE: We need to be really careful with the code up until this point, + // as we might be running with elevated privileges. + let server_connection = bootstrap_internal_server_and_drop_privs(config)?; + + env_logger::Builder::new() + .filter_level(verbose.log_level_filter()) + .init(); + + Ok(server_connection) } - - log::debug!("Starting the server connection bootstrap process"); - - let socket = bootstrap_server_connection(server_socket_path, config_path)?; - - drop_privs()?; - - Ok(socket) } -/// Inner function for [`bootstrap_server_connection_and_drop_privileges`]. -/// See that function for more information. -fn bootstrap_server_connection( - socket_path: Option, - config_path: Option, +fn connect_to_external_server( + server_socket_path: Option, ) -> anyhow::Result { // TODO: ensure this is both readable and writable - if let Some(socket_path) = socket_path { + if let Some(socket_path) = server_socket_path { log::debug!("Connecting to socket at {:?}", socket_path); return match StdUnixStream::connect(socket_path) { Ok(socket) => Ok(socket), @@ -80,15 +103,6 @@ fn bootstrap_server_connection( }, }; } - if let Some(config_path) = config_path { - // ensure config exists and is readable - if fs::metadata(&config_path).is_err() { - return Err(anyhow::anyhow!("Config file not found or not readable")); - } - - log::debug!("Starting server with config at {:?}", config_path); - return invoke_server_with_config(config_path); - } if fs::metadata(DEFAULT_SOCKET_PATH).is_ok() { log::debug!("Connecting to default socket at {:?}", DEFAULT_SOCKET_PATH); @@ -102,13 +116,60 @@ fn bootstrap_server_connection( }; } + anyhow::bail!("No socket path provided, and no default socket found"); +} + +// TODO: this function is security critical, it should be integration tested +// in isolation. +/// Drop privileges to the real user and group of the process. +/// If the process is not running with elevated privileges, this function +/// is a no-op. +fn drop_privs() -> anyhow::Result<()> { + log::debug!("Dropping privileges"); + let real_uid = nix::unistd::getuid(); + let real_gid = nix::unistd::getgid(); + + nix::unistd::setuid(real_uid).context("Failed to drop privileges")?; + nix::unistd::setgid(real_gid).context("Failed to drop privileges")?; + + debug_assert_eq!(nix::unistd::getuid(), real_uid); + debug_assert_eq!(nix::unistd::getgid(), real_gid); + + log::debug!("Privileges dropped successfully"); + Ok(()) +} + +fn bootstrap_internal_server_and_drop_privs( + config_path: Option, +) -> anyhow::Result { + if let Some(config_path) = config_path { + if !executable_is_suid_or_sgid()? { + anyhow::bail!("Executable is not SUID/SGID - refusing to start internal sever"); + } + + // ensure config exists and is readable + if fs::metadata(&config_path).is_err() { + return Err(anyhow::anyhow!("Config file not found or not readable")); + } + + log::debug!("Starting server with config at {:?}", config_path); + let socket = invoke_server_with_config(config_path)?; + drop_privs()?; + return Ok(socket); + }; + let config_path = PathBuf::from(DEFAULT_CONFIG_PATH); if fs::metadata(&config_path).is_ok() { + if !executable_is_suid_or_sgid()? { + anyhow::bail!("Executable is not SUID/SGID - refusing to start internal sever"); + } log::debug!("Starting server with default config at {:?}", config_path); - return invoke_server_with_config(config_path); - } + let socket = invoke_server_with_config(config_path)?; + drop_privs()?; + return Ok(socket); + }; - anyhow::bail!("No socket path or config path provided, and no default socket or config found"); + anyhow::bail!("No config path provided, and no default config found"); } // TODO: we should somehow ensure that the forked process is killed on completion, diff --git a/src/core/common.rs b/src/core/common.rs index 151cd5d..254e369 100644 --- a/src/core/common.rs +++ b/src/core/common.rs @@ -1,5 +1,6 @@ use anyhow::Context; use nix::unistd::{Group as LibcGroup, User as LibcUser}; +use std::{fs, os::unix::fs::PermissionsExt}; #[cfg(not(target_os = "macos"))] use std::ffi::CString; diff --git a/src/main.rs b/src/main.rs index 2782011..e67b72d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -124,14 +124,13 @@ fn main() -> anyhow::Result<()> { return Ok(()); } - let server_connection = - bootstrap_server_connection_and_drop_privileges(args.server_socket_path, args.config)?; + let connection = bootstrap_server_connection_and_drop_privileges( + args.server_socket_path, + args.config, + args.verbose, + )?; - env_logger::Builder::new() - .filter_level(args.verbose.log_level_filter()) - .init(); - - tokio_run_command(args.command, server_connection)?; + tokio_run_command(args.command, connection)?; Ok(()) } diff --git a/src/server/command.rs b/src/server/command.rs index 0b93606..e8ff2b2 100644 --- a/src/server/command.rs +++ b/src/server/command.rs @@ -1,23 +1,16 @@ -use std::os::fd::FromRawFd; use std::path::PathBuf; use anyhow::Context; use clap::Parser; use clap_verbosity_flag::Verbosity; -use futures::SinkExt; -use indoc::concatdoc; use systemd_journal_logger::JournalLog; -use std::os::unix::net::UnixStream as StdUnixStream; -use tokio::net::UnixStream as TokioUnixStream; - -use crate::core::common::UnixUser; -use crate::core::protocol::{Response, create_server_to_client_message_stream}; -use crate::server::config::read_config_from_path_with_arg_overrides; -use crate::server::server_loop::listen_for_incoming_connections; use crate::server::{ - config::{ServerConfig, ServerConfigArgs}, - server_loop::handle_requests_for_single_session, + config::{ServerConfigArgs, read_config_from_path_with_arg_overrides}, + server_loop::{ + listen_for_incoming_connections_with_socket_path, + listen_for_incoming_connections_with_systemd_socket, + }, }; #[derive(Parser, Debug, Clone)] @@ -97,7 +90,9 @@ pub async fn handle_command( let config = read_config_from_path_with_arg_overrides(config_path, args.config_overrides)?; match args.subcmd { - ServerCommand::Listen => listen_for_incoming_connections(socket_path, config).await, + ServerCommand::Listen => { + listen_for_incoming_connections_with_socket_path(socket_path, config).await + } ServerCommand::SocketActivate => { if !args.systemd { anyhow::bail!(concat!( @@ -106,7 +101,7 @@ pub async fn handle_command( )); } - socket_activate(config).await + listen_for_incoming_connections_with_systemd_socket(config).await } } } @@ -136,73 +131,3 @@ fn start_watchdog_thread_if_enabled() { log::debug!("Systemd watchdog not enabled, skipping watchdog thread"); } } - -async fn socket_activate(config: ServerConfig) -> anyhow::Result<()> { - let conn = get_socket_from_systemd().await?; - - let uid = match conn.peer_cred() { - Ok(cred) => cred.uid(), - Err(e) => { - log::error!("Failed to get peer credentials from socket: {}", e); - let mut message_stream = create_server_to_client_message_stream(conn); - message_stream - .send(Response::Error( - (concatdoc! { - "Server failed to get peer credentials from socket\n", - "Please check the server logs or contact the system administrators" - }) - .to_string(), - )) - .await - .ok(); - anyhow::bail!("Failed to get peer credentials from socket"); - } - }; - - log::debug!("Accepted connection from uid {}", uid); - - let unix_user = match UnixUser::from_uid(uid) { - Ok(user) => user, - Err(e) => { - log::error!("Failed to get username from uid: {}", e); - let mut message_stream = create_server_to_client_message_stream(conn); - message_stream - .send(Response::Error( - (concatdoc! { - "Server failed to get user data from the system\n", - "Please check the server logs or contact the system administrators" - }) - .to_string(), - )) - .await - .ok(); - anyhow::bail!("Failed to get username from uid"); - } - }; - - log::info!("Accepted connection from {}", unix_user.username); - - sd_notify::notify(false, &[sd_notify::NotifyState::Ready]).ok(); - - handle_requests_for_single_session(conn, &unix_user, &config).await?; - - Ok(()) -} - -async fn get_socket_from_systemd() -> anyhow::Result { - let fd = sd_notify::listen_fds() - .context("Failed to get file descriptors from systemd")? - .next() - .context("No file descriptors received from systemd")?; - - debug_assert!(fd == 3, "Unexpected file descriptor from systemd: {}", fd); - - log::debug!( - "Received file descriptor from systemd with id: '{}', assuming socket", - fd - ); - - let std_unix_stream = unsafe { StdUnixStream::from_raw_fd(fd) }; - let socket = TokioUnixStream::from_std(std_unix_stream)?; - Ok(socket) -} diff --git a/src/server/server_loop.rs b/src/server/server_loop.rs index 6917d0c..5966f30 100644 --- a/src/server/server_loop.rs +++ b/src/server/server_loop.rs @@ -1,8 +1,14 @@ -use std::{collections::BTreeSet, fs, path::PathBuf}; +use std::{ + collections::BTreeSet, + fs, + os::unix::{io::FromRawFd, net::UnixListener as StdUnixListener}, + path::PathBuf, +}; +use anyhow::Context; use futures_util::{SinkExt, StreamExt}; use indoc::concatdoc; -use tokio::net::{UnixListener, UnixStream}; +use tokio::net::{UnixListener as TokioUnixListener, UnixStream as TokioUnixStream}; use sqlx::MySqlConnection; use sqlx::prelude::*; @@ -34,10 +40,9 @@ use crate::{ // TODO: consider using a connection pool -pub async fn listen_for_incoming_connections( +pub async fn listen_for_incoming_connections_with_socket_path( socket_path: Option, config: ServerConfig, - // db_connection: &mut MySqlConnection, ) -> anyhow::Result<()> { let socket_path = socket_path.unwrap_or(PathBuf::from(DEFAULT_SOCKET_PATH)); @@ -55,8 +60,35 @@ pub async fn listen_for_incoming_connections( Err(e) => return Err(e.into()), } - let listener = UnixListener::bind(socket_path)?; + let listener = TokioUnixListener::bind(socket_path)?; + listen_for_incoming_connections_with_listener(listener, config).await +} + +pub async fn listen_for_incoming_connections_with_systemd_socket( + config: ServerConfig, +) -> anyhow::Result<()> { + let fd = sd_notify::listen_fds() + .context("Failed to get file descriptors from systemd")? + .next() + .context("No file descriptors received from systemd")?; + + debug_assert!(fd == 3, "Unexpected file descriptor from systemd: {}", fd); + + log::debug!( + "Received file descriptor from systemd with id: '{}', assuming socket", + fd + ); + + let std_unix_listener = unsafe { StdUnixListener::from_raw_fd(fd) }; + let listener = TokioUnixListener::from_std(std_unix_listener)?; + listen_for_incoming_connections_with_listener(listener, config).await +} + +pub async fn listen_for_incoming_connections_with_listener( + listener: TokioUnixListener, + config: ServerConfig, +) -> anyhow::Result<()> { sd_notify::notify(false, &[sd_notify::NotifyState::Ready]).ok(); while let Ok((conn, _addr)) = listener.accept().await { @@ -113,12 +145,23 @@ pub async fn listen_for_incoming_connections( Ok(()) } +async fn close_or_ignore_db_connection(db_connection: MySqlConnection) { + if let Err(e) = db_connection.close().await { + log::error!("Failed to close database connection: {}", e); + log::error!("{}", e); + log::error!("Ignoring..."); + } +} + pub async fn handle_requests_for_single_session( - socket: UnixStream, + socket: TokioUnixStream, unix_user: &UnixUser, config: &ServerConfig, ) -> anyhow::Result<()> { let mut message_stream = create_server_to_client_message_stream(socket); + + log::debug!("Opening connection to database"); + let mut db_connection = match create_mysql_connection_from_config(&config.mysql).await { Ok(connection) => connection, Err(err) => { @@ -136,6 +179,24 @@ pub async fn handle_requests_for_single_session( } }; + log::debug!("Verifying that database connection is valid"); + + if let Err(e) = db_connection.ping().await { + log::error!("Failed to ping database: {}", e); + message_stream + .send(Response::Error( + (concatdoc! { + "Server failed to connect to database\n", + "Please check the server logs or contact the system administrators" + }) + .to_string(), + )) + .await?; + message_stream.flush().await?; + close_or_ignore_db_connection(db_connection).await; + return Err(e.into()); + } + log::debug!("Successfully connected to database"); let result = handle_requests_for_single_session_with_db_connection( @@ -145,11 +206,7 @@ pub async fn handle_requests_for_single_session( ) .await; - if let Err(e) = db_connection.close().await { - log::error!("Failed to close database connection: {}", e); - log::error!("{}", e); - log::error!("Ignoring..."); - } + close_or_ignore_db_connection(db_connection).await; result } @@ -157,7 +214,7 @@ pub async fn handle_requests_for_single_session( // TODO: ensure proper db_connection hygiene for functions that invoke // this function -pub async fn handle_requests_for_single_session_with_db_connection( +async fn handle_requests_for_single_session_with_db_connection( mut stream: ServerToClientMessageStream, unix_user: &UnixUser, db_connection: &mut MySqlConnection,