From 48240489a7189adb06f930cc58542640bc8cc918 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 16:46:12 +0200 Subject: [PATCH 1/9] Have server notify the client about db connection errors --- src/core/protocol/request_response.rs | 3 +- src/main.rs | 19 ++++++- src/server/server_loop.rs | 82 +++++++++++++++++++-------- 3 files changed, 77 insertions(+), 27 deletions(-) diff --git a/src/core/protocol/request_response.rs b/src/core/protocol/request_response.rs index 13bc013..94725cb 100644 --- a/src/core/protocol/request_response.rs +++ b/src/core/protocol/request_response.rs @@ -73,7 +73,6 @@ pub enum Response { UnlockUsers(UnlockUsersOutput), // Generic responses - OperationAborted, + Ready, Error(String), - Exit, } diff --git a/src/main.rs b/src/main.rs index 165e46e..a539de3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,10 +9,12 @@ use std::path::PathBuf; use std::os::unix::net::UnixStream as StdUnixStream; use tokio::net::UnixStream as TokioUnixStream; +use futures::StreamExt; + use crate::{ core::{ bootstrap::{bootstrap_server_connection_and_drop_privileges, drop_privs}, - protocol::create_client_to_server_message_stream, + protocol::{create_client_to_server_message_stream, Response}, }, server::command::ServerArgs, }; @@ -205,7 +207,20 @@ fn tokio_run_command(command: Command, server_connection: StdUnixStream) -> anyh .unwrap() .block_on(async { let tokio_socket = TokioUnixStream::from_std(server_connection)?; - let message_stream = create_client_to_server_message_stream(tokio_socket); + let mut message_stream = create_client_to_server_message_stream(tokio_socket); + + while let Some(Ok(message)) = message_stream.next().await { + match message { + Response::Error(err) => { + anyhow::bail!("{}", err); + } + Response::Ready => break, + message => { + eprintln!("Unexpected message from server: {:?}", message); + } + } + } + match command { Command::User(user_args) => { cli::user_command::handle_command(user_args, message_stream).await diff --git a/src/server/server_loop.rs b/src/server/server_loop.rs index f768568..1fe6f30 100644 --- a/src/server/server_loop.rs +++ b/src/server/server_loop.rs @@ -1,7 +1,7 @@ use std::{collections::BTreeSet, fs, path::PathBuf}; use futures_util::{SinkExt, StreamExt}; -use tokio::io::AsyncWriteExt; +use indoc::concatdoc; use tokio::net::{UnixListener, UnixStream}; use sqlx::prelude::*; @@ -57,15 +57,43 @@ pub async fn listen_for_incoming_connections( sd_notify::notify(true, &[sd_notify::NotifyState::Ready]).ok(); - while let Ok((mut conn, _addr)) = listener.accept().await { - let uid = conn.peer_cred()?.uid(); + while let Ok((conn, _addr)) = listener.accept().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(); + continue; + } + }; + log::trace!("Accepted connection from uid {}", uid); let unix_user = match UnixUser::from_uid(uid) { Ok(user) => user, Err(e) => { - eprintln!("Failed to get UnixUser from uid: {}", e); - conn.shutdown().await?; + 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(); continue; } }; @@ -73,9 +101,9 @@ pub async fn listen_for_incoming_connections( log::info!("Accepted connection from {}", unix_user.username); match handle_requests_for_single_session(conn, &unix_user, &config).await { - Ok(_) => {} + Ok(()) => {} Err(e) => { - eprintln!("Failed to run server: {}", e); + log::error!("Failed to run server: {}", e); } } } @@ -88,8 +116,24 @@ pub async fn handle_requests_for_single_session( unix_user: &UnixUser, config: &ServerConfig, ) -> anyhow::Result<()> { - let message_stream = create_server_to_client_message_stream(socket); - let mut db_connection = create_mysql_connection_from_config(&config.mysql).await?; + let mut message_stream = create_server_to_client_message_stream(socket); + let mut db_connection = match create_mysql_connection_from_config(&config.mysql).await { + Ok(connection) => connection, + Err(err) => { + 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?; + return Err(err); + } + }; + log::debug!("Successfully connected to database"); let result = handle_requests_for_single_session_with_db_connection( @@ -100,9 +144,9 @@ pub async fn handle_requests_for_single_session( .await; if let Err(e) = db_connection.close().await { - eprintln!("Failed to close database connection: {}", e); - eprintln!("{}", e); - eprintln!("Ignoring..."); + log::error!("Failed to close database connection: {}", e); + log::error!("{}", e); + log::error!("Ignoring..."); } result @@ -116,6 +160,7 @@ pub async fn handle_requests_for_single_session_with_db_connection( unix_user: &UnixUser, db_connection: &mut MySqlConnection, ) -> anyhow::Result<()> { + stream.send(Response::Ready).await?; loop { // TODO: better error handling let request = match stream.next().await { @@ -133,17 +178,14 @@ pub async fn handle_requests_for_single_session_with_db_connection( Request::CreateDatabases(databases_names) => { let result = create_databases(databases_names, unix_user, db_connection).await; stream.send(Response::CreateDatabases(result)).await?; - stream.flush().await?; } Request::DropDatabases(databases_names) => { let result = drop_databases(databases_names, unix_user, db_connection).await; stream.send(Response::DropDatabases(result)).await?; - stream.flush().await?; } Request::ListDatabases => { let result = list_databases_for_user(unix_user, db_connection).await; stream.send(Response::ListAllDatabases(result)).await?; - stream.flush().await?; } Request::ListPrivileges(database_names) => { let response = match database_names { @@ -161,7 +203,6 @@ pub async fn handle_requests_for_single_session_with_db_connection( }; stream.send(response).await?; - stream.flush().await?; } Request::ModifyPrivileges(database_privilege_diffs) => { let result = apply_privilege_diffs( @@ -171,24 +212,20 @@ pub async fn handle_requests_for_single_session_with_db_connection( ) .await; stream.send(Response::ModifyPrivileges(result)).await?; - stream.flush().await?; } Request::CreateUsers(db_users) => { let result = create_database_users(db_users, unix_user, db_connection).await; stream.send(Response::CreateUsers(result)).await?; - stream.flush().await?; } Request::DropUsers(db_users) => { let result = drop_database_users(db_users, unix_user, db_connection).await; stream.send(Response::DropUsers(result)).await?; - stream.flush().await?; } Request::PasswdUser(db_user, password) => { let result = set_password_for_database_user(&db_user, &password, unix_user, db_connection) .await; stream.send(Response::PasswdUser(result)).await?; - stream.flush().await?; } Request::ListUsers(db_users) => { let response = match db_users { @@ -203,22 +240,21 @@ pub async fn handle_requests_for_single_session_with_db_connection( } }; stream.send(response).await?; - stream.flush().await?; } Request::LockUsers(db_users) => { let result = lock_database_users(db_users, unix_user, db_connection).await; stream.send(Response::LockUsers(result)).await?; - stream.flush().await?; } Request::UnlockUsers(db_users) => { let result = unlock_database_users(db_users, unix_user, db_connection).await; stream.send(Response::UnlockUsers(result)).await?; - stream.flush().await?; } Request::Exit => { break; } } + + stream.flush().await?; } Ok(()) -- 2.44.2 From ed12a3153b041f43fbd26ffaadead81ec06261b5 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 16:47:34 +0200 Subject: [PATCH 2/9] server/config: revamp - Adds options `socket_path` and `password_file` --- example-config.toml | 14 ++++++ src/core/bootstrap.rs | 4 +- src/server/config.rs | 105 ++++++++++++++++++++++++++++++------------ 3 files changed, 92 insertions(+), 31 deletions(-) diff --git a/example-config.toml b/example-config.toml index 2d3200e..c2f9e44 100644 --- a/example-config.toml +++ b/example-config.toml @@ -1,8 +1,22 @@ # This should go to `/etc/mysqladm/config.toml` +[server] +# Note that this gets ignored if you are using socket activation. +socket_path = "/var/run/mysqladm/mysqladm.sock" + [mysql] + +# if you use a socket, the host and port will be ignored +# socket_path = "/var/run/mysql/mysql.sock" + host = "localhost" port = 3306 + +# The username and password can be omitted if you are using +# socket based authentication. However, the vendored systemd +# service is running as DynamicUser, so by default you need +# to at least specify the username. username = "root" password = "secret" + timeout = 2 # seconds \ No newline at end of file diff --git a/src/core/bootstrap.rs b/src/core/bootstrap.rs index f64d89e..891b52a 100644 --- a/src/core/bootstrap.rs +++ b/src/core/bootstrap.rs @@ -7,7 +7,7 @@ use tokio::net::UnixStream as TokioUnixStream; use crate::{ core::common::{UnixUser, DEFAULT_CONFIG_PATH, DEFAULT_SOCKET_PATH}, - server::{config::read_config_form_path, server_loop::handle_requests_for_single_session}, + server::{config::read_config_from_path, server_loop::handle_requests_for_single_session}, }; // TODO: this function is security critical, it should be integration tested @@ -140,7 +140,7 @@ fn run_forked_server( server_socket: StdUnixStream, unix_user: UnixUser, ) -> anyhow::Result<()> { - let config = read_config_form_path(Some(config_path))?; + let config = read_config_from_path(Some(config_path))?; let result: anyhow::Result<()> = tokio::runtime::Builder::new_current_thread() .enable_all() diff --git a/src/server/config.rs b/src/server/config.rs index e3639ca..4989c07 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -21,21 +21,37 @@ pub struct ServerConfig { #[derive(Debug, Clone, Deserialize, Serialize)] #[serde(rename = "mysql")] pub struct MysqlConfig { - pub host: String, + pub socket_path: Option, + pub host: Option, pub port: Option, - pub username: String, - pub password: String, + pub username: Option, + pub password: Option, + pub password_file: Option, pub timeout: Option, } #[derive(Parser, Debug, Clone)] pub struct ServerConfigArgs { + /// Path to the socket of the MySQL server. + #[arg(long, value_name = "PATH", global = true)] + socket_path: Option, + /// Hostname of the MySQL server. - #[arg(long, value_name = "HOST", global = true)] + #[arg( + long, + value_name = "HOST", + global = true, + conflicts_with = "socket_path" + )] mysql_host: Option, /// Port of the MySQL server. - #[arg(long, value_name = "PORT", global = true)] + #[arg( + long, + value_name = "PORT", + global = true, + conflicts_with = "socket_path" + )] mysql_port: Option, /// Username to use for the MySQL connection. @@ -44,7 +60,7 @@ pub struct ServerConfigArgs { /// Path to a file containing the MySQL password. #[arg(long, value_name = "PATH", global = true)] - mysql_password_file: Option, + mysql_password_file: Option, /// Seconds to wait for the MySQL connection to be established. #[arg(long, value_name = "SECONDS", global = true)] @@ -57,30 +73,40 @@ pub fn read_config_from_path_with_arg_overrides( config_path: Option, args: ServerConfigArgs, ) -> anyhow::Result { - let config = read_config_form_path(config_path)?; + let config = read_config_from_path(config_path)?; - let mysql = &config.mysql; + let mysql = config.mysql; - let password = if let Some(path) = args.mysql_password_file { - fs::read_to_string(path) - .context("Failed to read MySQL password file") - .map(|s| s.trim().to_owned())? + let password = if let Some(path) = &args.mysql_password_file { + Some( + fs::read_to_string(path) + .context("Failed to read MySQL password file") + .map(|s| s.trim().to_owned())?, + ) + } else if let Some(path) = &mysql.password_file { + Some( + fs::read_to_string(path) + .context("Failed to read MySQL password file") + .map(|s| s.trim().to_owned())?, + ) } else { mysql.password.to_owned() }; Ok(ServerConfig { mysql: MysqlConfig { - host: args.mysql_host.unwrap_or(mysql.host.to_owned()), + socket_path: args.socket_path.or(mysql.socket_path), + host: args.mysql_host.or(mysql.host), port: args.mysql_port.or(mysql.port), - username: args.mysql_user.unwrap_or(mysql.username.to_owned()), + username: args.mysql_user.or(mysql.username.to_owned()), password, + password_file: args.mysql_password_file.or(mysql.password_file), timeout: args.mysql_connect_timeout.or(mysql.timeout), }, }) } -pub fn read_config_form_path(config_path: Option) -> anyhow::Result { +pub fn read_config_from_path(config_path: Option) -> anyhow::Result { let config_path = config_path.unwrap_or_else(|| PathBuf::from(DEFAULT_CONFIG_PATH)); log::debug!("Reading config from {:?}", &config_path); @@ -97,30 +123,51 @@ pub fn read_config_form_path(config_path: Option) -> anyhow::Result anyhow::Result { +fn log_config(config: &MysqlConfig) { let mut display_config = config.clone(); - "".clone_into(&mut display_config.password); + display_config.password = display_config + .password + .as_ref() + .map(|_| "".to_owned()); log::debug!( "Connecting to MySQL server with parameters: {:#?}", display_config ); +} + +/// Use the provided configuration to establish a connection to a MySQL server. +pub async fn create_mysql_connection_from_config( + config: &MysqlConfig, +) -> anyhow::Result { + log_config(config); + + let mut mysql_options = MySqlConnectOptions::new() + .database("mysql"); + + if let Some(username) = &config.username { + mysql_options = mysql_options.username(username); + } + + if let Some(password) = &config.password { + mysql_options = mysql_options.password(password); + } + + if let Some(socket_path) = &config.socket_path { + mysql_options = mysql_options.socket(socket_path); + } else if let Some(host) = &config.host { + mysql_options = mysql_options.host(host); + mysql_options = mysql_options.port(config.port.unwrap_or(DEFAULT_PORT)); + } else { + anyhow::bail!("No MySQL host or socket path provided"); + } match tokio::time::timeout( Duration::from_secs(config.timeout.unwrap_or(DEFAULT_TIMEOUT)), - MySqlConnectOptions::new() - .host(&config.host) - .username(&config.username) - .password(&config.password) - .port(config.port.unwrap_or(DEFAULT_PORT)) - .database("mysql") - .connect(), + mysql_options.connect(), ) .await { - Ok(connection) => connection.context("Failed to connect to MySQL"), - Err(_) => Err(anyhow!("Timed out after 2 seconds")).context("Failed to connect to MySQL"), + Ok(connection) => connection.context("Failed to connect to the database"), + Err(_) => Err(anyhow!("Timed out after 2 seconds")).context("Failed to connect to the database"), } } -- 2.44.2 From f43499fca0d4143bef5f385373a08a282a558474 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 17:11:19 +0200 Subject: [PATCH 3/9] "downgrade" nixpkgs to stable, to avoid rust 1.80 breakage grcov is breaking, see https://github.com/NixOS/nixpkgs/issues/332957 --- flake.lock | 14 +++++++------- flake.nix | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/flake.lock b/flake.lock index 3b08218..9b3e2aa 100644 --- a/flake.lock +++ b/flake.lock @@ -2,16 +2,16 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1723637854, - "narHash": "sha256-med8+5DSWa2UnOqtdICndjDAEjxr5D7zaIiK4pn0Q7c=", + "lastModified": 1723938990, + "narHash": "sha256-9tUadhnZQbWIiYVXH8ncfGXGvkNq3Hag4RCBEMUk7MI=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "c3aa7b8938b17aebd2deecf7be0636000d62a2b9", + "rev": "c42fcfbdfeae23e68fc520f9182dde9f38ad1890", "type": "github" }, "original": { "owner": "NixOS", - "ref": "nixos-unstable", + "ref": "nixos-24.05", "repo": "nixpkgs", "type": "github" } @@ -29,11 +29,11 @@ ] }, "locked": { - "lastModified": 1723947704, - "narHash": "sha256-TcVf66N2NgGhxORFytzgqWcg0XJ+kk8uNLNsTRI5sYM=", + "lastModified": 1724034091, + "narHash": "sha256-b1g7w0sw+MDAhUAeCoX1vlTghsqcDZkxr+k9OZmxPa8=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "456e78a55feade2c3bc6d7bc0bf5e710c9d86120", + "rev": "c7d36e0947826e0751a5214ffe82533fbc909bc0", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index c0728ed..85494f2 100644 --- a/flake.nix +++ b/flake.nix @@ -1,6 +1,6 @@ { inputs = { - nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; + nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.05"; rust-overlay.url = "github:oxalica/rust-overlay"; rust-overlay.inputs.nixpkgs.follows = "nixpkgs"; -- 2.44.2 From 20669569f34bc6d61675a8e1edd383b79709ecb4 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 17:44:21 +0200 Subject: [PATCH 4/9] Fix binary collation issues for privs as well Ref #66 --- src/core/database_privileges.rs | 8 ++++---- src/server/common.rs | 15 ++++++++++++++ .../sql/database_privilege_operations.rs | 20 ++++++++++--------- src/server/sql/user_operations.rs | 15 +------------- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/core/database_privileges.rs b/src/core/database_privileges.rs index 847108f..696bfd6 100644 --- a/src/core/database_privileges.rs +++ b/src/core/database_privileges.rs @@ -14,8 +14,8 @@ use crate::server::sql::database_privilege_operations::{ pub fn db_priv_field_human_readable_name(name: &str) -> String { match name { - "db" => "Database".to_owned(), - "user" => "User".to_owned(), + "Db" => "Database".to_owned(), + "User" => "User".to_owned(), "select_priv" => "Select".to_owned(), "insert_priv" => "Insert".to_owned(), "update_priv" => "Update".to_owned(), @@ -128,8 +128,8 @@ pub fn format_privileges_line_for_editor( DATABASE_PRIVILEGE_FIELDS .into_iter() .map(|field| match field { - "db" => format!("{:width$}", privs.db, width = database_name_len), - "user" => format!("{:width$}", privs.user, width = username_len), + "Db" => format!("{:width$}", privs.db, width = database_name_len), + "User" => format!("{:width$}", privs.user, width = username_len), privilege => format!( "{:width$}", yn(privs.get_privilege_by_name(privilege)), diff --git a/src/server/common.rs b/src/server/common.rs index db540ce..fb39122 100644 --- a/src/server/common.rs +++ b/src/server/common.rs @@ -1,4 +1,5 @@ use crate::core::common::UnixUser; +use sqlx::prelude::*; /// This function creates a regex that matches items (users, databases) /// that belong to the user or any of the user's groups. @@ -9,3 +10,17 @@ pub fn create_user_group_matching_regex(user: &UnixUser) -> String { format!("({}|{})(_.+)?", user.username, user.groups.join("|")) } } + +/// Some mysql versions with some collations mark some columns as binary fields, +/// which in the current version of sqlx is not parsable as string. +/// See: https://github.com/launchbadge/sqlx/issues/3387 +#[inline] +pub fn try_get_with_binary_fallback( + row: &sqlx::mysql::MySqlRow, + column: &str, +) -> Result { + row.try_get(column).or_else(|_| { + row.try_get::, _>(column) + .map(|v| String::from_utf8_lossy(&v).to_string()) + }) +} \ No newline at end of file diff --git a/src/server/sql/database_privilege_operations.rs b/src/server/sql/database_privilege_operations.rs index ec6b776..a9d1dc5 100644 --- a/src/server/sql/database_privilege_operations.rs +++ b/src/server/sql/database_privilege_operations.rs @@ -32,7 +32,7 @@ use crate::{ }, }, server::{ - common::create_user_group_matching_regex, + common::{create_user_group_matching_regex, try_get_with_binary_fallback}, input_sanitization::{quote_identifier, validate_name, validate_ownership_by_unix_user}, sql::database_operations::unsafe_database_exists, }, @@ -42,8 +42,8 @@ use crate::{ /// from the `db` table in the database. If you need to add or remove privilege /// fields, this is a good place to start. pub const DATABASE_PRIVILEGE_FIELDS: [&str; 13] = [ - "db", - "user", + "Db", + "User", "select_priv", "insert_priv", "update_priv", @@ -97,6 +97,8 @@ impl DatabasePrivilegeRow { } } +// TODO: get by name instead of row tuple position + #[inline] fn get_mysql_row_priv_field(row: &MySqlRow, position: usize) -> Result { let field = DATABASE_PRIVILEGE_FIELDS[position]; @@ -113,8 +115,8 @@ fn get_mysql_row_priv_field(row: &MySqlRow, position: usize) -> Result for DatabasePrivilegeRow { fn from_row(row: &MySqlRow) -> Result { Ok(Self { - db: row.try_get("db")?, - user: row.try_get("user")?, + db: try_get_with_binary_fallback(row, "Db")?, + user: try_get_with_binary_fallback(row, "User")?, select_priv: get_mysql_row_priv_field(row, 2)?, insert_priv: get_mysql_row_priv_field(row, 3)?, update_priv: get_mysql_row_priv_field(row, 4)?, @@ -137,7 +139,7 @@ async fn unsafe_get_database_privileges( connection: &mut MySqlConnection, ) -> Result, sqlx::Error> { let result = sqlx::query_as::<_, DatabasePrivilegeRow>(&format!( - "SELECT {} FROM `db` WHERE `db` = ?", + "SELECT {} FROM `db` WHERE `Db` = ?", DATABASE_PRIVILEGE_FIELDS .iter() .map(|field| quote_identifier(field)) @@ -166,7 +168,7 @@ pub async fn unsafe_get_database_privileges_for_db_user_pair( connection: &mut MySqlConnection, ) -> Result, sqlx::Error> { let result = sqlx::query_as::<_, DatabasePrivilegeRow>(&format!( - "SELECT {} FROM `db` WHERE `db` = ? AND `user` = ?", + "SELECT {} FROM `db` WHERE `Db` = ? AND `User` = ?", DATABASE_PRIVILEGE_FIELDS .iter() .map(|field| quote_identifier(field)) @@ -316,7 +318,7 @@ async fn unsafe_apply_privilege_diff( .join(","); sqlx::query( - format!("UPDATE `db` SET {} WHERE `db` = ? AND `user` = ?", changes).as_str(), + format!("UPDATE `db` SET {} WHERE `Db` = ? AND `User` = ?", changes).as_str(), ) .bind(p.db.to_string()) .bind(p.user.to_string()) @@ -325,7 +327,7 @@ async fn unsafe_apply_privilege_diff( .map(|_| ()) } DatabasePrivilegesDiff::Deleted(p) => { - sqlx::query("DELETE FROM `db` WHERE `db` = ? AND `user` = ?") + sqlx::query("DELETE FROM `db` WHERE `Db` = ? AND `User` = ?") .bind(p.db.to_string()) .bind(p.user.to_string()) .execute(connection) diff --git a/src/server/sql/user_operations.rs b/src/server/sql/user_operations.rs index 06da219..120903b 100644 --- a/src/server/sql/user_operations.rs +++ b/src/server/sql/user_operations.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use sqlx::prelude::*; use sqlx::MySqlConnection; +use crate::server::common::try_get_with_binary_fallback; use crate::{ core::{ common::UnixUser, @@ -350,20 +351,6 @@ pub struct DatabaseUser { pub databases: Vec, } -/// Some mysql versions with some collations mark some columns as binary fields, -/// which in the current version of sqlx is not parsable as string. -/// See: https://github.com/launchbadge/sqlx/issues/3387 -#[inline] -fn try_get_with_binary_fallback( - row: &sqlx::mysql::MySqlRow, - column: &str, -) -> Result { - row.try_get(column).or_else(|_| { - row.try_get::, _>(column) - .map(|v| String::from_utf8_lossy(&v).to_string()) - }) -} - impl FromRow<'_, sqlx::mysql::MySqlRow> for DatabaseUser { fn from_row(row: &sqlx::mysql::MySqlRow) -> Result { Ok(Self { -- 2.44.2 From 3556eb37ea9b3a0524893dfb51d8156021925b63 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 17:46:08 +0200 Subject: [PATCH 5/9] Dont drop privs as server --- src/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index a539de3..d0ac70b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,7 +13,7 @@ use futures::StreamExt; use crate::{ core::{ - bootstrap::{bootstrap_server_connection_and_drop_privileges, drop_privs}, + bootstrap::bootstrap_server_connection_and_drop_privileges, protocol::{create_client_to_server_message_stream, Response}, }, server::command::ServerArgs, @@ -148,7 +148,6 @@ fn handle_mysql_admutils_command() -> anyhow::Result> { fn handle_server_command(args: &Args) -> anyhow::Result> { match args.command { Command::Server(ref command) => { - drop_privs()?; tokio_start_server( args.server_socket_path.clone(), args.config.clone(), -- 2.44.2 From d1d06514a9b73015029315c01c43a6c34f43079a Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 17:52:16 +0200 Subject: [PATCH 6/9] cargo fmt + clippy --- src/cli/mysql_admutils_compatibility/mysql_dbadm.rs | 10 +++++----- src/cli/mysql_admutils_compatibility/mysql_useradm.rs | 10 +++++----- src/core/bootstrap.rs | 3 +++ src/main.rs | 10 +++++----- src/server/common.rs | 2 +- src/server/config.rs | 7 ++++--- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs b/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs index 0b4518e..0e881c1 100644 --- a/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs +++ b/src/cli/mysql_admutils_compatibility/mysql_dbadm.rs @@ -56,11 +56,11 @@ The Y/N-values corresponds to the following mysql privileges: /// Please consider using the newer mysqladm command instead. #[derive(Parser)] #[command( - bin_name = "mysql-dbadm", - version, - about, - disable_help_subcommand = true, - verbatim_doc_comment, + bin_name = "mysql-dbadm", + version, + about, + disable_help_subcommand = true, + verbatim_doc_comment )] pub struct Args { #[command(subcommand)] diff --git a/src/cli/mysql_admutils_compatibility/mysql_useradm.rs b/src/cli/mysql_admutils_compatibility/mysql_useradm.rs index 5de17f0..cdc7254 100644 --- a/src/cli/mysql_admutils_compatibility/mysql_useradm.rs +++ b/src/cli/mysql_admutils_compatibility/mysql_useradm.rs @@ -32,11 +32,11 @@ use crate::{ /// Please consider using the newer mysqladm command instead. #[derive(Parser)] #[command( - bin_name = "mysql-useradm", - version, - about, - disable_help_subcommand = true, - verbatim_doc_comment, + bin_name = "mysql-useradm", + version, + about, + disable_help_subcommand = true, + verbatim_doc_comment )] pub struct Args { #[command(subcommand)] diff --git a/src/core/bootstrap.rs b/src/core/bootstrap.rs index 891b52a..1d3c06f 100644 --- a/src/core/bootstrap.rs +++ b/src/core/bootstrap.rs @@ -32,15 +32,18 @@ pub fn drop_privs() -> anyhow::Result<()> { /// This function is used to bootstrap the connection to the server. /// This can happen in two ways: +/// /// 1. If a socket path is provided, or exists in the default location, /// the function will connect to the socket and authenticate with the /// server to ensure that the server knows the uid of the client. +/// /// 2. If a config path is provided, or exists in the default location, /// and the config is readable, the function will assume it is either /// setuid or setgid, and will fork a child process to run the server /// with the provided config. The server will exit silently by itself /// when it is done, and this function will only return for the client /// with the socket for the server. +/// /// If neither of these options are available, the function will fail. pub fn bootstrap_server_connection_and_drop_privileges( server_socket_path: Option, diff --git a/src/main.rs b/src/main.rs index d0ac70b..549072f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -109,17 +109,17 @@ fn main() -> anyhow::Result<()> { env_logger::init(); #[cfg(feature = "mysql-admutils-compatibility")] - if let Some(_) = handle_mysql_admutils_command()? { + if handle_mysql_admutils_command()?.is_some() { return Ok(()); } let args: Args = Args::parse(); - if let Some(_) = handle_server_command(&args)? { + if handle_server_command(&args)?.is_some() { return Ok(()); } - if let Some(_) = handle_generate_completions_command(&args)? { + if handle_generate_completions_command(&args)?.is_some() { return Ok(()); } @@ -139,8 +139,8 @@ fn handle_mysql_admutils_command() -> anyhow::Result> { }); match argv0.as_deref() { - Some("mysql-dbadm") => mysql_dbadm::main().map(|result| Some(result)), - Some("mysql-useradm") => mysql_useradm::main().map(|result| Some(result)), + Some("mysql-dbadm") => mysql_dbadm::main().map(Some), + Some("mysql-useradm") => mysql_useradm::main().map(Some), _ => Ok(None), } } diff --git a/src/server/common.rs b/src/server/common.rs index fb39122..c42187a 100644 --- a/src/server/common.rs +++ b/src/server/common.rs @@ -23,4 +23,4 @@ pub fn try_get_with_binary_fallback( row.try_get::, _>(column) .map(|v| String::from_utf8_lossy(&v).to_string()) }) -} \ No newline at end of file +} diff --git a/src/server/config.rs b/src/server/config.rs index 4989c07..3010e43 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -141,8 +141,7 @@ pub async fn create_mysql_connection_from_config( ) -> anyhow::Result { log_config(config); - let mut mysql_options = MySqlConnectOptions::new() - .database("mysql"); + let mut mysql_options = MySqlConnectOptions::new().database("mysql"); if let Some(username) = &config.username { mysql_options = mysql_options.username(username); @@ -168,6 +167,8 @@ pub async fn create_mysql_connection_from_config( .await { Ok(connection) => connection.context("Failed to connect to the database"), - Err(_) => Err(anyhow!("Timed out after 2 seconds")).context("Failed to connect to the database"), + Err(_) => { + Err(anyhow!("Timed out after 2 seconds")).context("Failed to connect to the database") + } } } -- 2.44.2 From b21aa0eece37879056a99f31c24c854bdd5d25ba Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 17:57:35 +0200 Subject: [PATCH 7/9] Fix sql regex to work like ownership validation --- Cargo.lock | 1 + Cargo.toml | 3 +++ src/server/common.rs | 29 +++++++++++++++++++++++++++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e921e0..d5e3f10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1077,6 +1077,7 @@ dependencies = [ "prettytable", "rand", "ratatui", + "regex", "sd-notify", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 4517c66..faa78e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,3 +49,6 @@ codegen-units = 1 [build-dependencies] anyhow = "1.0.82" + +[dev-dependencies] +regex = "1.10.6" diff --git a/src/server/common.rs b/src/server/common.rs index c42187a..454dcbe 100644 --- a/src/server/common.rs +++ b/src/server/common.rs @@ -5,9 +5,9 @@ use sqlx::prelude::*; /// that belong to the user or any of the user's groups. pub fn create_user_group_matching_regex(user: &UnixUser) -> String { if user.groups.is_empty() { - format!("{}(_.+)?", user.username) + format!("{}_.+", user.username) } else { - format!("({}|{})(_.+)?", user.username, user.groups.join("|")) + format!("({}|{})_.+", user.username, user.groups.join("|")) } } @@ -24,3 +24,28 @@ pub fn try_get_with_binary_fallback( .map(|v| String::from_utf8_lossy(&v).to_string()) }) } + +#[cfg(test)] +mod tests { + use super::*; + use regex::Regex; + + #[test] + fn test_create_user_group_matching_regex() { + let user = UnixUser { + username: "user".to_owned(), + groups: vec!["group1".to_owned(), "group2".to_owned()], + }; + + let regex = create_user_group_matching_regex(&user); + let re = Regex::new(®ex).unwrap(); + + assert!(re.is_match("user_something")); + assert!(re.is_match("group1_something")); + assert!(re.is_match("group2_something")); + + assert!(!re.is_match("other_something")); + assert!(!re.is_match("user")); + assert!(!re.is_match("usersomething")); + } +} \ No newline at end of file -- 2.44.2 From b9a1d91630524e23ef0a8e0b28afa40a49729c52 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 18:00:57 +0200 Subject: [PATCH 8/9] Add nixos module --- flake.nix | 7 ++- nix/module.nix | 141 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 nix/module.nix diff --git a/flake.nix b/flake.nix index 85494f2..99f873b 100644 --- a/flake.nix +++ b/flake.nix @@ -52,11 +52,16 @@ overlays = { default = self.overlays.mysqladm-rs; - greg-ng = final: prev: { + mysqladm-rs = final: prev: { inherit (self.packages.${prev.system}) mysqladm-rs; }; }; + nixosModules = { + default = self.nixosModules.mysqladm-rs; + mysqladm-rs = import ./nix/module.nix; + }; + packages = let cargoToml = builtins.fromTOML (builtins.readFile ./Cargo.toml); cargoLock = ./Cargo.lock; diff --git a/nix/module.nix b/nix/module.nix new file mode 100644 index 0000000..dd39f4f --- /dev/null +++ b/nix/module.nix @@ -0,0 +1,141 @@ +{ config, pkgs, lib, ... }: +let + cfg = config.services.mysqladm-rs; + format = pkgs.formats.toml { }; +in +{ + options.services.mysqladm-rs = { + enable = lib.mkEnableOption "Enable mysqladm-rs"; + + package = lib.mkPackageOption pkgs "mysqladm-rs" { }; + + createLocalDatabaseUser = lib.mkOption { + type = lib.types.bool; + default = false; + description = "Create a local database user for mysqladm-rs"; + }; + + settings = lib.mkOption { + default = { }; + type = lib.types.submodule { + freeformType = format.type; + options = { + server = { + socket_path = lib.mkOption { + type = lib.types.path; + default = "/var/run/mysqladm/mysqladm.sock"; + description = "Path to the MySQL socket"; + }; + }; + + mysql = { + socket_path = lib.mkOption { + type = with lib.types; nullOr path; + default = "/var/run/mysqld/mysqld.sock"; + description = "Path to the MySQL socket"; + }; + host = lib.mkOption { + type = with lib.types; nullOr str; + default = null; + description = "MySQL host"; + }; + port = lib.mkOption { + type = with lib.types; nullOr port; + default = 3306; + description = "MySQL port"; + }; + username = lib.mkOption { + type = lib.types.str; + default = "mysqladm"; + description = "MySQL username"; + }; + passwordFile = lib.mkOption { + type = with lib.types; nullOr path; + default = null; + description = "Path to a file containing the MySQL password"; + }; + timeout = lib.mkOption { + type = lib.types.ints.positive; + default = 2; + description = "Number of seconds to wait for a response from the MySQL server"; + }; + }; + }; + }; + }; + }; + + config = let + nullStrippedConfig = lib.filterAttrsRecursive (_: v: v != null) cfg.settings; + configFile = format.generate "mysqladm-rs.conf" nullStrippedConfig; + in lib.mkIf config.services.mysqladm-rs.enable { + environment.systemPackages = [ cfg.package ]; + + services.mysql.ensureUsers = lib.mkIf cfg.createLocalDatabaseUser [ + { + name = cfg.settings.mysql.username; + ensurePermissions = { + "mysql.*" = "SELECT, INSERT, UPDATE, DELETE"; + "*.*" = "CREATE USER, GRANT OPTION"; + }; + } + ]; + + systemd.services."mysqladm@" = { + description = "MySQL administration tool for non-admin users"; + environment.RUST_LOG = "debug"; + serviceConfig = { + Type = "notify"; + ExecStart = "${lib.getExe cfg.package} server socket-activate --config ${configFile}"; + + User = "mysqladm"; + Group = "mysqladm"; + DynamicUser = true; + + # This is required to read unix user/group details. + PrivateUsers = false; + + CapabilityBoundingSet = ""; + LockPersonality = true; + MemoryDenyWriteExecute = true; + NoNewPrivileges = true; + PrivateDevices = true; + PrivateMounts = true; + PrivateTmp = "yes"; + ProcSubset = "pid"; + ProtectClock = true; + ProtectControlGroups = true; + ProtectHome = true; + ProtectHostname = true; + ProtectKernelLogs = true; + ProtectKernelModules = true; + ProtectKernelTunables = true; + ProtectProc = "invisible"; + ProtectSystem = "strict"; + RemoveIPC = true; + UMask = "0000"; + RestrictAddressFamilies = [ "AF_UNIX" "AF_INET" "AF_INET6" ]; + RestrictNamespaces = true; + RestrictRealtime = true; + RestrictSUIDSGID = true; + SystemCallArchitectures = "native"; + SystemCallFilter = [ + "@system-service" + "~@privileged" + "~@resources" + ]; + }; + }; + + systemd.sockets."mysqladm" = { + description = "MySQL administration tool for non-admin users"; + wantedBy = [ "sockets.target" ]; + restartTriggers = [ configFile ]; + socketConfig = { + ListenStream = cfg.settings.server.socket_path; + Accept = "yes"; + PassCredentials = true; + }; + }; + }; +} \ No newline at end of file -- 2.44.2 From 51302d75f07409b898b18f8f58bad701aa95369c Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 19 Aug 2024 18:06:47 +0200 Subject: [PATCH 9/9] `create-users`: default to setting no password in prompt --- src/cli/user_command.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/user_command.rs b/src/cli/user_command.rs index 8e51095..c49bff1 100644 --- a/src/cli/user_command.rs +++ b/src/cli/user_command.rs @@ -140,6 +140,7 @@ async fn create_users( "Do you want to set a password for user '{}'?", username )) + .default(false) .interact()? { let password = read_password_from_stdin_with_double_check(username)?; -- 2.44.2