partially revert "detect recursive self-connects and stop them"

This partially reverts commit be20765129.

The intended solution is prone to race conditions where server_connected
triggers before the new connection is registered. We revert the change here,
but keep the tests.
This commit is contained in:
Maximilian Hils 2021-03-30 08:55:49 +02:00
parent be20765129
commit d0e54bc32c
2 changed files with 22 additions and 26 deletions

View File

@ -5,7 +5,7 @@ from typing import Dict, Optional, Tuple
from mitmproxy import command, controller, ctx, flow, http, log, master, options, platform, tcp, websocket from mitmproxy import command, controller, ctx, flow, http, log, master, options, platform, tcp, websocket
from mitmproxy.flow import Error, Flow from mitmproxy.flow import Error, Flow
from mitmproxy.proxy import commands, events from mitmproxy.proxy import commands, events
from mitmproxy.proxy import server, server_hooks from mitmproxy.proxy import server
from mitmproxy.proxy.layers.tcp import TcpMessageInjected from mitmproxy.proxy.layers.tcp import TcpMessageInjected
from mitmproxy.proxy.layers.websocket import WebSocketMessageInjected from mitmproxy.proxy.layers.websocket import WebSocketMessageInjected
from mitmproxy.utils import asyncio_utils, human, strutils from mitmproxy.utils import asyncio_utils, human, strutils
@ -180,8 +180,3 @@ class Proxyserver:
self.inject_event(event) self.inject_event(event)
except ValueError as e: except ValueError as e:
ctx.log.warn(str(e)) ctx.log.warn(str(e))
def server_connected(self, ctx: server_hooks.ServerConnectionHookData):
# check if the outbound part of this connection appeared as a new client.
if ctx.server.sockname in self._connections:
ctx.server.error = "Stopped mitmproxy from recursively connecting to itself."

View File

@ -177,31 +177,32 @@ class ConnectionHandler(metaclass=abc.ABCMeta):
else: else:
addr = human.format_address(command.connection.address) addr = human.format_address(command.connection.address)
self.log(f"server connect {addr}") self.log(f"server connect {addr}")
await self.handle_hook(server_hooks.ServerConnectedHook(hook_data)) connected_hook = asyncio_utils.create_task(
self.handle_hook(server_hooks.ServerConnectedHook(hook_data)),
name=f"handle_hook(server_connected) {addr}",
client=self.client.peername,
)
if not connected_hook:
return # this should not be needed, see asyncio_utils.create_task
if errmsg := command.connection.error: self.server_event(events.OpenConnectionCompleted(command, None))
self.log(f"server connection to {addr} killed: {errmsg}")
self.server_event(events.OpenConnectionCompleted(command, f"Connection killed: {errmsg}"))
del self.transports[command.connection]
writer.close()
else:
self.server_event(events.OpenConnectionCompleted(command, None))
# during connection opening, this function is the designated handler that can be cancelled. # during connection opening, this function is the designated handler that can be cancelled.
# once we have a connection, we do want the teardown here to happen in any case, so we # once we have a connection, we do want the teardown here to happen in any case, so we
# reassign the handler to .handle_connection and then clean up here once that is done. # reassign the handler to .handle_connection and then clean up here once that is done.
new_handler = asyncio_utils.create_task( new_handler = asyncio_utils.create_task(
self.handle_connection(command.connection), self.handle_connection(command.connection),
name=f"server connection handler for {addr}", name=f"server connection handler for {addr}",
client=self.client.peername, client=self.client.peername,
) )
if not new_handler: if not new_handler:
return # this should not be needed, see asyncio_utils.create_task return # this should not be needed, see asyncio_utils.create_task
self.transports[command.connection].handler = new_handler self.transports[command.connection].handler = new_handler
await asyncio.wait([new_handler]) await asyncio.wait([new_handler])
self.log(f"server disconnect {addr}") self.log(f"server disconnect {addr}")
command.connection.timestamp_end = time.time() command.connection.timestamp_end = time.time()
await connected_hook # wait here for this so that closed always comes after connected.
await self.handle_hook(server_hooks.ServerDisconnectedHook(hook_data)) await self.handle_hook(server_hooks.ServerDisconnectedHook(hook_data))
async def handle_connection(self, connection: Connection) -> None: async def handle_connection(self, connection: Connection) -> None: