reuse certificate errors to avoid unnecessary connections

mitmproxy/connection.py

@@ -43,7 +43,13 @@ class Connection(serializable.Serializable, metaclass=ABCMeta):
     sockname: Optional[Address]
     """Our local `(ip, port)` tuple for this connection."""
     error: Optional[str] = None
-    """A string describing the connection error."""
+    """
+    A string describing a general error with connections to this address.
+
+    The purpose of this property is to signal that new connections to the particular endpoint should not be attempted,
+    for example because it uses an untrusted TLS certificate. Regular (unexpected) disconnects do not set the error
+    property. This property is only reused per client connection.
+    """
 
     tls: bool = False
     """

mitmproxy/proxy/events.py

@@ -84,7 +84,7 @@ class CommandCompleted(Event):
         command_reply_subclasses[command_cls] = cls
 
     def __repr__(self):
-        return f"Reply({repr(self.command)})"
+        return f"Reply({repr(self.command)},{repr(self.reply)})"
 
 
 command_reply_subclasses: typing.Dict[commands.Command, typing.Type[CommandCompleted]] = {}

mitmproxy/proxy/layers/http/__init__.py

@@ -683,32 +683,40 @@ class HttpLayer(layer.Layer):
         # Do we already have a connection we can re-use?
         if reuse:
             for connection in self.connections:
-                # see "tricky multiplexing edge case" in make_http_connection for an explanation
-                conn_is_pending_or_h2 = (
-                    connection.alpn == b"h2"
-                    or connection in self.waiting_for_establishment
-                )
-                h2_to_h1 = self.context.client.alpn == b"h2" and not conn_is_pending_or_h2
                 connection_suitable = (
                     event.connection_spec_matches(connection)
-                    and not h2_to_h1
                 )
                 if connection_suitable:
                     if connection in self.waiting_for_establishment:
                         self.waiting_for_establishment[connection].append(event)
                         return
-                    elif connection.connected:
+                    elif connection.error:
                         stream = self.command_sources.pop(event)
-                        yield from self.event_to_child(stream, GetHttpConnectionCompleted(event, (connection, None)))
+                        yield from self.event_to_child(stream, GetHttpConnectionCompleted(event, (None, connection.error)))
                         return
+                    elif connection.connected:
+                        # see "tricky multiplexing edge case" in make_http_connection for an explanation
+                        h2_to_h1 = self.context.client.alpn == b"h2" and connection.alpn != b"h2"
+                        if not h2_to_h1:
+                            stream = self.command_sources.pop(event)
+                            yield from self.event_to_child(stream, GetHttpConnectionCompleted(event, (connection, None)))
+                            return
                     else:
                         pass  # the connection is at least half-closed already, we want a new one.
 
-        can_use_context_connection = (
+        context_connection_matches = (
             self.context.server not in self.connections and
-            self.context.server.connected and
             event.connection_spec_matches(self.context.server)
         )
+        can_use_context_connection = (
+            context_connection_matches
+            and self.context.server.connected
+        )
+        if context_connection_matches and self.context.server.error:
+            stream = self.command_sources.pop(event)
+            yield from self.event_to_child(stream, GetHttpConnectionCompleted(event, (None, self.context.server.error)))
+            return
+
         context = self.context.fork()
 
         stack = tunnel.LayerStack()

mitmproxy/proxy/layers/tls.py

@@ -193,8 +193,9 @@ class _TLSLayer(tunnel.TunnelLayer):
             elif last_err == ('SSL routines', 'ssl3_get_record', 'wrong version number') and data[:4].isascii():
                 err = f"The remote server does not speak TLS."
             else:  # pragma: no cover
-                # TODO: Add test case one we find one.
+                # TODO: Add test case once we find one.
                 err = f"OpenSSL {e!r}"
+            self.conn.error = err
             return False, err
         else:
             # Here we set all attributes that are only known *after* the handshake.

test/mitmproxy/proxy/layers/http/test_http.py

@@ -1002,3 +1002,18 @@ def test_dont_reuse_closed(tctx):
             >> DataReceived(server2, b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n")
             << SendData(tctx.client, b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n")
     )
+
+
+def test_reuse_error(tctx):
+    """Test that an errored connection is reused."""
+    tctx.server.address = ("example.com", 443)
+    tctx.server.error = "tls verify failed"
+    error_html = Placeholder(bytes)
+    assert (
+            Playbook(http.HttpLayer(tctx, HTTPMode.transparent), hooks=False)
+            >> DataReceived(tctx.client, b"GET / HTTP/1.1\r\n\r\n")
+            << SendData(tctx.client, error_html)
+            << CloseConnection(tctx.client)
+    )
+    assert b"502 Bad Gateway" in error_html()
+    assert b"tls verify failed" in error_html()

test/mitmproxy/proxy/test_layer.py

@@ -52,7 +52,7 @@ class TestLayer:
                 << commands.Log(" >! DataReceived(client, b'foo')", "debug")
                 >> tutils.reply(None, to=-3)
                 << commands.Log(" >> Reply(OpenConnection({'connection': Server("
-                                "{'id': '…rverid', 'address': None, 'state': <ConnectionState.OPEN: 3>})}))", "debug")
+                                "{'id': '…rverid', 'address': None, 'state': <ConnectionState.OPEN: 3>})}),None)", "debug")
                 << commands.Log(" !> DataReceived(client, b'foo')", "debug")
 
                 << commands.Log("baz", "info")