Merge pull request #56 from ionut-arm/remove-fs-check

Remove filesystem checks

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),
         }
     }
 }