diff --git a/src/core/protocol/request_validation.rs b/src/core/protocol/request_validation.rs index 3e84791..78778d3 100644 --- a/src/core/protocol/request_validation.rs +++ b/src/core/protocol/request_validation.rs @@ -115,7 +115,7 @@ impl OwnerValidationError { } } -#[derive(Error, Debug, PartialEq, Eq, Clone, Copy, Serialize, Deserialize)] +#[derive(Error, Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub enum AuthorizationError { #[error("Sanitization error: {0}")] SanitizationError(NameValidationError), @@ -154,3 +154,126 @@ impl AuthorizationError { } } } + +const MAX_NAME_LENGTH: usize = 64; + +pub fn validate_name(name: &str) -> Result<(), NameValidationError> { + if name.is_empty() { + Err(NameValidationError::EmptyString) + } else if name.len() > MAX_NAME_LENGTH { + Err(NameValidationError::TooLong) + } else if !name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + Err(NameValidationError::InvalidCharacters) + } else { + Ok(()) + } +} + +pub fn validate_ownership_by_unix_user( + name: &str, + user: &UnixUser, +) -> Result<(), OwnerValidationError> { + let prefixes = std::iter::once(user.username.to_owned()) + .chain(user.groups.iter().cloned()) + .collect::>(); + + validate_ownership_by_prefixes(name, &prefixes) +} + +/// Core logic for validating the ownership of a database name. +/// This function checks if the given name matches any of the given prefixes. +/// These prefixes will in most cases be the user's unix username and any +/// unix groups the user is a member of. +pub fn validate_ownership_by_prefixes( + name: &str, + prefixes: &[String], +) -> Result<(), OwnerValidationError> { + if name.is_empty() { + return Err(OwnerValidationError::StringEmpty); + } + + if prefixes + .iter() + .filter(|p| name.starts_with(&(p.to_string() + "_"))) + .collect::>() + .is_empty() + { + return Err(OwnerValidationError::NoMatch); + }; + + Ok(()) +} + +pub fn validate_db_or_user_request( + db_or_user: &DbOrUser, + unix_user: &UnixUser, +) -> Result<(), AuthorizationError> { + validate_name(db_or_user.name()).map_err(AuthorizationError::SanitizationError)?; + + validate_ownership_by_unix_user(db_or_user.name(), unix_user) + .map_err(AuthorizationError::OwnershipError)?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_validate_name() { + assert_eq!(validate_name(""), Err(NameValidationError::EmptyString)); + assert_eq!(validate_name("abcdefghijklmnopqrstuvwxyz"), Ok(())); + assert_eq!(validate_name("ABCDEFGHIJKLMNOPQRSTUVWXYZ"), Ok(())); + assert_eq!(validate_name("0123456789_-"), Ok(())); + + for c in "\n\t\r !@#$%^&*()+=[]{}|;:,.<>?/".chars() { + assert_eq!( + validate_name(&c.to_string()), + Err(NameValidationError::InvalidCharacters) + ); + } + + assert_eq!(validate_name(&"a".repeat(MAX_NAME_LENGTH)), Ok(())); + + assert_eq!( + validate_name(&"a".repeat(MAX_NAME_LENGTH + 1)), + Err(NameValidationError::TooLong) + ); + } + + #[test] + fn test_validate_owner_by_prefixes() { + let prefixes = vec!["user".to_string(), "group".to_string()]; + + assert_eq!( + validate_ownership_by_prefixes("", &prefixes), + Err(OwnerValidationError::StringEmpty) + ); + + assert_eq!( + validate_ownership_by_prefixes("user_testdb", &prefixes), + Ok(()) + ); + assert_eq!( + validate_ownership_by_prefixes("group_testdb", &prefixes), + Ok(()) + ); + assert_eq!( + validate_ownership_by_prefixes("group_test_db", &prefixes), + Ok(()) + ); + assert_eq!( + validate_ownership_by_prefixes("group_test-db", &prefixes), + Ok(()) + ); + + assert_eq!( + validate_ownership_by_prefixes("nonexistent_testdb", &prefixes), + Err(OwnerValidationError::NoMatch) + ); + } +} diff --git a/src/server.rs b/src/server.rs index 831dea7..248c22c 100644 --- a/src/server.rs +++ b/src/server.rs @@ -2,7 +2,6 @@ mod authorization; pub mod command; mod common; pub mod config; -pub mod input_sanitization; pub mod landlock; pub mod session_handler; pub mod sql; diff --git a/src/server/authorization.rs b/src/server/authorization.rs index 2c7694a..58864aa 100644 --- a/src/server/authorization.rs +++ b/src/server/authorization.rs @@ -1,10 +1,7 @@ -use crate::{ - core::{ - common::UnixUser, - protocol::{CheckAuthorizationError, request_validation::AuthorizationError}, - types::DbOrUser, - }, - server::input_sanitization::{validate_name, validate_ownership_by_unix_user}, +use crate::core::{ + common::UnixUser, + protocol::{CheckAuthorizationError, request_validation::validate_db_or_user_request}, + types::DbOrUser, }; pub async fn check_authorization( @@ -14,17 +11,8 @@ pub async fn check_authorization( let mut results = std::collections::BTreeMap::new(); for db_or_user in dbs_or_users { - if let Err(err) = validate_name(db_or_user.name()) - .map_err(AuthorizationError::SanitizationError) - .map_err(CheckAuthorizationError) - { - results.insert(db_or_user.clone(), Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(db_or_user.name(), unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(CheckAuthorizationError) + if let Err(err) = + validate_db_or_user_request(&db_or_user, unix_user).map_err(CheckAuthorizationError) { results.insert(db_or_user.clone(), Err(err)); continue; diff --git a/src/server/input_sanitization.rs b/src/server/input_sanitization.rs deleted file mode 100644 index 46aa8e9..0000000 --- a/src/server/input_sanitization.rs +++ /dev/null @@ -1,136 +0,0 @@ -use crate::core::{ - common::UnixUser, - protocol::request_validation::{NameValidationError, OwnerValidationError}, -}; - -const MAX_NAME_LENGTH: usize = 64; - -pub fn validate_name(name: &str) -> Result<(), NameValidationError> { - if name.is_empty() { - Err(NameValidationError::EmptyString) - } else if name.len() > MAX_NAME_LENGTH { - Err(NameValidationError::TooLong) - } else if !name - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') - { - Err(NameValidationError::InvalidCharacters) - } else { - Ok(()) - } -} - -pub fn validate_ownership_by_unix_user( - name: &str, - user: &UnixUser, -) -> Result<(), OwnerValidationError> { - let prefixes = std::iter::once(user.username.to_owned()) - .chain(user.groups.iter().cloned()) - .collect::>(); - - validate_ownership_by_prefixes(name, &prefixes) -} - -/// Core logic for validating the ownership of a database name. -/// This function checks if the given name matches any of the given prefixes. -/// These prefixes will in most cases be the user's unix username and any -/// unix groups the user is a member of. -pub fn validate_ownership_by_prefixes( - name: &str, - prefixes: &[String], -) -> Result<(), OwnerValidationError> { - if name.is_empty() { - return Err(OwnerValidationError::StringEmpty); - } - - if prefixes - .iter() - .filter(|p| name.starts_with(&(p.to_string() + "_"))) - .collect::>() - .is_empty() - { - return Err(OwnerValidationError::NoMatch); - }; - - Ok(()) -} - -#[inline] -pub fn quote_literal(s: &str) -> String { - format!("'{}'", s.replace('\'', r"\'")) -} - -#[inline] -pub fn quote_identifier(s: &str) -> String { - format!("`{}`", s.replace('`', r"\`")) -} - -#[cfg(test)] -mod tests { - use super::*; - #[test] - fn test_quote_literal() { - let payload = "' OR 1=1 --"; - assert_eq!(quote_literal(payload), r#"'\' OR 1=1 --'"#); - } - - #[test] - fn test_quote_identifier() { - let payload = "` OR 1=1 --"; - assert_eq!(quote_identifier(payload), r#"`\` OR 1=1 --`"#); - } - - #[test] - fn test_validate_name() { - assert_eq!(validate_name(""), Err(NameValidationError::EmptyString)); - assert_eq!(validate_name("abcdefghijklmnopqrstuvwxyz"), Ok(())); - assert_eq!(validate_name("ABCDEFGHIJKLMNOPQRSTUVWXYZ"), Ok(())); - assert_eq!(validate_name("0123456789_-"), Ok(())); - - for c in "\n\t\r !@#$%^&*()+=[]{}|;:,.<>?/".chars() { - assert_eq!( - validate_name(&c.to_string()), - Err(NameValidationError::InvalidCharacters) - ); - } - - assert_eq!(validate_name(&"a".repeat(MAX_NAME_LENGTH)), Ok(())); - - assert_eq!( - validate_name(&"a".repeat(MAX_NAME_LENGTH + 1)), - Err(NameValidationError::TooLong) - ); - } - - #[test] - fn test_validate_owner_by_prefixes() { - let prefixes = vec!["user".to_string(), "group".to_string()]; - - assert_eq!( - validate_ownership_by_prefixes("", &prefixes), - Err(OwnerValidationError::StringEmpty) - ); - - assert_eq!( - validate_ownership_by_prefixes("user_testdb", &prefixes), - Ok(()) - ); - assert_eq!( - validate_ownership_by_prefixes("group_testdb", &prefixes), - Ok(()) - ); - assert_eq!( - validate_ownership_by_prefixes("group_test_db", &prefixes), - Ok(()) - ); - assert_eq!( - validate_ownership_by_prefixes("group_test-db", &prefixes), - Ok(()) - ); - - assert_eq!( - validate_ownership_by_prefixes("nonexistent_testdb", &prefixes), - Err(OwnerValidationError::NoMatch) - ); - } -} diff --git a/src/server/sql.rs b/src/server/sql.rs index 8db4e8b..b810e3f 100644 --- a/src/server/sql.rs +++ b/src/server/sql.rs @@ -1,3 +1,29 @@ pub mod database_operations; pub mod database_privilege_operations; pub mod user_operations; + +#[inline] +pub fn quote_literal(s: &str) -> String { + format!("'{}'", s.replace('\'', r"\'")) +} + +#[inline] +pub fn quote_identifier(s: &str) -> String { + format!("`{}`", s.replace('`', r"\`")) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn test_quote_literal() { + let payload = "' OR 1=1 --"; + assert_eq!(quote_literal(payload), r#"'\' OR 1=1 --'"#); + } + + #[test] + fn test_quote_identifier() { + let payload = "` OR 1=1 --"; + assert_eq!(quote_identifier(payload), r#"`\` OR 1=1 --`"#); + } +} diff --git a/src/server/sql/database_operations.rs b/src/server/sql/database_operations.rs index 1b8b6a2..434452f 100644 --- a/src/server/sql/database_operations.rs +++ b/src/server/sql/database_operations.rs @@ -6,7 +6,8 @@ use sqlx::prelude::*; use serde::{Deserialize, Serialize}; use crate::core::protocol::CompleteDatabaseNameResponse; -use crate::core::protocol::request_validation::AuthorizationError; +use crate::core::protocol::request_validation::validate_db_or_user_request; +use crate::core::types::DbOrUser; use crate::core::types::MySQLDatabase; use crate::core::types::MySQLUser; use crate::{ @@ -18,10 +19,7 @@ use crate::{ ListDatabasesResponse, }, }, - server::{ - common::create_user_group_matching_regex, - input_sanitization::{quote_identifier, validate_name, validate_ownership_by_unix_user}, - }, + server::{common::create_user_group_matching_regex, sql::quote_identifier}, }; // NOTE: this function is unsafe because it does no input validation. @@ -95,17 +93,9 @@ pub async fn create_databases( let mut results = BTreeMap::new(); for database_name in database_names { - if let Err(err) = validate_name(&database_name) - .map_err(AuthorizationError::SanitizationError) - .map_err(CreateDatabaseError::AuthorizationError) - { - results.insert(database_name.to_owned(), Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&database_name, unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(CreateDatabaseError::AuthorizationError) + if let Err(err) = + validate_db_or_user_request(&DbOrUser::Database(database_name.clone()), unix_user) + .map_err(CreateDatabaseError::AuthorizationError) { results.insert(database_name.to_owned(), Err(err)); continue; @@ -155,17 +145,9 @@ pub async fn drop_databases( let mut results = BTreeMap::new(); for database_name in database_names { - if let Err(err) = validate_name(&database_name) - .map_err(AuthorizationError::SanitizationError) - .map_err(DropDatabaseError::AuthorizationError) - { - results.insert(database_name.to_owned(), Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&database_name, unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(DropDatabaseError::AuthorizationError) + if let Err(err) = + validate_db_or_user_request(&DbOrUser::Database(database_name.clone()), unix_user) + .map_err(DropDatabaseError::AuthorizationError) { results.insert(database_name.to_owned(), Err(err)); continue; @@ -258,17 +240,9 @@ pub async fn list_databases( let mut results = BTreeMap::new(); for database_name in database_names { - if let Err(err) = validate_name(&database_name) - .map_err(AuthorizationError::SanitizationError) - .map_err(ListDatabasesError::AuthorizationError) - { - results.insert(database_name.to_owned(), Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&database_name, unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(ListDatabasesError::AuthorizationError) + if let Err(err) = + validate_db_or_user_request(&DbOrUser::Database(database_name.clone()), unix_user) + .map_err(ListDatabasesError::AuthorizationError) { results.insert(database_name.to_owned(), Err(err)); continue; diff --git a/src/server/sql/database_privilege_operations.rs b/src/server/sql/database_privilege_operations.rs index f41dd74..d7dc770 100644 --- a/src/server/sql/database_privilege_operations.rs +++ b/src/server/sql/database_privilege_operations.rs @@ -31,14 +31,16 @@ use crate::{ DiffDoesNotApplyError, GetAllDatabasesPrivilegeDataError, GetDatabasesPrivilegeDataError, ListAllPrivilegesResponse, ListPrivilegesResponse, ModifyDatabasePrivilegesError, ModifyPrivilegesResponse, - request_validation::AuthorizationError, + request_validation::validate_db_or_user_request, }, - types::{MySQLDatabase, MySQLUser}, + types::{DbOrUser, MySQLDatabase, MySQLUser}, }, server::{ 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, user_operations::unsafe_user_exists}, + sql::{ + database_operations::unsafe_database_exists, quote_identifier, + user_operations::unsafe_user_exists, + }, }, }; @@ -145,17 +147,9 @@ pub async fn get_databases_privilege_data( let mut results = BTreeMap::new(); for database_name in database_names.iter() { - if let Err(err) = validate_name(database_name) - .map_err(AuthorizationError::SanitizationError) - .map_err(GetDatabasesPrivilegeDataError::AuthorizationError) - { - results.insert(database_name.to_owned(), Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(database_name, unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(GetDatabasesPrivilegeDataError::AuthorizationError) + if let Err(err) = + validate_db_or_user_request(&DbOrUser::Database(database_name.clone()), unix_user) + .map_err(GetDatabasesPrivilegeDataError::AuthorizationError) { results.insert(database_name.to_owned(), Err(err)); continue; @@ -411,33 +405,19 @@ pub async fn apply_privilege_diffs( diff.get_database_name().to_owned(), diff.get_user_name().to_owned(), ); - if let Err(err) = validate_name(diff.get_database_name()) - .map_err(AuthorizationError::SanitizationError) - .map_err(ModifyDatabasePrivilegesError::DatabaseAuthorizationError) + if let Err(err) = validate_db_or_user_request( + &DbOrUser::Database(diff.get_database_name().to_owned()), + unix_user, + ) + .map_err(ModifyDatabasePrivilegesError::UserAuthorizationError) { results.insert(key, Err(err)); continue; } - if let Err(err) = validate_ownership_by_unix_user(diff.get_database_name(), unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(ModifyDatabasePrivilegesError::DatabaseAuthorizationError) - { - results.insert(key, Err(err)); - continue; - } - - if let Err(err) = validate_name(diff.get_user_name()) - .map_err(AuthorizationError::SanitizationError) - .map_err(ModifyDatabasePrivilegesError::UserAuthorizationError) - { - results.insert(key, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(diff.get_user_name(), unix_user) - .map_err(AuthorizationError::OwnershipError) - .map_err(ModifyDatabasePrivilegesError::UserAuthorizationError) + if let Err(err) = + validate_db_or_user_request(&DbOrUser::User(diff.get_user_name().to_owned()), unix_user) + .map_err(ModifyDatabasePrivilegesError::UserAuthorizationError) { results.insert(key, Err(err)); continue; diff --git a/src/server/sql/user_operations.rs b/src/server/sql/user_operations.rs index bdf5c21..c78f458 100644 --- a/src/server/sql/user_operations.rs +++ b/src/server/sql/user_operations.rs @@ -7,7 +7,8 @@ use serde::{Deserialize, Serialize}; use sqlx::MySqlConnection; use sqlx::prelude::*; -use crate::core::protocol::request_validation::AuthorizationError; +use crate::core::protocol::request_validation::validate_db_or_user_request; +use crate::core::types::DbOrUser; use crate::{ core::{ common::UnixUser, @@ -22,7 +23,7 @@ use crate::{ }, server::{ common::{create_user_group_matching_regex, try_get_with_binary_fallback}, - input_sanitization::{quote_literal, validate_name, validate_ownership_by_unix_user}, + sql::quote_literal, }, }; @@ -100,16 +101,7 @@ pub async fn create_database_users( let mut results = BTreeMap::new(); for db_user in db_users { - if let Err(err) = validate_name(&db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(CreateUserError::AuthorizationError) - { - results.insert(db_user, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + if let Err(err) = validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(CreateUserError::AuthorizationError) { results.insert(db_user, Err(err)); @@ -153,16 +145,7 @@ pub async fn drop_database_users( let mut results = BTreeMap::new(); for db_user in db_users { - if let Err(err) = validate_name(&db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(DropUserError::AuthorizationError) - { - results.insert(db_user, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + if let Err(err) = validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(DropUserError::AuthorizationError) { results.insert(db_user, Err(err)); @@ -204,12 +187,7 @@ pub async fn set_password_for_database_user( connection: &mut MySqlConnection, _db_is_mariadb: bool, ) -> SetUserPasswordResponse { - validate_name(db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(SetPasswordError::AuthorizationError)?; - - validate_ownership_by_unix_user(db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(SetPasswordError::AuthorizationError)?; match unsafe_user_exists(db_user, &mut *connection).await { @@ -295,16 +273,7 @@ pub async fn lock_database_users( let mut results = BTreeMap::new(); for db_user in db_users { - if let Err(err) = validate_name(&db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(LockUserError::AuthorizationError) - { - results.insert(db_user, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + if let Err(err) = validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(LockUserError::AuthorizationError) { results.insert(db_user, Err(err)); @@ -362,16 +331,7 @@ pub async fn unlock_database_users( let mut results = BTreeMap::new(); for db_user in db_users { - if let Err(err) = validate_name(&db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(UnlockUserError::AuthorizationError) - { - results.insert(db_user, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + if let Err(err) = validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(UnlockUserError::AuthorizationError) { results.insert(db_user, Err(err)); @@ -477,16 +437,7 @@ pub async fn list_database_users( let mut results = BTreeMap::new(); for db_user in db_users { - if let Err(err) = validate_name(&db_user) - .map_err(AuthorizationError::SanitizationError) - .map_err(ListUsersError::AuthorizationError) - { - results.insert(db_user, Err(err)); - continue; - } - - if let Err(err) = validate_ownership_by_unix_user(&db_user, unix_user) - .map_err(AuthorizationError::OwnershipError) + if let Err(err) = validate_db_or_user_request(&DbOrUser::User(db_user.clone()), unix_user) .map_err(ListUsersError::AuthorizationError) { results.insert(db_user, Err(err));