WarpTLS cannot teminate TCP connections if TLS handshake is not done
kazu-yamamoto opened this issue · 1 comments
kazu-yamamoto commented
Warp can terminate TCP connections even if no bytes are sent from clients.
% gtelnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
Connection closed by foreign host.
Unfortunately, WarpTLS cannot:
% gtelnet 127.0.0.1 443
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
... (waiting forever)
The difference is their mkConn
functions. WarpTLS's one returns only after TLS handshake is done while Warp's one can immediately return.
Run.hs
has the following function:
fork set mkConn addr app counter ii = settingsFork set $ \unmask ->
-- Call the user-supplied on exception code if any
-- exceptions are thrown.
--
-- Intentionally using Control.Exception.handle, since we want to
-- catch all exceptions and avoid them from propagating, even
-- async exceptions. See:
-- https://github.com/yesodweb/wai/issues/850
Control.Exception.handle (settingsOnException set Nothing) $
-- Run the connection maker to get a new connection, and ensure
-- that the connection is closed. If the mkConn call throws an
-- exception, we will leak the connection. If the mkConn call is
-- vulnerable to attacks (e.g., Slowloris), we do nothing to
-- protect the server. It is therefore vital that mkConn is well
-- vetted.
--
-- We grab the connection before registering timeouts since the
-- timeouts will be useless during connection creation, due to the
-- fact that async exceptions are still masked.
UnliftIO.bracket mkConn cleanUp (serve unmask)
where
cleanUp (conn, _) =
connClose conn `UnliftIO.finally` do
writeBuffer <- readIORef $ connWriteBuffer conn
bufFree writeBuffer
-- We need to register a timeout handler for this thread, and
-- cancel that handler as soon as we exit.
serve unmask (conn, transport) = UnliftIO.bracket register cancel $ \th -> do
-- We now have fully registered a connection close handler in
-- the case of all exceptions, so it is safe to once again
-- allow async exceptions.
unmask
.
-- Call the user-supplied code for connection open and
-- close events
UnliftIO.bracket (onOpen addr) (onClose addr)
$ \goingon ->
-- Actually serve this connection. bracket with closeConn
-- above ensures the connection is closed.
when goingon $ serveConnection conn ii th addr transport set app
where
register = T.registerKillThread (timeoutManager ii) (connClose conn)
cancel = T.cancel
onOpen adr = increase counter >> settingsOnOpen set adr
onClose adr _ = decrease counter >> settingsOnClose set adr
The time handle is created after mkConn
returns.
As the comment says, asynchronous exceptions do not reach to the resource acquire handler of bracket
.
I guess that we should register timeout first then call mkConn
without bracket
.
kazu-yamamoto commented
Uhhm, registerKillThread
requires connClose conn
.