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

@@ -3206,7 +3206,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 +4430,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 False IsNotTrustable)])
+        , (1, 1, Map.fromList [(PeerAddr 4, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode False IsNotTrustable)])
         ],
       publicRootPeers = Cardano.PublicRootPeers.fromPublicRootPeers
         (Map.fromList [ (PeerAddr 14, DoNotAdvertisePeer)
@@ -4479,7 +4478,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 False IsNotTrustable)])],
       publicRootPeers = PublicRootPeers.empty Cardano.ExtraPeers.empty,
       targets = Script . NonEmpty.fromList $ targets'',
       pickKnownPeersForPeerShare = Script (PickFirst :| []),
@@ -4521,7 +4520,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 False IsNotTrustable)])],
       publicRootPeers = PublicRootPeers.empty Cardano.ExtraPeers.empty,
       targets = Script . NonEmpty.fromList $ targets'',
       pickKnownPeersForPeerShare = Script (PickFirst :| []),
@@ -4571,8 +4570,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 False IsNotTrustable)])
+        , (1, 1, Map.fromList [(PeerAddr 13, LocalRootConfig DoAdvertisePeer InitiatorAndResponderDiffusionMode False 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 (ConnectionMode (..))
 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 -> PeerAddr -> ConnectionMode -> 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,22 @@
+<!--
+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: `ConnectionMode`.
+- `acquireOutboundConnectionImpl` only creates a new connection if the `ConnectionMode` function permits it.
+- `jobPromoteColdPeer` only creates a new connection if no inbound connection is found and the peer is not behind a firewall.
+
+### Non-Breaking
+
+- Added `LocalRoots` type in `Ouroboros.Network.PeerSelection.State.LocalRootPeers` with the following fields:
+  - `rootConfig` of type `RootConfig`
+  - `behindFirewall` of type `Bool`
+- Added `localRootBehindFirewall` field to `LocalRootConfig`.
+- Added a new sum type: `ConnectionMode`.
+- 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 CreateNewIfNoInbound
     connect n cm =
-      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr
+      acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr CreateNewIfNoInbound
         `catch` \(_ :: IOException) -> threadDelay 1
                                     >> connect (pred n) cm
         `catch` \(_ :: Mux.Error)   -> threadDelay 1

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

@@ -1338,8 +1338,9 @@ with args@Arguments {
         -> ConnectionHandlerFn handlerTrace socket peerAddr handle handleError version versionData m
         -> DiffusionMode
         -> peerAddr
+        -> ConnectionMode
         -> m (Connected peerAddr handle handleError)
-    acquireOutboundConnectionImpl stateVar stdGenVar handler diffusionMode peerAddr = do
+    acquireOutboundConnectionImpl stateVar stdGenVar handler diffusionMode peerAddr connectionMode = do
         let provenance = Outbound
         traceWith tracer (TrIncludeConnection provenance peerAddr)
         (trace, mutableConnState@MutableConnState { connVar, connStateId }
@@ -1463,20 +1464,30 @@ with args@Arguments {
                 :: 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 inboundRequired connectionMode
+              then do
+                return ( Just (Right (TrInboundConnectionNotFound connectionMode peerAddr))
+                   , mutableConnState
+                   , Left (withCallStack
+                            (InboundConnectionNotFound connectionMode peerAddr))
+                   )
+              else do
+                -- 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
+                       )
 
         traverse_ (either (traceWith trTracer) (traceWith tracer)) trace
         traceCounters stateVar
@@ -2468,6 +2479,7 @@ withCallStack k = k callStack
 --
 data Trace peerAddr handlerTrace
   = TrIncludeConnection            Provenance peerAddr
+  | TrInboundConnectionNotFound    ConnectionMode peerAddr
   | TrReleaseConnection            Provenance (ConnectionId peerAddr)
   | TrConnect                      (Maybe peerAddr) -- ^ local address
                                    peerAddr         -- ^ remote address

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

@@ -114,6 +114,8 @@ module Ouroboros.Network.ConnectionManager.Types
   , resultInState
   , DemotedToColdRemoteTr (..)
   , AcquireOutboundConnection
+  , ConnectionMode (..)
+  , inboundRequired
   , IncludeInboundConnection
     -- *** Outbound side
   , acquireOutboundConnection
@@ -497,9 +499,24 @@ data Connected peerAddr handle handleError =
     --
   | Disconnected !(ConnectionId peerAddr) !(DisconnectionException handleError)
 
+-- | Describes the behavior for handling connections when no inbound connection
+-- is found.
+--  - 'CreateNewIfNoInbound': If no inbound connection exists, create a new
+-- conection.
+--  - 'RequireInbound': Strictly require an inbound connection; fail if none
+-- exists.
+data ConnectionMode
+    = CreateNewIfNoInbound
+    | RequireInbound
+    deriving Show
+
+inboundRequired :: ConnectionMode -> Bool
+inboundRequired RequireInbound = True
+inboundRequired _other         = False
 
 type AcquireOutboundConnection peerAddr handle handleError m
-    = DiffusionMode -> peerAddr -> m (Connected peerAddr handle handleError)
+    = DiffusionMode -> peerAddr -> ConnectionMode -> m (Connected peerAddr handle handleError)
+
 type IncludeInboundConnection socket peerAddr handle handleError m
     = Word32
     -- ^ inbound connections hard limit.
@@ -723,6 +740,11 @@ data ConnectionManagerError peerAddr
     --
     | ForbiddenConnection   !(ConnectionId peerAddr) !CallStack
 
+    -- | No matching inbound connection found and creating new connection is
+    -- not allowed.
+    --
+    | InboundConnectionNotFound !ConnectionMode !peerAddr !CallStack
+
     -- | Connections that would be forbidden by the kernel (@TCP@ semantics).
     --
     | ImpossibleConnection  !(ConnectionId peerAddr) !CallStack
@@ -774,6 +796,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

ouroboros-network/framework/sim-tests/Test/Ouroboros/Network/ConnectionManager.hs

@@ -817,7 +817,7 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
                                 -- handshake negotiation.
                                 timeout (1 + 5 + testTimeWaitTimeout)
                                   (acquireOutboundConnection
-                                    connectionManager InitiatorAndResponderDiffusionMode addr))
+                                    connectionManager InitiatorAndResponderDiffusionMode addr CreateNewIfNoInbound))
                             `catches`
                               [ Handler $ \(e :: IOException) -> return (Left (toException e))
                               , Handler $ \(e :: SomeConnectionManagerError) ->
@@ -983,10 +983,11 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
         Just (SomeConnectionManagerError e@UnknownPeer {})          -> throwIO e
 
         -- If 'ForbiddenConnection' is thrown we let the test continue.
-        Just (SomeConnectionManagerError ForbiddenConnection {})    -> pure ()
-        Just (SomeConnectionManagerError ConnectionExists {})       -> pure ()
-        Just (SomeConnectionManagerError ConnectionTerminating {})  -> pure ()
-        Just (SomeConnectionManagerError ConnectionTerminated {})   -> pure ()
+        Just (SomeConnectionManagerError ForbiddenConnection {})       -> pure ()
+        Just (SomeConnectionManagerError ConnectionExists {})          -> pure ()
+        Just (SomeConnectionManagerError ConnectionTerminating {})     -> pure ()
+        Just (SomeConnectionManagerError ConnectionTerminated {})      -> pure ()
+        Just (SomeConnectionManagerError InboundConnectionNotFound {}) -> pure ()
 
 
 -- | This includes the @Overwritten@ transition.

ouroboros-network/framework/sim-tests/Test/Ouroboros/Network/Server/Sim.hs

@@ -879,7 +879,7 @@ multinodeExperiment inboundTrTracer trTracer inboundTracer debugTracer cmTracer
                                        case fromException e of
                                          Just SomeAsyncException {} -> Nothing
                                          _                          -> Just e)
-                          $ acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr
+                          $ acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr CreateNewIfNoInbound
             case connHandle of
               Left _ ->
                 go connMap

ouroboros-network/framework/tests-lib/Test/Ouroboros/Network/ConnectionManager/Experiments.hs

@@ -770,7 +770,7 @@ unidirectionalExperiment stdGen timeouts snocket makeBearer confSock socket clie
                 replicateM
                   (numberOfRounds clientAndServerData)
                   (bracket
-                     (acquireOutboundConnection connectionManager InitiatorOnlyDiffusionMode serverAddr)
+                     (acquireOutboundConnection connectionManager InitiatorOnlyDiffusionMode serverAddr CreateNewIfNoInbound)
                      (\case
                         Connected connId _ _ -> releaseOutboundConnection connectionManager connId
                         Disconnected {} -> error "unidirectionalExperiment: impossible happened")
@@ -876,7 +876,8 @@ bidirectionalExperiment
                       (acquireOutboundConnection
                         connectionManager0
                         InitiatorAndResponderDiffusionMode
-                        localAddr1))
+                        localAddr1
+                        CreateNewIfNoInbound))
                     (\case
                       Connected connId _ _ ->
                         releaseOutboundConnection
@@ -902,7 +903,8 @@ bidirectionalExperiment
                       (acquireOutboundConnection
                         connectionManager1
                         InitiatorAndResponderDiffusionMode
-                        localAddr0))
+                        localAddr0
+                        CreateNewIfNoInbound))
                     (\case
                       Connected connId _ _ ->
                         releaseOutboundConnection

ouroboros-network/framework/tests-lib/Test/Ouroboros/Network/ConnectionManager/Utils.hs

@@ -300,43 +300,45 @@ connectionManagerTraceMap
       ntnAddr
       (ConnectionHandlerTrace ntnVersion ntnVersionData)
   -> String
-connectionManagerTraceMap (TrIncludeConnection p _)        =
+connectionManagerTraceMap (TrInboundConnectionNotFound cm _) =
+  "TrInboundConnectionNotFound " ++ show cm
+connectionManagerTraceMap (TrIncludeConnection p _)          =
   "TrIncludeConnection " ++ show p
-connectionManagerTraceMap (TrReleaseConnection p _)        =
+connectionManagerTraceMap (TrReleaseConnection p _)          =
   "TrUnregisterConnection " ++ show p
-connectionManagerTraceMap TrConnect {}                     =
+connectionManagerTraceMap TrConnect {}                       =
   "TrConnect"
-connectionManagerTraceMap (TrConnectError _ _ _)           =
+connectionManagerTraceMap (TrConnectError _ _ _)             =
   "TrConnectError"
-connectionManagerTraceMap (TrTerminatingConnection p _)    =
+connectionManagerTraceMap (TrTerminatingConnection p _)      =
   "TrTerminatingConnection " ++ show p
-connectionManagerTraceMap (TrTerminatedConnection p _)     =
+connectionManagerTraceMap (TrTerminatedConnection p _)       =
   "TrTerminatedConnection " ++ show p
-connectionManagerTraceMap (TrConnectionHandler _ _)        =
+connectionManagerTraceMap (TrConnectionHandler _ _)          =
   "TrConnectionHandler"
-connectionManagerTraceMap TrShutdown                       =
+connectionManagerTraceMap TrShutdown                         =
   "TrShutdown"
-connectionManagerTraceMap (TrConnectionExists p _ as)      =
+connectionManagerTraceMap (TrConnectionExists p _ as)        =
   "TrConnectionExists " ++ show p ++ " " ++ show as
-connectionManagerTraceMap (TrForbiddenConnection _)        =
+connectionManagerTraceMap (TrForbiddenConnection _)          =
   "TrForbiddenConnection"
-connectionManagerTraceMap (TrConnectionFailure _)          =
+connectionManagerTraceMap (TrConnectionFailure _)            =
   "TrConnectionFailure"
-connectionManagerTraceMap (TrConnectionNotFound p _)       =
+connectionManagerTraceMap (TrConnectionNotFound p _)         =
   "TrConnectionNotFound " ++ show p
-connectionManagerTraceMap (TrForbiddenOperation _ as)      =
+connectionManagerTraceMap (TrForbiddenOperation _ as)        =
   "TrForbiddenOperation" ++ show as
-connectionManagerTraceMap (TrPruneConnections _ _ _)       =
+connectionManagerTraceMap (TrPruneConnections _ _ _)         =
   "TrPruneConnections"
-connectionManagerTraceMap (TrConnectionCleanup _)          =
+connectionManagerTraceMap (TrConnectionCleanup _)            =
   "TrConnectionCleanup"
-connectionManagerTraceMap (TrConnectionTimeWait _)         =
+connectionManagerTraceMap (TrConnectionTimeWait _)           =
   "TrConnectionTimeWait"
-connectionManagerTraceMap (TrConnectionTimeWaitDone _)     =
+connectionManagerTraceMap (TrConnectionTimeWaitDone _)       =
   "TrConnectionTimeWaitDone"
-connectionManagerTraceMap (TrConnectionManagerCounters _)  =
+connectionManagerTraceMap (TrConnectionManagerCounters _)    =
   "TrConnectionManagerCounters"
-connectionManagerTraceMap (TrState _)                      =
+connectionManagerTraceMap (TrState _)                        =
   "TrState"
-connectionManagerTraceMap (TrUnexpectedlyFalseAssertion _) =
+connectionManagerTraceMap (TrUnexpectedlyFalseAssertion _)   =
   "TrUnexpectedlyFalseAssertion"