local socket: tighten access rights

Author: coot

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,7 @@ module Ouroboros.Network.Snocket
   , FileDescriptor
   , socketFileDescriptor
   , localSocketFileDescriptor
+  , configureLocalSocketFileDescriptor
     -- ** for testing
   , invalidFileDescriptor
     -- * Re-exports
@@ -56,12 +58,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 +74,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 +638,34 @@ localSocketFileDescriptor =
 localSocketFileDescriptor = socketFileDescriptor . getLocalHandle
 #endif
 
+-- | Restrict the local (Node-to-Client) socket so that only the owning user
+-- can connect.
+--
+-- On POSIX this sets mode @0600@ on the AF_UNIX socket.  Both @fchmod@ on the
+-- socket file descriptor and @chmod@ on the bound path are called: the
+-- former is the canonical Linux/modern-macOS path; the latter is portability
+-- insurance for systems where @fchmod@ on a socket fd does not propagate to
+-- the on-disk inode.
+--
+-- 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.bind' (which creates the socket file) and
+-- 'Snocket.listen' (which makes connections possible).
+configureLocalSocketFileDescriptor :: LocalSocket -> LocalAddress -> IO ()
+#if defined(mingw32_HOST_OS) || defined(wasm32_HOST_ARCH)
+configureLocalSocketFileDescriptor _ _ = return ()
+#else
+configureLocalSocketFileDescriptor sock (LocalAddress path) = do
+  FileDescriptor fd <- localSocketFileDescriptor sock
+  let mode = Unix.ownerReadMode .|. Unix.ownerWriteMode
+  Unix.setFdMode (Unix.Fd (fromIntegral @Int @CInt fd)) mode
+  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,7 +84,8 @@ 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,
+import Ouroboros.Network.Snocket (LocalAddress (..), LocalSocket (..),
+           RemoteAddress, configureLocalSocketFileDescriptor,
            localSocketFileDescriptor, makeLocalBearer, makeSocketBearer')
 import Ouroboros.Network.Snocket qualified as Snocket
 import Ouroboros.Network.Socket (configureSocket, configureSystemdSocket)
@@ -164,6 +172,7 @@ runM Interfaces
        , diNtcSnocket
        , diNtcBearer
        , diNtcGetFileDescriptor
+       , diNtcConfigureFileDescriptor
        , diRng
        , diDnsActions
        , diConnStateIdSupply
@@ -318,7 +327,7 @@ 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 diNtcConfigureFileDescriptor diNtcSnocket localAddr
       $ \localSocket -> do
         localInbInfoChannel <- newInformationChannel
 
@@ -912,7 +921,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 +930,6 @@ mkInterfaces :: IOManager
                                Resolver
                                IO)
 mkInterfaces iocp tracer egressPollInterval = do
-
   diRng <- newStdGen
   diConnStateIdSupply <- atomically $ CM.newConnStateIdSupply Proxy
 
@@ -941,11 +949,44 @@ mkInterfaces iocp tracer egressPollInterval = do
     diNtcSnocket           = Snocket.localSnocket iocp,
     diNtcBearer            = makeLocalBearer,
     diNtcGetFileDescriptor = localSocketFileDescriptor,
+    diNtcConfigureFileDescriptor = \sock addr -> do
+      configureLocalSocketFileDescriptor sock 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,9 @@ data Interfaces ntnFd ntnAddr ntcFd ntcAddr
         diNtcGetFileDescriptor
           :: ntcFd -> m FileDescriptor,
 
+        diNtcConfigureFileDescriptor
+          :: ntcFd -> 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,14 @@ withLocalSocket :: forall ntnAddr ntcFd ntcAddr m a.
                    )
                 => Tracer m (DiffusionTracer ntnAddr ntcAddr)
                 -> (ntcFd -> m FileDescriptor)
