[sans-io] fix tcp half close behavior

This commit is contained in:
Maximilian Hils 2020-11-22 00:13:38 +01:00
parent 9f89c23a52
commit 3bffcf5e2f
8 changed files with 44 additions and 32 deletions

View File

@ -408,6 +408,13 @@ class HttpStream(layer.Layer):
yield SendHttp(ResponseData(self.stream_id, command.data), self.context.client)
elif isinstance(command, commands.CloseConnection) and command.connection == self.context.client:
yield SendHttp(ResponseProtocolError(self.stream_id, "EOF"), self.context.client)
elif isinstance(command, commands.CloseConnection):
# If we are running TCP over HTTP we want to be consistent with half-closes.
# The easiest approach for this is to just always full close for now.
# Alternatively, we could signal that we want a half close only through ResponseProtocolError,
# but that is more complex to implement.
command.half_close = False
yield command
else:
yield command
@ -468,13 +475,9 @@ class HttpLayer(layer.Layer):
stream = self.command_sources.pop(event.command)
yield from self.event_to_child(stream, event)
elif isinstance(event, events.ConnectionEvent):
if isinstance(event, events.ConnectionClosed):
# for practical purposes, we assume that a peer which sent at least a FIN
# is not interested in any more data from us, see
# see https://github.com/httpwg/http-core/issues/22
yield commands.CloseConnection(event.connection)
if event.connection == self.context.server and self.context.server not in self.connections:
pass
# We didn't do anything with this connection yet, now the peer has closed it - let's close it too!
yield commands.CloseConnection(event.connection)
else:
handler = self.connections[event.connection]
yield from self.event_to_child(handler, event)

View File

@ -97,7 +97,10 @@ class Http1Connection(HttpConnection, metaclass=abc.ABCMeta):
if isinstance(event, events.DataReceived):
return
elif isinstance(event, events.ConnectionClosed):
return
# for practical purposes, we assume that a peer which sent at least a FIN
# is not interested in any more data from us, see
# see https://github.com/httpwg/http-core/issues/22
yield commands.CloseConnection(event.connection)
else: # pragma: no cover
yield from ()
raise AssertionError(f"Unexpected event: {event}")
@ -203,9 +206,9 @@ class Http1Server(Http1Connection):
else:
pass # FIXME: protect against header size DoS
elif isinstance(event, events.ConnectionClosed):
if bytes(self.buf).strip():
yield commands.Log(f"Client closed connection before sending request headers: {bytes(self.buf)}")
yield commands.Log(f"Receive Buffer: {bytes(self.buf)}", level="debug")
buf = bytes(self.buf)
if buf.strip():
yield commands.Log(f"Client closed connection before completing request headers: {buf}")
yield commands.CloseConnection(self.conn)
else:
raise AssertionError(f"Unexpected event: {event}")
@ -259,7 +262,7 @@ class Http1Client(Http1Connection):
elif isinstance(event, RequestEndOfMessage):
if "chunked" in self.request.headers.get("transfer-encoding", "").lower():
yield commands.SendData(self.conn, b"0\r\n\r\n")
elif http1.expected_http_body_size(self.request) == -1:
elif http1.expected_http_body_size(self.request, self.response) == -1:
assert not self.send_queue
yield commands.CloseConnection(self.conn, half_close=True)
yield from self.mark_done(request=True)

View File

@ -79,8 +79,10 @@ class TCPLayer(layer.Layer):
(self.context.server.state & ConnectionState.CAN_READ)
)
if all_done:
yield commands.CloseConnection(self.context.server)
yield commands.CloseConnection(self.context.client)
if self.context.server.state is not ConnectionState.CLOSED:
yield commands.CloseConnection(self.context.server)
if self.context.client.state is not ConnectionState.CLOSED:
yield commands.CloseConnection(self.context.client)
self._handle_event = self.done
if self.flow:
yield TcpEndHook(self.flow)

View File

@ -237,9 +237,9 @@ class _TLSLayer(tunnel.TunnelLayer):
self.tls.sendall(data)
yield from self.tls_interact()
def send_close(self) -> layer.CommandGenerator[None]:
def send_close(self, half_close: bool) -> layer.CommandGenerator[None]:
# We should probably shutdown the TLS connection properly here.
yield from super().send_close()
yield from super().send_close(half_close)
class ServerTLSLayer(_TLSLayer):

View File

