Merge pull request #28 from DemiMarie-parity/strict-ipc-permissions

Don’t allow other users to connect by default
paritytech · Jun 16, 2020 · 96c9bf6 · 96c9bf6
2 parents 3dd7803 + 86e09f0
commit 96c9bf6
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 8 deletions.
diff --git a/src/unix.rs b/src/unix.rs
@@ -16,16 +16,17 @@ pub struct SecurityAttributes {
 }
 
 impl SecurityAttributes {
-    /// New default security attributes.
+    /// New default security attributes. These only allow access by the
+    /// process’s own user and the system administrator.
     pub fn empty() -> Self {
         SecurityAttributes {
-            mode: None
+            mode: Some(0o600)
         }
     }
 
     /// New security attributes that allow everyone to connect.
     pub fn allow_everyone_connect(mut self) -> io::Result<Self> {
-        self.mode = Some(0o777);
+        self.mode = Some(0o666);
         Ok(self)
     }
 
@@ -36,6 +37,9 @@ impl SecurityAttributes {
     }
 
     /// New security attributes that allow everyone to create.
+    ///
+    /// This does not work on unix, where it is equivalent to
+    /// [`SecurityAttributes::allow_everyone_connect`].
     pub fn allow_everyone_create() -> io::Result<Self> {
         Ok(SecurityAttributes {
             mode: None
@@ -44,9 +48,9 @@ impl SecurityAttributes {
 
     /// called in unix, after server socket has been created
     /// will apply security attributes to the socket.
-     pub(crate) unsafe fn apply_permissions(&self, path: &str) -> io::Result<()> {
+    pub(crate) unsafe fn apply_permissions(&self, path: &str) -> io::Result<()> {
         let path = CString::new(path.to_owned())?;
-         if let Some(mode) = self.mode {
+        if let Some(mode) = self.mode {
             if chmod(path.as_ptr(), mode as _) == -1 {
                 return Err(Error::last_os_error())
             }

diff --git a/src/win.rs b/src/win.rs
@@ -51,6 +51,7 @@ impl Endpoint {
             NamedPipeBuilder::new(&self.path)
                 .first(true)
                 .inbound(true)
+                .accept_remote(false)
                 .outbound(true)
                 .out_buffer_size(65536)
                 .in_buffer_size(65536)
@@ -156,7 +157,7 @@ impl Stream for Incoming {
             }
             Err(e) => {
                 if e.kind() == io::ErrorKind::WouldBlock {
-                    self.inner.pipe.clear_write_ready(ctx);
+                    self.inner.pipe.clear_write_ready(ctx)?;
                     Poll::Pending
                 } else {
                     Poll::Ready(Some(Err(e)))
@@ -172,9 +173,10 @@ pub struct Connection {
 }
 
 impl Connection {
+    /// Wraps an existing named pipe
     pub fn wrap(pipe: NamedPipe) -> Self {
         Self { inner: pipe }
-	}
+    }
 }
 
 impl AsyncRead for Connection {
@@ -218,10 +220,24 @@ pub struct SecurityAttributes {
     attributes: Option<InnerAttributes>,
 }
 
+pub const DEFAULT_SECURITY_ATTRIBUTES: SecurityAttributes = SecurityAttributes {
+    attributes: Some(InnerAttributes {
+        descriptor: SecurityDescriptor {
+            descriptor_ptr: ptr::null_mut(),
+        },
+        acl: Acl { acl_ptr: ptr::null_mut() },
+        attrs: SECURITY_ATTRIBUTES {
+            nLength: mem::size_of::<SECURITY_ATTRIBUTES>() as u32,
+            lpSecurityDescriptor: ptr::null_mut(),
+            bInheritHandle: 0,
+        },
+    })
+};
+
 impl SecurityAttributes {
     /// New default security attributes.
     pub fn empty() -> SecurityAttributes {
-        SecurityAttributes { attributes: None }
+        DEFAULT_SECURITY_ATTRIBUTES
     }
 
     /// New default security attributes that allow everyone to connect.