Peers behind a firewall

cardano-diffusion/changelog.d/20251108_103013_edgr_behind_firewall.md

@@ -0,0 +1,10 @@
+<!--
+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.
+-->
+
+### Non-Breaking
+
+- Added a property test to verify that the node never connects to peers behind a firewall.

cardano-diffusion/lib/Cardano/Network/Diffusion/Topology.hs

@@ -30,5 +30,6 @@ isValidTrustedPeerConfiguration
               IsTrustable    -> not
                               . null
                               . rootAccessPoints
+                              . rootConfig
                               $ localRoots
           ) lprgs

cardano-diffusion/tests/lib/Test/Cardano/Network/PeerSelection.hs

@@ -59,6 +59,7 @@ import Cardano.Network.PeerSelection
 -- import Cardano.Network.PeerSelection.PublicRootPeers (PublicRootPeers (..))
 import Cardano.Network.PeerSelection.PublicRootPeers qualified as Cardano.PublicRootPeers
 
+import Ouroboros.Network.ConnectionManager.Types (Provenance(Outbound))
 import Ouroboros.Network.DiffusionMode
 import Ouroboros.Network.ExitPolicy (RepromoteDelay (..))
 import Ouroboros.Network.PeerSelection
@@ -3206,7 +3207,6 @@ prop_governor_target_established_above (MaxTime maxTime) env =
                      <*> govInProgressIneligibleSig
                      <*> demotionOpportunitiesIgnoredTooLong)
 
-
 -- | Like 'prop_governor_target_established_above' but for big ledger peers.
 --
 prop_governor_target_established_big_ledger_peers_above
@@ -4431,8 +4431,8 @@ prop_issue_3550 = prop_governor_target_established_below defaultMaxTime $
           (PeerAddr 29,[],GovernorScripts {peerShareScript = Script (Nothing :| []), peerSharingScript = Script (PeerSharingDisabled :| []), connectionScript = Script ((ToWarm,NoDelay) :| [(ToCold,NoDelay),(Noop,NoDelay)])})
         ],
       localRootPeers = LocalRootPeers.fromGroups
