Skip to content

Fix ignored QUIC connections never relaying past the ClientHello - #8339

Open
kasnder wants to merge 2 commits into
mitmproxy:mainfrom
kasnder:quic-passthrough-relay
Open

Fix ignored QUIC connections never relaying past the ClientHello#8339
kasnder wants to merge 2 commits into
mitmproxy:mainfrom
kasnder:quic-passthrough-relay

Conversation

@kasnder

@kasnder kasnder commented Jul 29, 2026

Copy link
Copy Markdown

Description

Fixes #8338: QUIC connections never relay past the ClientHello when an addon sets
ignore_connection in the tls_clienthello hook. The connection stalls in both directions
and mitmproxy raises AssertionError: Unexpected event type at UDPLayer.start.

ClientQuicLayer.receive_handshake_data swaps the QUIC layers for an ignored UDPLayer, and
two things go wrong, both because the swap does not account for the layers it replaces.

The parent layer's dispatch is patched twice, and neither assignment holds.
parent_layer._handle_event = replacement_layer._handle_event captures a bound method while
the UDP layer is still in its start state; UDPLayer.start later sets
self._handle_event = self.relay_messages on the replacement layer, which the parent never
re-reads. The accompanying parent_layer.handle_event = replacement_layer.handle_event
cannot compensate, because an outer layer may already hold a bound reference to the
original — NextLayer._ask caches exactly that as NextLayer._handle when it decides the
layer stack, which happens on the first datagram, before the ClientHello is parsed. Events
arrive through the cached method, find the parent unpaused, and dispatch into the frozen
start state.

This keeps handle_event untouched and replaces only _handle_event, with a dispatcher that
delegates to the UDP layer's current state function. Layer.handle_event re-reads
self._handle_event for every event, so that is the only assignment a stale cache cannot
defeat. The parent then keeps owning blocking and pause handling, and datagrams that arrive
while OpenConnection is pending are relayed in order once it completes.

A command issued before the swap can still be in flight, and its reply is routed to the
parent, which now dispatches it into the UDP layer. RawQuicLayer(force_raw=True) builds a
datagram UDPLayer in its constructor, so --mode reverse:quic:// reliably has an
outstanding UdpStartHook when the ClientHello arrives. Such replies are now dropped: the
parent resumes the UDP layer's own blocking commands itself, so anything that reaches the
dispatcher answers a command from a layer that no longer exists.

Neither facet is a race, and neither depends on connection_strategy. Both fire in either
server state.

Why the existing test misses this

test_passthrough_from_clienthello passes today because tutils.Playbook re-resolves
self.layer.handle_event for every event, so it always reaches the patched attribute, and
make_client_tls_layer wires child layers by hand "to avoid NextLayer noise". The added
tests drive events through a reference bound up front, which is what a real server run does,
and feed back a reply the ignored layer never asked for.

Verification

End-to-end, with an aioquic echo server behind mitmdump --mode reverse:quic:// and an
addon setting ignore_connection, driven by an aioquic client:

unmodified 12.2.3 / main with this change
client ConnectionError (no handshake) handshake completes, echo returned
target server receives nothing receives the stream data
mitmproxy has crashed! 9 0

Test suite on main (c69122b), Python 3.13.14:

  • test/mitmproxy/proxy + test/mitmproxy/addons/test_next_layer.py: 546 passed, 4 skipped.
  • The three added test cases fail without the change, with the same assertion the
    end-to-end run produces.
  • ruff format --check, ruff check, and mypy on the changed module are clean.

Checklist

  • I have updated tests where applicable.
  • I have added an entry to the CHANGELOG.

When an addon sets `ignore_connection` in the `tls_clienthello` hook,
`ClientQuicLayer.receive_handshake_data` swaps the QUIC layers for an ignored
`UDPLayer`. From the first event after the ClientHello, mitmproxy then raises

  AssertionError: Unexpected event type at UDPLayer.start:
  Expected Start, got DataReceived(client, ...)

and nothing is relayed in either direction, so the connection stalls. With
`--mode reverse:quic://` and an aioquic client, the handshake never completes
and the target server sees no datagrams at all.

Two things go wrong, both because the swap does not account for the layers it
replaces.

1. The parent layer's dispatch is patched twice, and neither assignment holds.
   `parent_layer._handle_event = replacement_layer._handle_event` captures a
   bound method while the UDP layer is still in its `start` state;
   `UDPLayer.start` later sets `self._handle_event = self.relay_messages` on the
   replacement layer, which the parent never re-reads. The accompanying
   `parent_layer.handle_event = replacement_layer.handle_event` cannot
   compensate, because an outer layer may already hold a bound reference to the
   original -- `NextLayer._ask` caches exactly that as `NextLayer._handle` when
   it decides the layer stack, which happens on the first datagram, before the
   ClientHello is parsed. Events arrive through the cached method, find the
   parent unpaused, and dispatch into the frozen `start` state.

   Keep `handle_event` untouched and replace only `_handle_event`, with a
   dispatcher that delegates to the UDP layer's current state function.
   `Layer.handle_event` re-reads `self._handle_event` for every event, so that
   is the only assignment a stale cache cannot defeat. The parent then keeps
   owning blocking and pause handling, and datagrams that arrive while
   `OpenConnection` is pending are relayed in order once it completes.

2. A command issued before the swap by one of the replaced layers can still be
   in flight, and its reply is routed to the parent, which now dispatches it
   into the UDP layer. `RawQuicLayer(force_raw=True)` builds a datagram
   `UDPLayer` in its constructor, so `--mode reverse:quic://` reliably has an
   outstanding `UdpStartHook` when the ClientHello arrives. Drop such replies:
   the parent resumes the UDP layer's own blocking commands itself, so anything
   reaching the dispatcher belongs to a layer that no longer exists.

Neither facet is a race, and neither depends on the connection strategy. The
first fires on the first post-ClientHello event in either server state, and the
`OpenConnectionCompleted` reply is misdelivered the same way -- it is
`replacement_layer` that holds `_paused` while the event reaches `parent_layer`.

The existing `test_passthrough_from_clienthello` misses this because
`tutils.Playbook` re-resolves `self.layer.handle_event` for every event, so it
always reaches the patched attribute, and `make_client_tls_layer` wires child
layers by hand to avoid `NextLayer`. The added tests drive events through a
reference bound up front, which is what a real server run does, and feed back a
reply the ignored layer never asked for. All three fail without this fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QUIC passthrough: ignore_connection in tls_clienthello never relays past the ClientHello

1 participant