Describe the bug
I set min_mtu and initial_mtu to anything above 1200, because I send my own datagrams over QUIC and want to know up front how big they can be. With that, closing a connection while the handshake is still running hits this:
assertion failed: transmit.datagram_remaining_mut() >= MIN_PACKET_SPACE
Here is what I think goes on. A CONNECTION_CLOSE is written in every packet number space that still has keys. Each space passes its datagram on to the next one, so the next packet can go into the same datagram. The normal path for that checks first whether there is still room for a whole packet (max_packet_size > MIN_PACKET_SPACE). The CONNECTION_CLOSE branch does not check. It just returns.
A client pads its Initial CONNECTION_CLOSE packet to MIN_INITIAL_SIZE, 1200 bytes. If initial_mtu is 1200, that fills the datagram exactly. Nothing is left, so the Handshake space starts a new datagram and all is well. 1200 is the default, so I guess that is why you never run into this. If initial_mtu is larger, something is left over, and if that is less than MIN_PACKET_SPACE (87 bytes) the Handshake space writes into it anyway.
The TODO right above that branch says the same thing:
// TODO(flub): This breaks during the handshake if we can not coalesce
// packets due to space reasons: the next space would either fail a
// debug_assert checking for enough packet space or produce an invalid
// packet. [...]
I see both of those: the failed assert in a debug build, a broken packet in a release build.
Relevant Logs
Debug build, with min_mtu and initial_mtu both at 1250:
send{space=Initial pn=1 path_id=0}: frame=ACK_ECN ranges: [0..1] delay: 0µs
send{space=Initial pn=1 path_id=0}: frame=CONNECTION_CLOSE error_code=12 reason=''
send{space=Initial pn=1 path_id=0}: PADDING * 1145
send{space=Initial pn=1 path_id=0}: wrote packet size=1200
thread '...' panicked at noq-proto/src/connection/mod.rs:1601:17:
assertion failed: transmit.datagram_remaining_mut() >= MIN_PACKET_SPACE
Expected behavior
Close goes through without panic and without sending out a broken packet.
noq
Version: main, commit c334d2da2. It is in noq-v1.1.0 as well.
Platform(s)
Linux x86_64. Nothing about it looks platform specific: it happens inside noq-proto's own test harness, without any sockets.
Additional Context / Screenshots / GIFs
I have a fix and a test for this and will open a PR.
lolwut
Describe the bug
I set
min_mtuandinitial_mtuto anything above 1200, because I send my own datagrams over QUIC and want to know up front how big they can be. With that, closing a connection while the handshake is still running hits this:Here is what I think goes on. A CONNECTION_CLOSE is written in every packet number space that still has keys. Each space passes its datagram on to the next one, so the next packet can go into the same datagram. The normal path for that checks first whether there is still room for a whole packet (
max_packet_size > MIN_PACKET_SPACE). The CONNECTION_CLOSE branch does not check. It just returns.A client pads its Initial CONNECTION_CLOSE packet to
MIN_INITIAL_SIZE, 1200 bytes. Ifinitial_mtuis 1200, that fills the datagram exactly. Nothing is left, so the Handshake space starts a new datagram and all is well. 1200 is the default, so I guess that is why you never run into this. Ifinitial_mtuis larger, something is left over, and if that is less thanMIN_PACKET_SPACE(87 bytes) the Handshake space writes into it anyway.The TODO right above that branch says the same thing:
I see both of those: the failed assert in a debug build, a broken packet in a release build.
Relevant Logs
Debug build, with
min_mtuandinitial_mtuboth at 1250:Expected behavior
Close goes through without panic and without sending out a broken packet.
noq
Version:
main, commitc334d2da2. It is innoq-v1.1.0as well.Platform(s)
Linux x86_64. Nothing about it looks platform specific: it happens inside
noq-proto's own test harness, without any sockets.Additional Context / Screenshots / GIFs
I have a fix and a test for this and will open a PR.
lolwut
the issue being reported and wrote this as concisely and clearly
as they could. Taking full responsibility for the issue being
accurate.