-        [ (1, 1, Map.fromList [(PeerAddr 16, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])
-        , (1, 1, Map.fromList [(PeerAddr 4, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])
+        [ (1, 1, Map.fromList [(PeerAddr 16, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])
+        , (1, 1, Map.fromList [(PeerAddr 4, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])
         ],
       publicRootPeers = Cardano.PublicRootPeers.fromPublicRootPeers
         (Map.fromList [ (PeerAddr 14, DoNotAdvertisePeer)
@@ -4479,7 +4479,7 @@ prop_issue_3515 = prop_governor_nolivelock $
                            peerSharingScript = Script (PeerSharingDisabled :| []),
                            connectionScript = Script ((ToCold,NoDelay) :| [(Noop,NoDelay)])
                          })],
-      localRootPeers = LocalRootPeers.fromGroups [(1,1,Map.fromList [(PeerAddr 10, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])],
+      localRootPeers = LocalRootPeers.fromGroups [(1,1,Map.fromList [(PeerAddr 10, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])],
       publicRootPeers = PublicRootPeers.empty Cardano.ExtraPeers.empty,
       targets = Script . NonEmpty.fromList $ targets'',
       pickKnownPeersForPeerShare = Script (PickFirst :| []),
@@ -4521,7 +4521,7 @@ prop_issue_3494 = prop_governor_nofail $
                                                 peerSharingScript = Script (PeerSharingDisabled :| []),
                                                 connectionScript = Script ((ToCold,NoDelay) :| [(Noop,NoDelay)])
                                               })],
-      localRootPeers = LocalRootPeers.fromGroups [(1,1,Map.fromList [(PeerAddr 64, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])],
+      localRootPeers = LocalRootPeers.fromGroups [(1,1,Map.fromList [(PeerAddr 64, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])],
       publicRootPeers = PublicRootPeers.empty Cardano.ExtraPeers.empty,
       targets = Script . NonEmpty.fromList $ targets'',
       pickKnownPeersForPeerShare = Script (PickFirst :| []),
@@ -4571,8 +4571,8 @@ prop_issue_3233 = prop_governor_nolivelock $
          (PeerAddr 15,[],GovernorScripts {peerShareScript = Script (Just ([],PeerShareTimeSlow) :| []), peerSharingScript = Script (PeerSharingDisabled :| []), connectionScript = Script ((Noop,NoDelay) :| [])})
         ],
       localRootPeers = LocalRootPeers.fromGroups
-        [ (1, 1, Map.fromList [(PeerAddr 15, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])
-        , (1, 1, Map.fromList [(PeerAddr 13, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode IsNotTrustable)])
+        [ (1, 1, Map.fromList [(PeerAddr 15, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])
+        , (1, 1, Map.fromList [(PeerAddr 13, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode Outbound IsNotTrustable)])
         ],
       publicRootPeers = Cardano.PublicRootPeers.fromPublicRootPeers
         (Map.fromList [(PeerAddr 4, DoNotAdvertisePeer)]),

cardano-diffusion/tests/lib/Test/Cardano/Network/PeerSelection/MockEnvironment.hs

@@ -62,6 +62,7 @@ import Control.Monad.IOSim
 import Control.Tracer (Tracer (..), contramap, traceWith)
 
 import Ouroboros.Network.BlockFetch (FetchMode (..), PraosFetchMode (..))
+import Ouroboros.Network.ConnectionManager.Types (Provenance)
 import Ouroboros.Network.DiffusionMode
 import Ouroboros.Network.ExitPolicy
 import Ouroboros.Network.PeerSelection hiding (requestPublicRootPeers)
@@ -550,8 +551,8 @@ mockPeerSelectionActions' tracer
           traceWith tracer (TraceEnvPeerShareResult addr peeraddrs)
           return (PeerSharingResult peeraddrs)
 
-    establishPeerConnection :: IsBigLedgerPeer -> DiffusionMode -> PeerAddr -> m (PeerConn m)
-    establishPeerConnection _ _ peeraddr = do
+    establishPeerConnection :: IsBigLedgerPeer -> DiffusionMode -> Provenance -> PeerAddr -> m (PeerConn m)
+    establishPeerConnection _ _ _ peeraddr = do
       --TODO: add support for variable delays and synchronous failure
       traceWith tracer (TraceEnvEstablishConn peeraddr)
       threadDelay 1

ouroboros-network/changelog.d/20251108_101451_edgr_behind_firewall.md

@@ -0,0 +1,21 @@
+<!--
+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
+
+- Changed the type of `localRoots` to `LocalRoots`.
+- Modified `AcquireOutboundConnection` to include an additional parameter: `Provenance`.
+- `acquireOutboundConnectionImpl` only creates a new connection if `Provenance` permits it.
+- `jobPromoteColdPeer` only creates a new connection if no inbound connection is found and provenance is set to `Outbound`.
+
+### Non-Breaking
+
+- Added `LocalRoots` type in `Ouroboros.Network.PeerSelection.State.LocalRootPeers` with the following fields:
+  - `rootConfig` of type `RootConfig`
+  - `provenance` of type `Provenance`
+- Added `localProvenance` field to `LocalRootConfig`.
+- Added a new constructor `InboundConnectionNotFound` for `ConnectionManagerError`.

ouroboros-network/demo/connection-manager.hs

@@ -569,9 +569,9 @@ bidirectionalExperiment
                             (HandlerError
                               UnversionedProtocol))
     connect n cm | n <= 1 =
-      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr
+      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr Outbound
     connect n cm =
-      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr
+      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr Outbound
         `catch` \(_ :: IOException) -> threadDelay 1
                                     >> connect (pred n) cm
         `catch` \(_ :: Mux.Error)   -> threadDelay 1

ouroboros-network/framework/lib/Ouroboros/Network/ConnectionManager/Core.hs

@@ -1338,12 +1338,12 @@ with args@Arguments {
         -> ConnectionHandlerFn handlerTrace socket peerAddr handle handleError version versionData m
         -> DiffusionMode
         -> peerAddr
+        -> Provenance
         -> m (Connected peerAddr handle handleError)
-    acquireOutboundConnectionImpl stateVar stdGenVar handler diffusionMode peerAddr = do
+    acquireOutboundConnectionImpl stateVar stdGenVar handler diffusionMode peerAddr connProv = do
         let provenance = Outbound
         traceWith tracer (TrIncludeConnection provenance peerAddr)
-        (trace, mutableConnState@MutableConnState { connVar, connStateId }
-              , eHandleWedge) <- atomically $ do
+        (trace, eHandleWedge) <- atomically $ do
           state <- readTMVar stateVar
           stdGen <- stateTVar stdGenVar Random.split
           case State.lookupByRemoteAddr stdGen peerAddr state of
@@ -1353,14 +1353,12 @@ with args@Arguments {
               case connState of
                 ReservedOutboundState ->
                   return ( Just (Right (TrConnectionExists provenance peerAddr st))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ConnectionExists provenance peerAddr))
                          )
 
                 UnnegotiatedState Outbound _connId _connThread -> do
                   return ( Just (Right (TrConnectionExists provenance peerAddr st))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ConnectionExists provenance peerAddr))
                          )
@@ -1370,34 +1368,29 @@ with args@Arguments {
                   -- return 'There' to indicate that we need to block on
                   -- the connection state.
                   return ( Nothing
-                         , mutableConnState
-                         , Right (There connId)
+                         , Right (mutableConnState, There connId)
                          )
 
                 OutboundUniState {} -> do
                   return ( Just (Right (TrConnectionExists provenance peerAddr st))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ConnectionExists provenance peerAddr))
                          )
 
                 OutboundDupState {} -> do
                   return ( Just (Right (TrConnectionExists provenance peerAddr st))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ConnectionExists provenance peerAddr))
                          )
 
                 OutboundIdleState _connId _connThread _handle _dataFlow ->
                   let tr = State.abstractState (Known connState) in
                   return ( Just (Right (TrForbiddenOperation peerAddr tr))
-                         , mutableConnState
                          , Left (withCallStack (ForbiddenOperation peerAddr tr))
                          )
 
                 InboundIdleState connId _connThread _handle Unidirectional -> do
                   return ( Just (Right (TrForbiddenConnection connId))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ForbiddenConnection connId))
                          )
