Merge pull request #205 from sbailey-arm/add-user-and-group-checks-to-service

Added user and group checks. Auto create socket dir.

Author: hug-dev

Cargo.lock

@@ -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"]

Cargo.toml

@@ -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)

README.md

@@ -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
         ;;
         *)

ci.sh

@@ -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,24 +38,26 @@ 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<Self> {
-        // 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
         // will return `0`.
         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)?;
+                } else if socket.exists() {
                     fs::remove_file(&socket)?;
                 }
+                #[cfg(not(feature = "no-parsec-user-and-clients-group"))]
+                DomainSocketListener::set_socket_dir_permissions(parent_dir)?;
 
                 let listener = UnixListener::bind(SOCKET_PATH)?;
                 listener.set_nonblocking(true)?;
@@ -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 {