@ -64,8 +64,7 @@ class TunnelLayer(layer.Layer):
else:
yield from self.receive_data(event.data)
elif isinstance(event, events.ConnectionClosed):
if self.conn != self.tunnel_connection:
self.conn.state = context.ConnectionState.CLOSED
self.conn.state &= ~context.ConnectionState.CAN_READ
if self.tunnel_state is TunnelState.OPEN:
yield from self.receive_close()
elif self.tunnel_state is TunnelState.ESTABLISHING:
@ -82,9 +81,11 @@ class TunnelLayer(layer.Layer):
yield from self.send_data(command.data)
elif isinstance(command, commands.CloseConnection):
if self.conn != self.tunnel_connection:
# we don't have a use case for distinguishing between read/write here
self.conn.state = context.ConnectionState.CLOSED
yield from self.send_close()
if command.half_close:
self.conn.state &= ~context.ConnectionState.CAN_WRITE
else:
self.conn.state = context.ConnectionState.CLOSED
yield from self.send_close(command.half_close)
elif isinstance(command, commands.OpenConnection):
# create our own OpenConnection command object that blocks here.
self.tunnel_state = TunnelState.ESTABLISHING
@ -123,8 +124,8 @@ class TunnelLayer(layer.Layer):
def send_data(self, data: bytes) -> layer.CommandGenerator[None]:
yield commands.SendData(self.tunnel_connection, data)
def send_close(self) -> layer.CommandGenerator[None]:
yield commands.CloseConnection(self.tunnel_connection)
def send_close(self, half_close: bool) -> layer.CommandGenerator[None]:
yield commands.CloseConnection(self.tunnel_connection, half_close=half_close)
class LayerStack:

View File

@ -21,7 +21,7 @@ def expect(*event_types):
else:
event_types_str = '|'.join(e.__name__ for e in event_types) or "no events"
raise AssertionError(
f"Unexpected event type at t{f.__qualname__}: "
f"Unexpected event type at {f.__qualname__}: "
f"Expected {event_types_str}, got {event}."
)

View File

@ -248,7 +248,8 @@ def test_disconnect_while_intercept(tctx):
>> reply_next_layer(lambda ctx: http.HttpLayer(ctx, HTTPMode.transparent))
<< http.HttpRequestHook(flow)
>> ConnectionClosed(server1)
>> reply(to=-2)
<< CloseConnection(server1)
>> reply(to=-3)
<< OpenConnection(server2)
>> reply(None)
<< SendData(server2, b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")
@ -532,10 +533,10 @@ def test_upstream_proxy(tctx, redirect, scheme, strategy):
@pytest.mark.parametrize("mode", ["regular", "upstream"])
@pytest.mark.parametrize("strategy", ["eager", "lazy"])
def test_http_proxy_tcp(tctx, mode, strategy):
@pytest.mark.parametrize("close_first", ["client", "server"])
def test_http_proxy_tcp(tctx, mode, strategy, close_first):
"""Test TCP over HTTP CONNECT."""
server = Placeholder(Server)
flow = Placeholder(HTTPFlow)
if mode == "upstream":
tctx.options.mode = "upstream:http://proxy:8080"
@ -578,12 +579,16 @@ def test_http_proxy_tcp(tctx, mode, strategy):
else:
assert server().address == ("proxy", 8080)
if close_first == "client":
a, b = tctx.client, server
else:
a, b = server, tctx.client
assert (
playbook
>> ConnectionClosed(tctx.client)
<< CloseConnection(server)
>> ConnectionClosed(server)
<< CloseConnection(tctx.client)
>> ConnectionClosed(a)
<< CloseConnection(b)
>> ConnectionClosed(b)
<< CloseConnection(a)
)

View File

@ -58,7 +58,6 @@ def test_simple(tctx):
>> ConnectionClosed(tctx.server)
<< CloseConnection(tctx.client, half_close=True)
>> ConnectionClosed(tctx.client)
<< CloseConnection(tctx.client)
<< CloseConnection(tctx.server)
<< tcp.TcpEndHook(f)
>> reply()
@ -95,6 +94,5 @@ def test_receive_data_after_half_close(tctx):
>> DataReceived(tctx.client, b"i'm late")
<< SendData(tctx.server, b"i'm late")
>> ConnectionClosed(tctx.client)
<< CloseConnection(tctx.client)
<< CloseConnection(tctx.server)
)