From 39a2e39ba726b95f501da5b90cd8de8d956e8869 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Mon, 15 Dec 2025 15:17:37 +0900 Subject: [PATCH] WIP: implement denylists --- Cargo.toml | 5 ++ assets/debian/config.toml | 3 + assets/debian/group_denylist.txt | 58 +++++++++++++ nix/module.nix | 18 ++++ src/core/bootstrap.rs | 14 +++- src/core/protocol/request_validation.rs | 59 +++++++++++++ src/server.rs | 2 +- src/server/authorization.rs | 106 +++++++++++++++++++++++- src/server/config.rs | 6 ++ src/server/session_handler.rs | 18 +++- src/server/supervisor.rs | 59 ++++++++++++- 11 files changed, 335 insertions(+), 13 deletions(-) create mode 100644 assets/debian/group_denylist.txt diff --git a/Cargo.toml b/Cargo.toml index 0e195e6..5315315 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -132,6 +132,11 @@ assets = [ "etc/muscl/config.toml", "644", ], + [ + "assets/debian/group_denylist.txt", + "etc/muscl/group_denylist.txt", + "644", + ], [ "assets/completions/_*", "usr/share/zsh/site-functions/completions/", diff --git a/assets/debian/config.toml b/assets/debian/config.toml index 38a54f3..47f2b22 100644 --- a/assets/debian/config.toml +++ b/assets/debian/config.toml @@ -21,3 +21,6 @@ password_file = "/run/credentials/muscl.service/muscl_mysql_password" # Database connection timeout in seconds timeout = 2 + +[authorization] +group_denylist_file = "/etc/muscl/group_denylist.txt" diff --git a/assets/debian/group_denylist.txt b/assets/debian/group_denylist.txt new file mode 100644 index 0000000..9d37e4b --- /dev/null +++ b/assets/debian/group_denylist.txt @@ -0,0 +1,58 @@ +# These are the default system groups on debian. +# You can alos add groups by gid by prefixing the line with 'gid:'. + +group:adm +group:audio +group:avahi +group:backup +group:bin +group:cdrom +group:crontab +group:daemon +group:dialout +group:dip +group:disk +group:fax +group:floppy +group:games +group:gnats +group:input +group:irc +group:kmem +group:kvm +group:list +group:lp +group:mail +group:man +group:mlocate +group:netdev +group:news +group:nogroup +group:openldap +group:operator +group:plocate +group:plugdev +group:polkitd +group:postgres +group:proxy +group:render +group:root +group:sasl +group:shadow +group:src +group:staff +group:sudo +group:sync +group:sys +group:systemd-journal +group:systemd-network +group:systemd-resolve +group:systemd-timesync +group:tape +group:tty +group:users +group:utmp +group:uucp +group:video +group:voice +group:www-data diff --git a/nix/module.nix b/nix/module.nix index 9b13c6c..723e978 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -40,6 +40,14 @@ in }; }; + authorization = { + group_denylist = lib.mkOption { + type = with lib.types; nullOr (listOf str); + default = [ "wheel" ]; + description = "List of groups that are denied access"; + }; + }; + mysql = { socket_path = lib.mkOption { type = with lib.types; nullOr path; @@ -81,6 +89,12 @@ in environment.systemPackages = [ cfg.package ]; environment.etc."muscl/config.toml".source = lib.pipe cfg.settings [ + # Handle group_denylist_file + (conf: lib.recursiveUpdate conf { + authorization.group_denylist_file = if (conf.authorization.group_denylist != [ ]) then "/etc/muscl/group-denylist" else null; + authorization.group_denylist = null; + }) + # Remove nulls (lib.filterAttrsRecursive (_: v: v != null)) @@ -95,6 +109,10 @@ in (format.generate "muscl.conf") ]; + environment.etc."muscl/group-denylist" = lib.mkIf (cfg.settings.authorization.group_denylist != [ ]) { + text = lib.concatMapStringsSep "\n" (group: "group:${group}") cfg.settings.authorization.group_denylist; + }; + services.mysql.ensureUsers = lib.mkIf cfg.createLocalDatabaseUser [ { name = cfg.settings.mysql.username; diff --git a/src/core/bootstrap.rs b/src/core/bootstrap.rs index 9b9fee4..9189da8 100644 --- a/src/core/bootstrap.rs +++ b/src/core/bootstrap.rs @@ -9,10 +9,12 @@ use tokio::{net::UnixStream as TokioUnixStream, sync::RwLock}; use tracing_subscriber::prelude::*; use crate::{ - core::common::{ - DEFAULT_CONFIG_PATH, DEFAULT_SOCKET_PATH, UnixUser, executing_in_suid_sgid_mode, + core::{ + common::{DEFAULT_CONFIG_PATH, DEFAULT_SOCKET_PATH, UnixUser, executing_in_suid_sgid_mode}, + protocol::request_validation::GroupDenylist, }, server::{ + authorization::read_and_parse_group_denylist, config::{MysqlConfig, ServerConfig}, landlock::landlock_restrict_server, session_handler, @@ -270,6 +272,13 @@ fn run_forked_server( let config = ServerConfig::read_config_from_path(&config_path) .context("Failed to read server config in forked process")?; + let group_denylist = if let Some(denylist_path) = &config.authorization.group_denylist_file { + read_and_parse_group_denylist(denylist_path) + .context("Failed to read and parse group denylist")? + } else { + GroupDenylist::new() + }; + let result: anyhow::Result<()> = tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -292,6 +301,7 @@ fn run_forked_server( &unix_user, db_pool, db_is_mariadb, + &group_denylist, ) .await?; Ok(()) diff --git a/src/core/protocol/request_validation.rs b/src/core/protocol/request_validation.rs index fae3dd7..0579ce9 100644 --- a/src/core/protocol/request_validation.rs +++ b/src/core/protocol/request_validation.rs @@ -1,5 +1,8 @@ +use std::collections::HashSet; + use indoc::indoc; use itertools::Itertools; +use nix::{libc::gid_t, unistd::Group}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -60,6 +63,9 @@ pub enum AuthorizationError { // TODO: I don't think this should ever happen? #[error("Name cannot be empty")] StringEmpty, + + #[error("Group was found in denylist")] + DenylistError, } impl AuthorizationError { @@ -105,6 +111,9 @@ impl AuthorizationError { db_or_user.lowercased_noun() ) .to_string(), + AuthorizationError::DenylistError => { + format!("'{}' is denied by the group denylist", db_or_user.name()) + } } } @@ -112,6 +121,7 @@ impl AuthorizationError { match self { AuthorizationError::NoMatch => "no-match", AuthorizationError::StringEmpty => "string-empty", + AuthorizationError::DenylistError => "denylist-error", } } } @@ -155,8 +165,25 @@ impl ValidationError { } } +pub type GroupDenylist = HashSet; + const MAX_NAME_LENGTH: usize = 64; +// TODO: use this to render the help message for authorization-error/no-match +fn get_user_groups(user: &UnixUser, group_denylist: &GroupDenylist) -> Vec { + use nix::unistd::Group; + let mut groups = Vec::new(); + + for group in user.groups.iter() { + if let Some(g) = Group::from_name(group).ok().flatten() + && !group_denylist.contains(&g.gid.as_raw()) { + groups.push(group.clone()); + } + } + + groups +} + pub fn validate_name(name: &str) -> Result<(), NameValidationError> { if name.is_empty() { Err(NameValidationError::EmptyString) @@ -207,6 +234,30 @@ pub fn validate_authorization_by_prefixes( Ok(()) } +pub fn validate_authorization_by_group_denylist( + name: &str, + user: &UnixUser, + group_denylist: &GroupDenylist, +) -> Result<(), AuthorizationError> { + // NOTE: if the username matches, we allow it regardless of denylist + if user.username == name { + return Ok(()); + } + + let user_group = Group::from_name(name) + .ok() + .flatten() + .map(|g| g.gid.as_raw()); + + if let Some(gid) = user_group + && group_denylist.contains(&gid) + { + Err(AuthorizationError::DenylistError) + } else { + Ok(()) + } +} + pub fn validate_db_or_user_request( db_or_user: &DbOrUser, unix_user: &UnixUser, @@ -216,6 +267,14 @@ pub fn validate_db_or_user_request( validate_authorization_by_unix_user(db_or_user.name(), unix_user) .map_err(ValidationError::AuthorizationError)?; + validate_authorization_by_group_denylist( + db_or_user.name(), + unix_user, + // TODO: pass actual denylist + &GroupDenylist::new(), + ) + .map_err(ValidationError::AuthorizationError)?; + Ok(()) } diff --git a/src/server.rs b/src/server.rs index 248c22c..aa7d712 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,4 +1,4 @@ -mod authorization; +pub mod authorization; pub mod command; mod common; pub mod config; diff --git a/src/server/authorization.rs b/src/server/authorization.rs index 58864aa..383f6ff 100644 --- a/src/server/authorization.rs +++ b/src/server/authorization.rs @@ -1,12 +1,21 @@ +use std::{collections::HashSet, path::Path}; + +use anyhow::Context; +use nix::unistd::Group; + use crate::core::{ common::UnixUser, - protocol::{CheckAuthorizationError, request_validation::validate_db_or_user_request}, + protocol::{ + CheckAuthorizationError, + request_validation::{GroupDenylist, validate_db_or_user_request}, + }, types::DbOrUser, }; pub async fn check_authorization( dbs_or_users: Vec, unix_user: &UnixUser, + _group_denylist: &GroupDenylist, ) -> std::collections::BTreeMap> { let mut results = std::collections::BTreeMap::new(); @@ -17,9 +26,102 @@ pub async fn check_authorization( results.insert(db_or_user.clone(), Err(err)); continue; } - results.insert(db_or_user.clone(), Ok(())); } results } + +/// Reads and parses a group denylist file, returning a set of GUIDs +/// +/// The format of the denylist file is expected to be one group name or GID per line. +/// Lines starting with '#' are treated as comments and ignored. +/// Empty lines are also ignored. +/// +/// Each line looks like one of the following: +/// - `gid:1001` +/// - `group:admins` +pub fn read_and_parse_group_denylist(denylist_path: &Path) -> anyhow::Result { + let content = std::fs::read_to_string(denylist_path).context(format!( + "Failed to read denylist file at {:?}", + denylist_path + ))?; + + let mut groups = HashSet::with_capacity(content.lines().count()); + + for (line_number, line) in content.lines().enumerate() { + let trimmed_line = line.trim(); + + if trimmed_line.is_empty() || trimmed_line.starts_with('#') { + continue; + } + + let parts: Vec<&str> = trimmed_line.splitn(2, ':').collect(); + if parts.len() != 2 { + anyhow::bail!( + "Invalid format in denylist file at {:?} on line {}: {}", + denylist_path, + line_number + 1, + line + ); + } + + match parts[0] { + "gid" => { + let gid: u32 = parts[1].parse().with_context(|| { + format!( + "Invalid GID in denylist file at {:?} on line {}: {}", + denylist_path, + line_number + 1, + parts[1] + ) + })?; + let group = Group::from_gid(nix::unistd::Gid::from_raw(gid)) + .context(format!( + "Failed to get group for GID {} in denylist file at {:?} on line {}", + gid, + denylist_path, + line_number + 1 + ))? + .ok_or_else(|| { + anyhow::anyhow!( + "No group found for GID {} in denylist file at {:?} on line {}", + gid, + denylist_path, + line_number + 1 + ) + })?; + groups.insert(group.gid.as_raw()); + } + "group" => { + let group = Group::from_name(parts[1]) + .context(format!( + "Failed to get group for name '{}' in denylist file at {:?} on line {}", + parts[1], + denylist_path, + line_number + 1 + ))? + .ok_or_else(|| { + anyhow::anyhow!( + "No group found for name '{}' in denylist file at {:?} on line {}", + parts[1], + denylist_path, + line_number + 1 + ) + })?; + groups.insert(group.gid.as_raw()); + } + _ => { + anyhow::bail!( + "Invalid prefix '{}' in denylist file at {:?} on line {}: {}", + parts[0], + denylist_path, + line_number + 1, + line + ); + } + } + } + + Ok(groups) +} diff --git a/src/server/config.rs b/src/server/config.rs index 6ed0f20..22cc351 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -78,9 +78,15 @@ impl MysqlConfig { } } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct AuthorizationConfig { + pub group_denylist_file: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub struct ServerConfig { pub socket_path: Option, + pub authorization: AuthorizationConfig, pub mysql: MysqlConfig, } diff --git a/src/server/session_handler.rs b/src/server/session_handler.rs index 55a061b..02f8f6c 100644 --- a/src/server/session_handler.rs +++ b/src/server/session_handler.rs @@ -11,7 +11,7 @@ use crate::{ common::UnixUser, protocol::{ Request, Response, ServerToClientMessageStream, SetPasswordError, - create_server_to_client_message_stream, + create_server_to_client_message_stream, request_validation::GroupDenylist, }, }, server::{ @@ -39,6 +39,7 @@ pub async fn session_handler( socket: UnixStream, db_pool: Arc>, db_is_mariadb: bool, + group_denylist: &GroupDenylist, ) -> anyhow::Result<()> { let uid = match socket.peer_cred() { Ok(cred) => cred.uid(), @@ -85,8 +86,14 @@ pub async fn session_handler( (async move { tracing::info!("Accepted connection from user: {}", unix_user); - let result = - session_handler_with_unix_user(socket, &unix_user, db_pool, db_is_mariadb).await; + let result = session_handler_with_unix_user( + socket, + &unix_user, + db_pool, + db_is_mariadb, + group_denylist, + ) + .await; tracing::info!( "Finished handling requests for connection from user: {}", @@ -104,6 +111,7 @@ pub async fn session_handler_with_unix_user( unix_user: &UnixUser, db_pool: Arc>, db_is_mariadb: bool, + group_denylist: &GroupDenylist, ) -> anyhow::Result<()> { let mut message_stream = create_server_to_client_message_stream(socket); @@ -131,6 +139,7 @@ pub async fn session_handler_with_unix_user( unix_user, &mut db_connection, db_is_mariadb, + group_denylist, ) .await; @@ -147,6 +156,7 @@ async fn session_handler_with_db_connection( unix_user: &UnixUser, db_connection: &mut MySqlConnection, db_is_mariadb: bool, + group_denylist: &GroupDenylist, ) -> anyhow::Result<()> { stream.send(Response::Ready).await?; loop { @@ -178,7 +188,7 @@ async fn session_handler_with_db_connection( let response = match request { Request::CheckAuthorization(dbs_or_users) => { - let result = check_authorization(dbs_or_users, unix_user).await; + let result = check_authorization(dbs_or_users, unix_user, group_denylist).await; Response::CheckAuthorization(result) } Request::CompleteDatabaseName(partial_database_name) => { diff --git a/src/server/supervisor.rs b/src/server/supervisor.rs index 7aac28b..73ece09 100644 --- a/src/server/supervisor.rs +++ b/src/server/supervisor.rs @@ -17,9 +17,13 @@ use tokio::{ }; use tokio_util::{sync::CancellationToken, task::TaskTracker}; -use crate::server::{ - config::{MysqlConfig, ServerConfig}, - session_handler::session_handler, +use crate::{ + core::protocol::request_validation::GroupDenylist, + server::{ + authorization::read_and_parse_group_denylist, + config::{MysqlConfig, ServerConfig}, + session_handler::session_handler, + }, }; #[derive(Clone, Debug)] @@ -36,6 +40,7 @@ pub struct ReloadEvent; pub struct Supervisor { config_path: PathBuf, config: Arc>, + group_deny_list: Arc>, systemd_mode: bool, shutdown_cancel_token: CancellationToken, @@ -66,6 +71,23 @@ impl Supervisor { let config = ServerConfig::read_config_from_path(&config_path) .context("Failed to read server configuration")?; + let group_deny_list = match &config.authorization.group_denylist_file { + Some(denylist_path) => { + let denylist = read_and_parse_group_denylist(denylist_path) + .context("Failed to read group denylist file")?; + tracing::debug!( + "Loaded group denylist with {} entries from {:?}", + denylist.len(), + denylist_path + ); + Arc::new(RwLock::new(denylist)) + } + None => { + tracing::debug!("No group denylist file specified, proceeding without a denylist"); + Arc::new(RwLock::new(GroupDenylist::new())) + } + }; + let mut watchdog_duration = None; let mut watchdog_micro_seconds = 0; let watchdog_task = @@ -130,12 +152,14 @@ impl Supervisor { db_connection_pool.clone(), rx, db_is_mariadb.clone(), + group_deny_list.clone(), )) }; Ok(Self { config_path, config: Arc::new(Mutex::new(config)), + group_deny_list, systemd_mode, reload_message_receiver: reload_rx, shutdown_cancel_token, @@ -178,6 +202,26 @@ impl Supervisor { .context("Failed to read server configuration")?; let mut config = self.config.clone().lock_owned().await; *config = new_config; + + let group_deny_list = match &config.authorization.group_denylist_file { + Some(denylist_path) => { + let denylist = read_and_parse_group_denylist(denylist_path) + .context("Failed to read group denylist file")?; + + tracing::debug!( + "Loaded group denylist with {} entries from {:?}", + denylist.len(), + denylist_path + ); + denylist + } + None => { + tracing::debug!("No group denylist file specified, proceeding without a denylist"); + GroupDenylist::new() + } + }; + let mut group_deny_list_lock = self.group_deny_list.write().await; + *group_deny_list_lock = group_deny_list; Ok(()) } @@ -467,6 +511,7 @@ async fn listener_task( db_pool: Arc>, mut supervisor_message_receiver: broadcast::Receiver, db_is_mariadb: Arc>, + group_denylist: Arc>, ) -> anyhow::Result<()> { sd_notify::notify(false, &[sd_notify::NotifyState::Ready])?; @@ -503,8 +548,14 @@ async fn listener_task( let db_pool_clone = db_pool.clone(); let db_is_mariadb_clone = *db_is_mariadb.read().await; + let group_denylist_arc_clone = group_denylist.clone(); task_tracker.spawn(async move { - match session_handler(conn, db_pool_clone, db_is_mariadb_clone).await { + match session_handler( + conn, + db_pool_clone, + db_is_mariadb_clone, + &*group_denylist_arc_clone.read().await, + ).await { Ok(()) => {} Err(e) => { tracing::error!("Failed to run server: {}", e);