local socket: tighten access rights

Author: coot

ouroboros-network/changelog.d/20260505_173520_coot_local_socket_access.md

@@ -0,0 +1,38 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+For top level release notes, leave all the headers commented out.
+-->
+
+### Breaking
+
+- Added `Interfaces.diNtcConfigureSocketFileDescriptor`. It receives the local
+  socket file descriptor.  It is called before `bind.
+- Added `Interfaces.diNtcConfigureSocketFile`.  It recieves the file path of
+  the local socket.  It is called after `bind` and before `listen`.
+- Added two `DiffusionTracer` constructors: `ConfiguredLocalSocket` (fired
+  after the local socket's permissions are tightened) and
+  `InsecureLocalSocketDirectory` (warning when the parent directory of the
+  local socket has group or other write permission).
+- `mkInterfaces`'s tracer type is now specialised to `LocalAddress`
+  (`Tracer IO (DiffusionTracer ntnAddr LocalAddress)`).
+
+### Non-Breaking
+
+- Tighten access rights on the local (Node-to-Client) socket: on POSIX the
+  socket file is restricted to mode `0600` (owner read/write) via both
+  `fchmod` on the socket file descriptor and `chmod` on the bound path.
+  No-op on Windows (named pipes use ACLs) and on WASM (WASI does not
+  implement the POSIX permission model meaningfully).
+- Trace a warning at start-up if the parent directory of the local-socket
+  path has `group` or `other` write permission, since a `0600` socket
+  inside such a directory remains vulnerable to manipulation by another
+  local user with write access to that directory.
+
+<!--
+### Patch
+
+- A bullet item for the Patch category.
+
+-->

ouroboros-network/framework/lib/Ouroboros/Network/Snocket.hs

@@ -10,6 +10,7 @@
 {-# LANGUAGE RankNTypes                 #-}
 {-# LANGUAGE ScopedTypeVariables        #-}
 {-# LANGUAGE StandaloneDeriving         #-}
+{-# LANGUAGE TypeApplications           #-}
 {-# LANGUAGE TypeSynonymInstances       #-}
 
 #if !defined(mingw32_HOST_OS)
@@ -46,6 +47,8 @@ module Ouroboros.Network.Snocket
   , FileDescriptor
   , socketFileDescriptor
   , localSocketFileDescriptor
+  , configureLocalSocketFileDescriptor
+  , configureLocalSocketFile
     -- ** for testing
   , invalidFileDescriptor
     -- * Re-exports
@@ -56,12 +59,12 @@ import Control.DeepSeq (NFData (..))
 import Control.Exception
 import Data.Bifoldable (Bifoldable (..))
 import Data.Bifunctor (Bifunctor (..))
+import Data.Bits
 import Data.Hashable
 import Data.Word
 import GHC.Generics (Generic)
 import Quiet (Quiet (..))
 #if defined(mingw32_HOST_OS)
-import Data.Bits
 import Foreign.Ptr (IntPtr (..), ptrToIntPtr)
 import System.Win32 qualified as Win32
 import System.Win32.Async qualified as Win32.Async
@@ -72,6 +75,10 @@ import "Win32" System.Win32.NamedPipes qualified as Win32.NamedPipes
 -- earlier version of Win32-network comes with `NamedPipes` APIs
 import "Win32-network" System.Win32.NamedPipes qualified as Win32.NamedPipes
 #endif
+#elif !defined(wasm32_HOST_ARCH)
+import Foreign.C (CInt)
+import System.Posix.Files qualified as Unix
+import System.Posix.Types qualified as Unix
 #endif
 
 import NoThunks.Class
@@ -632,6 +639,49 @@ localSocketFileDescriptor =
 localSocketFileDescriptor = socketFileDescriptor . getLocalHandle
 #endif
 
+-- | Restrict the local (Node-to-Client) socket so that only the owning user or
+-- one in its group can connect.
+--
+-- On POSIX this sets mode @0600@ on the AF_UNIX socket.  Only @fchmod@ on the
+-- socket file descriptor is called.  On `Linux` this will also set file mode on
+-- the socket file once its created when `bind` is called.
+--
+-- On Windows this is a no-op: the local handle is a named pipe whose ACL is
+-- set at creation time and is not configurable via POSIX file modes.
+--
+-- On WASM this is a no-op: WASI does not implement the POSIX permission
+-- model in any meaningful way.
+--
+-- Must be called between `Snocket.open` and 'Snocket.bind'.
+configureLocalSocketFileDescriptor :: LocalSocket -> IO ()
+#if defined(mingw32_HOST_OS) || defined(wasm32_HOST_ARCH)
+configureLocalSocketFileDescriptor _ = return ()
+#else
+configureLocalSocketFileDescriptor sock = do
+  FileDescriptor fd <- localSocketFileDescriptor sock
+  let mode = Unix.ownerReadMode .|. Unix.ownerWriteMode
+  Unix.setFdMode (Unix.Fd (fromIntegral @Int @CInt fd)) mode
+#endif
+
+-- | Restrict the local (Node-to-Client) socket so that only the owning user or
+-- one in its group can connect.
+--
+-- This must be called between `bind` and `listen`.  On macOS, this is the only
+-- way to set file permissions on the socket.
+--
+-- On Windows and WASM this is a no-op (see
+-- `configureLocalSocketFileDescriptor`).
+--
+configureLocalSocketFile :: LocalAddress -> IO ()
+#if defined(mingw32_HOST_OS) || defined(wasm32_HOST_ARCH)
+configureLocalSocketFile _ = return ()
+#else
+configureLocalSocketFile LocalAddress { getFilePath = path } = do
+  let mode = Unix.ownerReadMode .|. Unix.ownerWriteMode
+  Unix.setFileMode path mode
+#endif
+
+
 -- | invalidFileDescriptor - when we need something for testing/simulation
 invalidFileDescriptor :: FileDescriptor
 invalidFileDescriptor = FileDescriptor (-1)

ouroboros-network/lib/Ouroboros/Network/Diffusion.hs

@@ -25,6 +25,9 @@ import Control.Concurrent.Class.MonadMVar (MonadMVar)
 import Control.Concurrent.Class.MonadSTM.Strict
 import Control.DeepSeq (NFData)
 import Control.Exception (IOException)
+#if !defined(mingw32_HOST_OS) && !defined(wasm32_HOST_ARCH)
+import Control.Monad (when)
+#endif
 import Control.Monad.Class.MonadAsync (Async, MonadAsync)
 import Control.Monad.Class.MonadAsync qualified as Async
 import Control.Monad.Class.MonadFork
@@ -33,6 +36,10 @@ import Control.Monad.Class.MonadTime.SI
 import Control.Monad.Class.MonadTimer.SI
 import Control.Monad.Fix (MonadFix)
 import Control.Tracer (Tracer, contramap, nullTracer, traceWith)
+#if !defined(mingw32_HOST_OS) && !defined(wasm32_HOST_ARCH)
+import Data.Bits ((.|.))
+import System.Posix.Files qualified as Unix
+#endif
 import Data.ByteString.Lazy (ByteString)
 import Data.Hashable (Hashable)
 import Data.IP qualified as IP
@@ -77,8 +84,10 @@ import Ouroboros.Network.PeerSharing (PeerSharingRegistry (..))
 import Ouroboros.Network.Protocol.Handshake
 import Ouroboros.Network.RethrowPolicy
 import Ouroboros.Network.Server qualified as Server
-import Ouroboros.Network.Snocket (LocalAddress, LocalSocket (..), RemoteAddress,
-           localSocketFileDescriptor, makeLocalBearer, makeSocketBearer')
+import Ouroboros.Network.Snocket (LocalAddress (..), LocalSocket (..),
+           RemoteAddress, configureLocalSocketFile,
+           configureLocalSocketFileDescriptor, localSocketFileDescriptor,
+           makeLocalBearer, makeSocketBearer')
 import Ouroboros.Network.Snocket qualified as Snocket
 import Ouroboros.Network.Socket (configureSocket, configureSystemdSocket)
 import Ouroboros.Network.Util (PrettyShow (..))
@@ -164,6 +173,8 @@ runM Interfaces
        , diNtcSnocket
        , diNtcBearer
        , diNtcGetFileDescriptor
+       , diNtcConfigureSocketFileDescriptor
+       , diNtcConfigureSocketFile
        , diRng
        , diDnsActions
        , diConnStateIdSupply
@@ -318,7 +329,10 @@ runM Interfaces
     mkLocalThread :: ThreadId m -> Either ntcFd ntcAddr -> m Void
     mkLocalThread mainThreadId localAddr = do
      labelThisThread "diffusion-local"
-     withLocalSocket tracer diNtcGetFileDescriptor diNtcSnocket localAddr
+     withLocalSocket tracer diNtcGetFileDescriptor
+                     diNtcConfigureSocketFileDescriptor
+                     diNtcConfigureSocketFile
+                     diNtcSnocket localAddr
       $ \localSocket -> do
         localInbInfoChannel <- newInformationChannel
 
@@ -912,7 +926,7 @@ run extraParams tracers args apps = do
 --
 
 mkInterfaces :: IOManager
-             -> Tracer IO (DiffusionTracer ntnAddr ntcAddr)
+             -> Tracer IO (DiffusionTracer ntnAddr LocalAddress)
              -> DiffTime
              -> IO (Interfaces Socket
                                RemoteAddress
@@ -921,7 +935,6 @@ mkInterfaces :: IOManager
                                Resolver
                                IO)
 mkInterfaces iocp tracer egressPollInterval = do
-
   diRng <- newStdGen
   diConnStateIdSupply <- atomically $ CM.newConnStateIdSupply Proxy
 
@@ -941,11 +954,45 @@ mkInterfaces iocp tracer egressPollInterval = do
     diNtcSnocket           = Snocket.localSnocket iocp,
     diNtcBearer            = makeLocalBearer,
     diNtcGetFileDescriptor = localSocketFileDescriptor,
+    diNtcConfigureSocketFileDescriptor = configureLocalSocketFileDescriptor,
+    diNtcConfigureSocketFile           = \addr -> do
+      configureLocalSocketFile addr
+      checkLocalSocketDirectory tracer addr,
     diDnsActions           = RootPeersDNS.ioDNSActions,
     diRng,
     diConnStateIdSupply
   }
 
+-- | Trace a warning if the parent directory of a local-socket path has
+-- @group@ or @other@ write permission.  A @0600@ socket inside such a
+-- directory is still vulnerable to manipulation by another local user
+-- with write access to that directory.
+--
+-- No-op on Windows (named pipes do not live in a POSIX-permissioned
+-- directory) and on WASM (WASI does not implement the POSIX permission
+-- model in any meaningful way).
+checkLocalSocketDirectory
+  :: Tracer IO (DiffusionTracer ntnAddr LocalAddress)
+  -> LocalAddress
+  -> IO ()
+#if defined(mingw32_HOST_OS) || defined(wasm32_HOST_ARCH)
+checkLocalSocketDirectory _ _ = return ()
+#else
+checkLocalSocketDirectory tracer addr@(LocalAddress path) = do
+  let dir = parentDirectory path
+  st <- Unix.getFileStatus dir
+  let mode      = Unix.fileMode st
+      writeMask = Unix.groupWriteMode .|. Unix.otherWriteMode
+  when (Unix.intersectFileModes mode writeMask /= Unix.nullFileMode) $
+    traceWith tracer (InsecureLocalSocketDirectory addr (fromIntegral mode))
+  where
+    parentDirectory :: FilePath -> FilePath
+    parentDirectory p = case break (== '/') (reverse p) of
+      (_, '/' : revParent) | not (null revParent) -> reverse revParent
+      (_, '/' : _)                                -> "/"
+      _                                           -> "."
+#endif
+
 --
 -- Data flow
 --

ouroboros-network/lib/Ouroboros/Network/Diffusion/Types.hs

@@ -61,6 +61,7 @@ import Data.Maybe.Strict
 import Data.Set (Set)
 import Data.Typeable (Typeable)
 import Data.Void (Void)
+import Data.Word (Word32)
 import System.Random (StdGen)
 
 import Network.Mux qualified as Mx
@@ -109,8 +110,15 @@ data DiffusionTracer ntnAddr ntcAddr
   | CreateSystemdSocketForSnocketPath ntcAddr
   | CreatedLocalSocket ntcAddr
   | ConfiguringLocalSocket ntcAddr FileDescriptor
+  | ConfiguredLocalSocket ntcAddr FileDescriptor
   | ListeningLocalSocket ntcAddr FileDescriptor
   | LocalSocketUp  ntcAddr FileDescriptor
+  -- | The parent directory of the local socket has @group@ or @other@ write
+  -- bits set.  This is a warning, not a failure: the socket itself is
+  -- mode @0600@, but the parent directory's permissions may still allow
+  -- another local user to rename or unlink the socket file.  The 'Word32'
+  -- is the parent directory's POSIX mode.
+  | InsecureLocalSocketDirectory ntcAddr Word32
   -- Rename as 'CreateServerSocket'
   | CreatingServerSocket ntnAddr
   | ConfiguringServerSocket ntnAddr
@@ -767,6 +775,16 @@ data Interfaces ntnFd ntnAddr ntcFd ntcAddr
         diNtcGetFileDescriptor
           :: ntcFd -> m FileDescriptor,
 
+        -- | configure node-to-client socket file descriptor (called between
+        -- `socket` and `bind`).
+        diNtcConfigureSocketFileDescriptor
+          :: ntcFd -> m (),
+
+        -- | configure node-to-client socket file (called between
+        -- `bind` and `listen`).
+        diNtcConfigureSocketFile
+          :: ntcAddr -> m (),
+
         -- | diffusion pseudo random generator. It is split between various
         -- components that need randomness, e.g. inbound governor, peer
         -- selection, policies, etc.

ouroboros-network/lib/Ouroboros/Network/Diffusion/Utils.hs

@@ -81,11 +81,20 @@ withLocalSocket :: forall ntnAddr ntcFd ntcAddr m a.
                    )
                 => Tracer m (DiffusionTracer ntnAddr ntcAddr)
                 -> (ntcFd -> m FileDescriptor)
+                -> (ntcFd -> m ())
+                -- ^ configure the local socket file descriptor (e.g. set
+                -- @0600@ permissions on POSIX)
+                -> (ntcAddr -> m ())
+                -- ^ configure the local socket file (e.g. set @0600@
+                -- permissions on POSIX)
                 -> Snocket m ntcFd ntcAddr
                 -> Either ntcFd ntcAddr
                 -> (ntcFd -> m a)
                 -> m a
-withLocalSocket tracer getFileDescriptor sn localAddress k =
+withLocalSocket tracer getFileDescriptor
+                configureSocketFileDescriptor
+                configureSocketFile
+                sn localAddress k =
   bracket
     (
       case localAddress of
@@ -116,7 +125,11 @@ withLocalSocket tracer getFileDescriptor sn localAddress k =
       Right (sd, addr) -> do
         traceWith tracer . ConfiguringLocalSocket addr
            =<< getFileDescriptor sd
+        configureSocketFileDescriptor sd
         Snocket.bind sn sd addr
+        configureSocketFile addr
+        traceWith tracer . ConfiguredLocalSocket addr
+           =<< getFileDescriptor sd
         traceWith tracer . ListeningLocalSocket addr
            =<< getFileDescriptor sd
         Snocket.listen sn sd

ouroboros-network/orphan-instances/Ouroboros/Network/OrphanInstances.hs

@@ -704,6 +704,11 @@ instance (ToJSON localAddress, ToJSON remoteAddress) => ToJSON (DiffusionTracer
     , "address" .= localAddress
     , "socket" .= String (pack (show socket))
     ]
+  toJSON (ConfiguredLocalSocket localAddress socket) = object
+    [ "kind" .= String "ConfiguredLocalSocket"
+    , "address" .= localAddress
+    , "socket" .= String (pack (show socket))
+    ]
   toJSON (ListeningLocalSocket localAddress socket) = object
     [ "kind" .= String "ListeningLocalSocket"
     , "address" .= localAddress
@@ -714,6 +719,11 @@ instance (ToJSON localAddress, ToJSON remoteAddress) => ToJSON (DiffusionTracer
     , "address" .= localAddress
     , "socket" .= String (pack (show fd))
     ]
+  toJSON (InsecureLocalSocketDirectory localAddress mode) = object
+    [ "kind" .= String "InsecureLocalSocketDirectory"
+    , "address" .= localAddress
+    , "mode" .= String (pack (show mode))
+    ]
   toJSON (CreatingServerSocket sockAddr) = object
     [ "kind" .= String "CreatingServerSocket"
     , "address" .= sockAddr

ouroboros-network/ouroboros-network.cabal

@@ -329,7 +329,8 @@ library
 
   if !os(windows)
     build-depends:
-      directory
+      directory,
+      unix,
 
 library framework
   import: ghc-options
@@ -400,6 +401,9 @@ library framework
 
   if os(windows)
     build-depends: Win32 >=2.5.4.1 && <3.0
+
+  if !os(windows)
+    build-depends: unix
   hs-source-dirs: framework
 
 library tests-lib

ouroboros-network/tests/lib/Test/Ouroboros/Network/Diffusion/Node.hs

@@ -273,12 +273,14 @@ run blockGeneratorArgs ni na
               , Diffusion.diWithBuffer            = \f -> f Nothing
               , Diffusion.diNtnConfigureSocket    = \_ _ -> return ()
               , Diffusion.diNtnConfigureSystemdSocket
-                                               = \_ _ -> return ()
+                                                  = \_ _ -> return ()
               , Diffusion.diNtnAddressType = ntnAddressType
               , Diffusion.diNtnToPeerAddr         = \a b -> TestAddress (Node.IPAddr a b)
               , Diffusion.diNtcSnocket            = iNtcSnocket ni
               , Diffusion.diNtcBearer             = iNtcBearer ni
               , Diffusion.diNtcGetFileDescriptor  = \_ -> pure invalidFileDescriptor
+              , Diffusion.diNtcConfigureSocketFileDescriptor = \_ -> pure ()
+              , Diffusion.diNtcConfigureSocketFile           = \_ -> pure ()
               , Diffusion.diRng                   = diffStgGen
               , Diffusion.diDnsActions            = \tracer lookupType toPeerAddr ->
                   mockDNSActions