Remove filesystem checks

This commit removes the filesystem checks for correct permissions on
Parsec "assets". The checks can now be removed because the new locations
in the filesystem mean that only a privileged user can create the files.

Signed-off-by: Ionut Mihalcea <[email protected]>

Author: ionut-arm

Cargo.toml

@@ -24,5 +24,4 @@ users = "0.10.0"
 mockstream = "0.0.3"
 
 [features]
-testing = ["parsec-interface/testing", "no-fs-permission-check"]
-no-fs-permission-check = []
+testing = ["parsec-interface/testing"]

README.md

@@ -7,18 +7,6 @@
 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.

src/core/ipc_handler/unix_socket.rs

@@ -7,17 +7,6 @@ use std::os::unix::net::UnixStream;
 use std::path::PathBuf;
 use std::time::Duration;
 
-#[cfg(not(feature = "no-fs-permission-check"))]
-use log::error;
-#[cfg(not(feature = "no-fs-permission-check"))]
-use std::ffi::OsStr;
-#[cfg(not(feature = "no-fs-permission-check"))]
-use std::fs;
-#[cfg(not(feature = "no-fs-permission-check"))]
-use std::io::{Error, ErrorKind};
-#[cfg(not(feature = "no-fs-permission-check"))]
-use std::os::unix::fs::MetadataExt;
-
 const DEFAULT_SOCKET_PATH: &str = "/run/parsec/parsec.sock";
 const DEFAULT_TIMEOUT: Duration = Duration::from_secs(1);
 
@@ -32,9 +21,6 @@ pub struct Handler {
 
 impl Connect for Handler {
     fn connect(&self) -> Result<Box<dyn ReadWrite>> {
-        #[cfg(not(feature = "no-fs-permission-check"))]
-        self.secure_parsec_socket_folder()?;
-
         let stream = UnixStream::connect(self.path.clone()).map_err(ClientErrorKind::Ipc)?;
 
         stream
@@ -57,74 +43,6 @@ 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(feature = "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 {

src/error.rs

@@ -63,26 +63,10 @@ impl PartialEq for ClientErrorKind {
                 }
             }
             ClientErrorKind::InvalidServiceResponseType => {
-                if let ClientErrorKind::InvalidServiceResponseType = other {
-                    true
-                } else {
-                    false
-                }
-            }
-            ClientErrorKind::InvalidProvider => {
-                if let ClientErrorKind::InvalidProvider = other {
-                    true
-                } else {
-                    false
-                }
-            }
-            ClientErrorKind::NoProvider => {
-                if let ClientErrorKind::NoProvider = other {
-                    true
-                } else {
-                    false
-                }
+                matches!(other, ClientErrorKind::InvalidServiceResponseType)
             }
+            ClientErrorKind::InvalidProvider => matches!(other, ClientErrorKind::InvalidProvider),
+            ClientErrorKind::NoProvider => matches!(other, ClientErrorKind::NoProvider),
         }
     }
 }