+                -> (ntcFd -> ntcAddr -> m ())
+                -- ^ configure the local socket file descriptor (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 configureFileDescriptor sn localAddress k =
   bracket
     (
       case localAddress of
@@ -117,6 +120,9 @@ withLocalSocket tracer getFileDescriptor sn localAddress k =
         traceWith tracer . ConfiguringLocalSocket addr
            =<< getFileDescriptor sd
         Snocket.bind sn sd addr
+        configureFileDescriptor sd 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

@@ -279,6 +279,7 @@ run blockGeneratorArgs ni na
               , Diffusion.diNtcSnocket            = iNtcSnocket ni
               , Diffusion.diNtcBearer             = iNtcBearer ni
               , Diffusion.diNtcGetFileDescriptor  = \_ -> pure invalidFileDescriptor
+              , Diffusion.diNtcConfigureFileDescriptor = \_ _ -> pure ()
               , Diffusion.diRng                   = diffStgGen
               , Diffusion.diDnsActions            = \tracer lookupType toPeerAddr ->
                   mockDNSActions

ouroboros-network/tracing/Ouroboros/Network/Tracing/PeerSelection.hs

@@ -54,6 +54,11 @@ instance (Show ntnAddr, Show ntcAddr) =>
     , "path" .= String (pack . show $ localAddress)
     , "socket" .= String (pack (show socket))
     ]
+  forMachine _dtal (Diff.ConfiguredLocalSocket localAddress socket) = mconcat
+    [ "kind" .= String "ConfiguredLocalSocket"
+    , "path" .= String (pack . show $ localAddress)
+    , "socket" .= String (pack (show socket))
+    ]
   forMachine _dtal (Diff.ListeningLocalSocket localAddress socket) = mconcat
     [ "kind" .= String "ListeningLocalSocket"
     , "path" .=  String (pack . show $ localAddress)
@@ -64,6 +69,11 @@ instance (Show ntnAddr, Show ntcAddr) =>
     , "path" .= String (pack . show $ localAddress)
     , "socket" .= String (pack (show fd))
     ]
+  forMachine _dtal (Diff.InsecureLocalSocketDirectory localAddress mode) = mconcat
+    [ "kind" .= String "InsecureLocalSocketDirectory"
+    , "path" .= String (pack . show $ localAddress)
+    , "mode" .= String (pack (show mode))
+    ]
   forMachine _dtal (Diff.CreatingServerSocket socket) = mconcat
     [ "kind" .= String "CreatingServerSocket"
     , "socket" .= String (pack (show socket))
@@ -109,10 +119,14 @@ instance MetaTrace (Diff.DiffusionTracer ntnAddr ntcAddr) where
       Namespace [] ["CreatedLocalSocket"]
     namespaceFor Diff.ConfiguringLocalSocket {} =
       Namespace [] ["ConfiguringLocalSocket"]
+    namespaceFor Diff.ConfiguredLocalSocket {} =
+      Namespace [] ["ConfiguredLocalSocket"]
     namespaceFor Diff.ListeningLocalSocket {} =
       Namespace [] ["ListeningLocalSocket"]
     namespaceFor Diff.LocalSocketUp {} =
       Namespace [] ["LocalSocketUp"]
+    namespaceFor Diff.InsecureLocalSocketDirectory {} =
+      Namespace [] ["InsecureLocalSocketDirectory"]
     namespaceFor Diff.CreatingServerSocket {} =
       Namespace [] ["CreatingServerSocket"]
     namespaceFor Diff.ListeningServerSocket {} =
@@ -136,8 +150,10 @@ instance MetaTrace (Diff.DiffusionTracer ntnAddr ntcAddr) where
     severityFor (Namespace _ ["CreateSystemdSocketForSnocketPath"]) _ = Just Info
     severityFor (Namespace _ ["CreatedLocalSocket"]) _ = Just Info
     severityFor (Namespace _ ["ConfiguringLocalSocket"]) _ = Just Info
+    severityFor (Namespace _ ["ConfiguredLocalSocket"]) _ = Just Info
     severityFor (Namespace _ ["ListeningLocalSocket"]) _ = Just Info
     severityFor (Namespace _ ["LocalSocketUp"]) _ = Just Info
+    severityFor (Namespace _ ["InsecureLocalSocketDirectory"]) _ = Just Warning
     severityFor (Namespace _ ["CreatingServerSocket"]) _ = Just Info
     severityFor (Namespace _ ["ListeningServerSocket"]) _ = Just Info
     severityFor (Namespace _ ["ServerSocketUp"]) _ = Just Info
@@ -160,10 +176,20 @@ instance MetaTrace (Diff.DiffusionTracer ntnAddr ntcAddr) where
       "CreatedLocalSocket"
     documentFor (Namespace _ ["ConfiguringLocalSocket"]) = Just
       "ConfiguringLocalSocket"
+    documentFor (Namespace _ ["ConfiguredLocalSocket"]) = Just $ mconcat
+      [ "ConfiguredLocalSocket: the local socket's permissions have been "
+      , "restricted (POSIX: mode 0600)."
+      ]
     documentFor (Namespace _ ["ListeningLocalSocket"]) = Just
       "ListeningLocalSocket"
     documentFor (Namespace _ ["LocalSocketUp"]) = Just
       "LocalSocketUp"
+    documentFor (Namespace _ ["InsecureLocalSocketDirectory"]) = Just $ mconcat
+      [ "InsecureLocalSocketDirectory: the parent directory of the local "
+      , "socket has group or other write permission, leaving the socket "
+      , "vulnerable to manipulation by another local user with write access "
+      , "to that directory."
+      ]
     documentFor (Namespace _ ["CreatingServerSocket"]) = Just
       "CreatingServerSocket"
     documentFor (Namespace _ ["ListeningServerSocket"]) = Just
@@ -189,8 +215,10 @@ instance MetaTrace (Diff.DiffusionTracer ntnAddr ntcAddr) where
       , Namespace [] ["CreateSystemdSocketForSnocketPath"]
       , Namespace [] ["CreatedLocalSocket"]
       , Namespace [] ["ConfiguringLocalSocket"]
+      , Namespace [] ["ConfiguredLocalSocket"]
       , Namespace [] ["ListeningLocalSocket"]
       , Namespace [] ["LocalSocketUp"]
+      , Namespace [] ["InsecureLocalSocketDirectory"]
       , Namespace [] ["CreatingServerSocket"]
       , Namespace [] ["ListeningServerSocket"]
       , Namespace [] ["ServerSocketUp"]