@@ -1412,15 +1405,13 @@ with args@Arguments {
                   return ( Just (Left (TransitionTrace
                                          connStateId
                                          (mkTransition connState connState')))
-                         , mutableConnState
-                         , Right (Here (Connected connId dataFlow handle))
+                         , Right (mutableConnState, Here (Connected connId dataFlow handle))
                          )
 
                 InboundState connId _connThread _handle Unidirectional -> do
                   -- the remote side negotiated unidirectional connection, we
                   -- cannot re-use it.
                   return ( Just (Right (TrForbiddenConnection connId))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ForbiddenConnection connId))
                          )
@@ -1435,13 +1426,11 @@ with args@Arguments {
                   return ( Just (Left (TransitionTrace
                                         connStateId
                                         (mkTransition connState connState')))
-                         , mutableConnState
-                         , Right (Here (Connected connId dataFlow handle))
+                         , Right (mutableConnState, Here (Connected connId dataFlow handle))
                          )
 
                 DuplexState _connId _connThread  _handle ->
                   return ( Just (Right (TrConnectionExists provenance peerAddr st))
-                         , mutableConnState
                          , Left (withCallStack
                                   (ConnectionExists provenance peerAddr))
                          )
@@ -1458,25 +1447,33 @@ with args@Arguments {
                   retry
 
             Nothing -> do
-              let connState' = ReservedOutboundState
-              (mutableConnState@MutableConnState { connVar, connStateId }
-                :: MutableConnState peerAddr handle handleError
-                                    version m)
-                <- State.newMutableConnState peerAddr connStateIdSupply connState'
-              -- TODO: label `connVar` using 'ConnectionId'
-              labelTVar connVar ("conn-state-" ++ show peerAddr)
-
-              writeTMVar stateVar
-                        (State.insertUnknownLocalAddr peerAddr mutableConnState state)
-              return ( Just (Left (TransitionTrace
-                                    connStateId
-                                    Transition {
-                                        fromState = Unknown,
-                                        toState   = Known connState'
-                                      }))
-                     , mutableConnState
-                     , Right Nowhere
-                     )
+              -- Only proceed if creating a new connection is allowed
+              if connProv == Inbound
+              then do
+                return ( Just (Right (TrInboundConnectionNotFound connProv peerAddr))
+                       , Left (withCallStack
+                                (InboundConnectionNotFound connProv peerAddr))
+                       )
+              else do
+                let connState' = ReservedOutboundState
+                (mutableConnState@MutableConnState { connVar, connStateId }
+                  :: MutableConnState peerAddr handle handleError
+                                      version m)
+                  <- State.newMutableConnState peerAddr connStateIdSupply connState'
+
+                -- TODO: label `connVar` using 'ConnectionId'
+                labelTVar connVar ("conn-state-" ++ show peerAddr)
+
+                writeTMVar stateVar
+                          (State.insertUnknownLocalAddr peerAddr mutableConnState state)
+                return ( Just (Left (TransitionTrace
+                                      connStateId
+                                      Transition {
+                                          fromState = Unknown,
+                                          toState   = Known connState'
+                                        }))
+                       , Right (mutableConnState, Nowhere)
+                       )
 
         traverse_ (either (traceWith trTracer) (traceWith tracer)) trace
         traceCounters stateVar
@@ -1485,7 +1482,7 @@ with args@Arguments {
             throwIO e
 
           -- connection manager does not have a connection with @peerAddr@.
-          Right Nowhere -> do
+          Right (mutableConnState@MutableConnState { connVar, connStateId }, Nowhere) -> do
             (reader, writer) <- newEmptyPromiseIO
 
             (connId, connThread) <-
@@ -1719,7 +1716,7 @@ with args@Arguments {
                   Right _  -> Connected    connId dataFlow handle
                   Left err -> Disconnected connId err
 
-          Right (There connId) -> do
+          Right (MutableConnState { connVar, connStateId }, There connId) -> do
             -- We can only enter the 'There' case if there is an inbound
             -- connection, and we are about to reuse it, but we need to wait
             -- for handshake.
@@ -1811,7 +1808,7 @@ with args@Arguments {
             return connected
 
           -- Connection manager has a connection which can be reused.
-          Right (Here connected) -> do
+          Right (_, Here connected) -> do
             traceCounters stateVar
             return connected
 
@@ -2468,6 +2465,7 @@ withCallStack k = k callStack
 --
 data Trace peerAddr handlerTrace
   = TrIncludeConnection            Provenance peerAddr
+  | TrInboundConnectionNotFound    Provenance peerAddr
   | TrReleaseConnection            Provenance (ConnectionId peerAddr)
   | TrConnect                      (Maybe peerAddr) -- ^ local address
                                    peerAddr         -- ^ remote address

ouroboros-network/framework/lib/Ouroboros/Network/ConnectionManager/Types.hs

@@ -497,9 +497,13 @@ data Connected peerAddr handle handleError =
     --
   | Disconnected !(ConnectionId peerAddr) !(DisconnectionException handleError)
 
-
+-- | Acquire outbound connection
+-- 'Inbound' provenance restricts acquiring an outbound connection to those
+-- initiated by a remote peer.
+-- 'Outbound' provenance doesn't require any additional pre-conditions.
 type AcquireOutboundConnection peerAddr handle handleError m
-    = DiffusionMode -> peerAddr -> m (Connected peerAddr handle handleError)
+    = DiffusionMode -> peerAddr -> Provenance -> m (Connected peerAddr handle handleError)
+
 type IncludeInboundConnection socket peerAddr handle handleError m
     = Word32
     -- ^ inbound connections hard limit.
@@ -723,6 +727,11 @@ data ConnectionManagerError peerAddr
     --
     | ForbiddenConnection   !(ConnectionId peerAddr) !CallStack
 
+    -- | No matching inbound connection found while establishing a new connection is
+    -- not allowed.
+    --
+    | InboundConnectionNotFound !Provenance !peerAddr !CallStack
+
     -- | Connections that would be forbidden by the kernel (@TCP@ semantics).
     --
     | ImpossibleConnection  !(ConnectionId peerAddr) !CallStack
@@ -774,6 +783,14 @@ instance ( Show peerAddr
              , "\n"
              , prettyCallStack cs
              ]
+    displayException (InboundConnectionNotFound connMode peerAddr cs) =
+      concat [ "No matching inbound connection found and creating new connection is not allowed with peer "
+             , show connMode
+             , "\n"
+             , show peerAddr
+             , "\n"
+             , prettyCallStack cs
+             ]
     displayException (ConnectionTerminating connId cs) =
       concat [ "Connection terminating "
              , show connId