Merge pull request #33 from hug-dev/socket-location

Change socket location and add checks

Author: hug-dev

Cargo.toml

@@ -20,9 +20,11 @@ log = "0.4.8"
 derivative = "2.1.1"
 uuid = "0.7.4"
 zeroize = "1.1.0"
+users = "0.10.0"
 
 [dev-dependencies]
 mockstream = "0.0.3"
 
 [features]
-testing = ["parsec-interface/testing"]
+testing = ["parsec-interface/testing", "no-fs-permission-check"]
+no-fs-permission-check = []

README.md

@@ -7,6 +7,18 @@
 This repository contains a Rust client for consuming the API provided by the [Parsec service](https://github.com/parallaxsecond/parsec).
 The low-level functionality that this library uses for IPC is implemented in the [interface crate](https://github.com/parallaxsecond/parsec-interface-rs).
 
+## Filesystem permission check
+
+To make sure that the client is communicating with a trusted Parsec service, some permission checks
+are done on the socket location. Please see the
+[Recommendations for Secure Deployment](https://parallaxsecond.github.io/parsec-book/threat_model/secure_deployment.html)
+for more information.
+This feature is activated by default but, knowing the risks, you can remove it with:
+```
+cargo build --features no-fs-permission-check
+```
+It is also desactivated for testing.
+
 ## License
 
 The software is provided under Apache-2.0. Contributions to this project are accepted under the same license.
@@ -18,6 +30,7 @@ This project uses the following third party crates:
 * derivative (MIT and Apache-2.0)
 * mockstream (MIT)
 * uuid (MIT and Apache-2.0)
+* users (MIT)
 
 ## Contributing
 

src/core/ipc_handler/unix_socket.rs

@@ -3,11 +3,16 @@
 //! Handler for Unix domain sockets
 use super::{Connect, ReadWrite};
 use crate::error::{ClientErrorKind, Result};
+use log::error;
+use std::ffi::OsStr;
+use std::fs;
+use std::io::{Error, ErrorKind};
+use std::os::unix::fs::MetadataExt;
 use std::os::unix::net::UnixStream;
 use std::path::PathBuf;
 use std::time::Duration;
 
-const DEFAULT_SOCKET_PATH: &str = "/tmp/security-daemon-socket";
+const DEFAULT_SOCKET_PATH: &str = "/tmp/parsec/parsec.sock";
 const DEFAULT_TIMEOUT: Duration = Duration::from_secs(1);
 
 /// IPC handler for Unix domain sockets
@@ -21,6 +26,9 @@ pub struct Handler {
 
 impl Connect for Handler {
     fn connect(&self) -> Result<Box<dyn ReadWrite>> {
+        #[cfg(not(no_fs_permission_check))]
+        self.secure_parsec_socket_folder()?;
+
         let stream = UnixStream::connect(self.path.clone()).map_err(ClientErrorKind::Ipc)?;
 
         stream
@@ -43,6 +51,74 @@ impl Handler {
     pub fn new(path: PathBuf, timeout: Option<Duration>) -> Self {
         Handler { path, timeout }
     }
+
+    /// Checks if the socket is inside a folder with correct owners and permissions to make sure it
+    /// is from the Parsec service.
+    #[cfg(not(no_fs_permission_check))]
+    fn secure_parsec_socket_folder(&self) -> Result<()> {
+        let mut socket_dir = self.path.clone();
+        if !socket_dir.pop() {
+            return Err(ClientErrorKind::Ipc(Error::new(
+                ErrorKind::Other,
+                "Socket permission checks failed",
+            ))
+            .into());
+        }
+        let meta = fs::metadata(socket_dir).map_err(ClientErrorKind::Ipc)?;
+
+        match users::get_user_by_uid(meta.uid()) {
+            Some(user) => {
+                if user.name() != OsStr::new("parsec") {
+                    error!("The socket directory must be owned by the parsec user.");
+                    return Err(ClientErrorKind::Ipc(Error::new(
+                        ErrorKind::Other,
+                        "Socket permission checks failed",
+                    ))
+                    .into());
+                }
+            }
+            None => {
+                error!("Can not find socket directory user owner.");
+                return Err(ClientErrorKind::Ipc(Error::new(
+                    ErrorKind::Other,
+                    "Socket permission checks failed",
+                ))
+                .into());
+            }
+        }
+
+        match users::get_group_by_gid(meta.gid()) {
+            Some(group) => {
+                if group.name() != OsStr::new("parsec-clients") {
+                    error!("The socket directory must be owned by the parsec-clients group.");
+                    return Err(ClientErrorKind::Ipc(Error::new(
+                        ErrorKind::Other,
+                        "Socket permission checks failed",
+                    ))
+                    .into());
+                }
+            }
+            None => {
+                error!("Can not find socket directory group owner.");
+                return Err(ClientErrorKind::Ipc(Error::new(
+                    ErrorKind::Other,
+                    "Socket permission checks failed",
+                ))
+                .into());
+            }
+        }
+
+        if (meta.mode() & 0o777) != 0o750 {
+            error!("The permission bits of the folder containing the Parsec socket must be 750.");
+            return Err(ClientErrorKind::Ipc(Error::new(
+                ErrorKind::Other,
+                "Socket permission checks failed",
+            ))
+            .into());
+        }
+
+        Ok(())
+    }
 }
 
 impl Default for Handler {