git.iko.soy/distributed-process
on master help sign in
Deal with invalid close requests
ecd82fcEdsko de Vries
network-transport/src/Network/Transport/TCP.hs+10−4
@@ -101,6 +101,7 @@ import qualified Data.IntSet as IntSet ( empty
101 , singleton
101 , singleton
102 , null
102 , null
103 , delete
103 , delete
104+ , member
104 )
105 )
105import Data.Map (Map)
106import Data.Map (Map)
106import qualified Data.Map as Map (empty)
107import qualified Data.Map as Map (empty)
@@ -1333,15 +1334,18 @@ handleIncomingMessages (ourEndPoint, theirEndPoint) = do
1333 putMVar mvar response
1334 putMVar mvar response
1334
1335
1335 -- Close a connection
1336 -- Close a connection
1337+ -- It is important that we verify that the connection is in fact open,
1338+ -- because otherwise we should not decrement the reference count
1336 closeConnection :: ConnectionId -> IO ()
1339 closeConnection :: ConnectionId -> IO ()
1337 closeConnection cid = do
1340 closeConnection cid = do
1338− -- TODO: we should check that this connection is in fact open
1339− writeChan ourChannel (ConnectionClosed cid)
1340− modifyMVar_ theirState $ \(RemoteEndPointValid vst) ->
1341+ modifyMVar_ theirState $ \(RemoteEndPointValid vst) -> do
1342+ unless (IntSet.member cid (vst ^. remoteIncoming)) $
1343+ throwIO $ userError "Invalid CloseConnection"
1341 return ( RemoteEndPointValid
1344 return ( RemoteEndPointValid
1342 . (remoteIncoming ^: IntSet.delete cid)
1345 . (remoteIncoming ^: IntSet.delete cid)
1343 $ vst
1346 $ vst
1344 )
1347 )
1348+ writeChan ourChannel (ConnectionClosed cid)
1345 closeIfUnused (ourEndPoint, theirEndPoint)
1349 closeIfUnused (ourEndPoint, theirEndPoint)
1346
1350
1347 -- Close the socket (if we don't have any outgoing connections)
1351 -- Close the socket (if we don't have any outgoing connections)
@@ -1382,7 +1386,9 @@ handleIncomingMessages (ourEndPoint, theirEndPoint) = do
1382 putMVar resolved ()
1386 putMVar resolved ()
1383 return True
1387 return True
1384
1388
1385− -- Read a message and output it on the endPoint's channel
1389+ -- Read a message and output it on the endPoint's channel By rights we
1390+ -- should verify that the connection ID is valid, but this is unnecessary
1391+ -- overhead
1386 readMessage :: N.Socket -> ConnectionId -> IO ()
1392 readMessage :: N.Socket -> ConnectionId -> IO ()
1387 readMessage sock connId =
1393 readMessage sock connId =
1388 recvWithLength sock >>= writeChan ourChannel . Received connId
1394 recvWithLength sock >>= writeChan ourChannel . Received connId
network-transport/tests/TestTCP.hs+38
@@ -691,6 +691,43 @@ testUnidirectionalError nextPort = do
691
691
692 takeMVar clientDone
692 takeMVar clientDone
693
693
694+testInvalidCloseConnection :: IO N.ServiceName -> IO ()
695+testInvalidCloseConnection nextPort = do
696+ Right (transport, internals) <- nextPort >>= createTransportExposeInternals "127.0.0.1"
697+ serverAddr <- newEmptyMVar
698+ clientDone <- newEmptyMVar
699+ serverDone <- newEmptyMVar
700+
701+ -- Server
702+ forkTry $ do
703+ Right endpoint <- newEndPoint transport
704+ putMVar serverAddr (address endpoint)
705+
706+ ConnectionOpened _ _ _ <- receive endpoint
707+
708+ -- At this point the client sends an invalid request, so we terminate the
709+ -- connection
710+ ErrorEvent (TransportError (EventConnectionLost _ [_]) _) <- receive endpoint
711+
712+ putMVar serverDone ()
713+
714+ -- Client
715+ forkTry $ do
716+ Right endpoint <- newEndPoint transport
717+ let ourAddr = address endpoint
718+
719+ -- Connect so that we have a TCP connection
720+ theirAddr <- readMVar serverAddr
721+ Right _ <- connect endpoint theirAddr ReliableOrdered defaultConnectHints
722+
723+ -- Get a handle on the TCP connection and manually send an invalid CloseConnection request
724+ Right sock <- socketBetween internals ourAddr theirAddr
725+ sendMany sock [encodeInt32 CloseConnection, encodeInt32 (12345 :: Int)]
726+
727+ putMVar clientDone ()
728+
729+ mapM_ takeMVar [clientDone, serverDone]
730+
694main :: IO ()
731main :: IO ()
695main = do
732main = do
696 portMVar <- newEmptyMVar
733 portMVar <- newEmptyMVar
@@ -708,5 +745,6 @@ main = do
708 , ("BreakTransport", testBreakTransport nextPort)
745 , ("BreakTransport", testBreakTransport nextPort)
709 , ("Reconnect", testReconnect nextPort)
746 , ("Reconnect", testReconnect nextPort)
710 , ("UnidirectionalError", testUnidirectionalError nextPort)
747 , ("UnidirectionalError", testUnidirectionalError nextPort)
748+ , ("InvalidCloseConnection", testInvalidCloseConnection nextPort)
711 ]
749 ]
712 testTransport (either (Left . show) (Right) <$> nextPort >>= createTransport "127.0.0.1")
750 testTransport (either (Left . show) (Right) <$> nextPort >>= createTransport "127.0.0.1")