From 8f8386611eb163653136eed6ba7d6521334d7928 Mon Sep 17 00:00:00 2001 From: Samuel Bailey Date: Tue, 14 Jul 2020 13:48:05 +0100 Subject: [PATCH] Added user and group checks. Auto create socket dir. Parsec must be run as user `parsec` and `parsec` must be member of group `parsec-client`. This is disabled if feature `testing` is specified. Also auto created `/tmp/parsec` if it does not already exist. Signed-off-by: Samuel Bailey --- Cargo.lock | 13 +++++ Cargo.toml | 3 + README.md | 2 + ci.sh | 4 +- src/front/domain_socket.rs | 113 ++++++++++++++++++++++++++++++++++--- 5 files changed, 125 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a8b9900..7e99a511 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -766,6 +766,7 @@ dependencies = [ "env_logger", "hex", "lazy_static", + "libc", "log", "parsec-interface", "picky", @@ -773,6 +774,7 @@ dependencies = [ "picky-asn1-der", "picky-asn1-x509", "pkcs11", + "psa-crypto", "rand", "ring", "sd-notify", @@ -783,6 +785,7 @@ dependencies = [ "threadpool", "toml 0.4.10", "tss-esapi", + "users", "uuid", "version", "zeroize", @@ -1510,6 +1513,16 @@ version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" +[[package]] +name = "users" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa4227e95324a443c9fcb06e03d4d85e91aabe9a5a02aa818688b6918b6af486" +dependencies = [ + "libc", + "log", +] + [[package]] name = "uuid" version = "0.7.4" diff --git a/Cargo.toml b/Cargo.toml index 756b4f0b..c6a90c8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,8 @@ picky = "5.0.0" psa-crypto = { version = "0.3.0" , default-features = false, features = ["operations"], optional = true } zeroize = { version = "1.1.0", features = ["zeroize_derive"] } picky-asn1-x509 = { version = "0.1.0", optional = true } +users = "0.10.0" +libc = "0.2.72" [dev-dependencies] ring = "0.16.12" @@ -62,6 +64,7 @@ features = ["docs"] [features] default = [] +no-parsec-user-and-clients-group = [] mbed-crypto-provider = ["psa-crypto"] pkcs11-provider = ["pkcs11", "picky-asn1-der", "picky-asn1", "picky-asn1-x509"] tpm-provider = ["tss-esapi", "picky-asn1-der", "picky-asn1", "picky-asn1-x509"] diff --git a/README.md b/README.md index 2f68df1f..b3c3a131 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,8 @@ This project uses the following third party crates: * sha2 (MIT and Apache-2.0) * hex (MIT and Apache-2.0) * picky (MIT and Apache-2.0) +* users (MIT) +* libc (MIT and Apache-2.0) This project uses the following third party libraries: * [**Mbed Crypto**](https://github.com/ARMmbed/mbed-crypto) (Apache-2.0) diff --git a/ci.sh b/ci.sh index 016ed461..9792fa10 100755 --- a/ci.sh +++ b/ci.sh @@ -67,9 +67,9 @@ while [ "$#" -gt 0 ]; do PROVIDER_NAME=$1 cp $(pwd)/e2e_tests/provider_cfg/$1/config.toml $CONFIG_PATH if [ "$PROVIDER_NAME" = "all" ]; then - FEATURES="--features=all-providers" + FEATURES="--features=all-providers,no-parsec-user-and-clients-group" else - FEATURES="--features=$1-provider" + FEATURES="--features=$1-provider,no-parsec-user-and-clients-group" fi ;; *) diff --git a/src/front/domain_socket.rs b/src/front/domain_socket.rs index 9caf915f..26af7aef 100644 --- a/src/front/domain_socket.rs +++ b/src/front/domain_socket.rs @@ -8,6 +8,8 @@ use super::listener; use listener::Connection; use listener::Listen; use log::error; +#[cfg(not(feature = "no-parsec-user-and-clients-group"))] +use std::ffi::CString; use std::fs; use std::fs::Permissions; use std::io::{Error, ErrorKind, Result}; @@ -18,6 +20,10 @@ use std::path::Path; use std::time::Duration; static SOCKET_PATH: &str = "/tmp/parsec/parsec.sock"; +#[cfg(not(feature = "no-parsec-user-and-clients-group"))] +const PARSEC_USERNAME: &str = "parsec"; +#[cfg(not(feature = "no-parsec-user-and-clients-group"))] +const PARSEC_GROUPNAME: &str = "parsec-clients"; /// Unix Domain Socket IPC manager /// @@ -32,13 +38,11 @@ pub struct DomainSocketListener { impl DomainSocketListener { /// Initialise the connection to the Unix socket. - /// - /// # Panics - /// - if a file/socket exists at the path specified for the socket and `remove_file` - /// fails - /// - if binding to the socket path fails pub fn new(timeout: Duration) -> Result { - // If this Parsec instance was socket activated (see the `parsec.socket` + #[cfg(not(feature = "no-parsec-user-and-clients-group"))] + DomainSocketListener::check_user_details()?; + + // is Parsec instance was socket activated (see the `parsec.socket` // file), the listener will be opened by systemd and passed to the // process. // If Parsec was service activated or not started under systemd, this @@ -46,8 +50,12 @@ impl DomainSocketListener { let listener = match sd_notify::listen_fds()? { 0 => { let socket = Path::new(SOCKET_PATH); - - if socket.exists() { + let parent_dir = socket.parent().unwrap(); + if !parent_dir.exists() { + fs::create_dir_all(parent_dir)?; + #[cfg(not(feature = "no-parsec-user-and-clients-group"))] + DomainSocketListener::set_socket_dir_permissions(parent_dir)?; + } else if socket.exists() { fs::remove_file(&socket)?; } @@ -84,6 +92,95 @@ impl DomainSocketListener { Ok(Self { listener, timeout }) } + + #[cfg(not(feature = "no-parsec-user-and-clients-group"))] + fn set_socket_dir_permissions(parent_dir: &Path) -> Result<()> { + if let Some(parent_dir_str) = parent_dir.to_str() { + fs::set_permissions(parent_dir, Permissions::from_mode(0o750))?; + // Although `parsec` has to be part of the `parsec_clients` group, it may not be the primary group. Therefore force group ownership to `parsec_clients` + if unsafe { + let parent_dir_cstr = CString::new(parent_dir_str) + .expect("Failed to convert socket path parent to cstring"); + { + libc::chown( + parent_dir_cstr.as_ptr(), + users::get_current_uid(), // To get to this point, user has to be `parsec` + users::get_group_by_name(PARSEC_GROUPNAME).unwrap().gid(), // `parsec_clients` exists by this point so should be safe + ) + } + } != 0 + { + error!( + "Changing ownership of {} to user {} and group {} failed.", + parent_dir_str, PARSEC_USERNAME, PARSEC_GROUPNAME + ); + return Err(Error::new( + ErrorKind::Other, + "Changing ownership of socket directory failed", + )); + } + } else { + error!( + "Error converting {} parent directory to string.", + SOCKET_PATH + ); + return Err(Error::new( + ErrorKind::InvalidInput, + "Error retrieving parent directory for socket", + )); + } + Ok(()) + } + + #[cfg(not(feature = "no-parsec-user-and-clients-group"))] + fn check_user_details() -> Result<()> { + // Check Parsec is running as parsec user + if users::get_current_username() != Some(PARSEC_USERNAME.into()) { + error!( + "Incorrect user. Parsec should be run as user {}.", + PARSEC_USERNAME + ); + return Err(Error::new( + ErrorKind::PermissionDenied, + "Parsec run as incorrect user", + )); + } + // Check Parsec client group exists and parsec user is a member of it + if let Some(parsec_clients_group) = users::get_group_by_name(PARSEC_GROUPNAME) { + if let Some(groups) = users::get_user_groups(PARSEC_USERNAME, users::get_current_gid()) + { + // Split to make `clippy` happy + let parsec_user_in_parsec_clients_group = groups.into_iter().any(|group| { + group.gid() == parsec_clients_group.gid() + && group.name() == parsec_clients_group.name() + }); + // Check the parsec user is a member of the parsec clients group + if parsec_user_in_parsec_clients_group { + return Ok(()); + } + error!( + "{} user not a member of {}.", + PARSEC_USERNAME, PARSEC_GROUPNAME + ); + Err(Error::new( + ErrorKind::PermissionDenied, + "User permissions incorrect", + )) + } else { + error!("Retrieval of groups for user {} failed.", PARSEC_USERNAME); + Err(Error::new( + ErrorKind::InvalidInput, + "Failed to retrieve user groups", + )) + } + } else { + error!("{} group does not exist.", PARSEC_GROUPNAME); + Err(Error::new( + ErrorKind::PermissionDenied, + "Group permissions incorrect", + )) + } + } } impl Listen for DomainSocketListener {