feat: Removes ability to read OneRtt packets after connection closure

[maddeleine]

Release Summary:

Removes ability to read OneRtt packets after connection closure

Resolved issues:

N/A

Description of changes:

We discovered that s2n-quic can continue to process OneRtt packets after the connection has closed. This isn't desirable and can lead to weird states in the dc-quic state machine.

Call-outs:

This is a behavior change.

Testing:

Includes a blocklist improvement that fails all connections that process packets after connection closure.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[Mark-Simulacrum]

This is a behavior change.

Can you elaborate on the user-visible effects from this that we expect to see? For example, do connection error messages get worse, or is there some other succinct description of what we expect, at least when focusing on s2n-quic-dc-driven handshakes (which don't really have "application" payloads today, so are hopefully easier to describe)?

[maddeleine]

This is a behavior change.

Can you elaborate on the user-visible effects from this that we expect to see? For example, do connection error messages get worse, or is there some other succinct description of what we expect, at least when focusing on s2n-quic-dc-driven handshakes (which don't really have "application" payloads today, so are hopefully easier to describe)?

This wouldn't be noticeable for our Stream APIs, i.e. as soon as the connection closes, you can't call any stream APIs since they get reset. Basically any OneRtt Stream data that arrived after connection closure would now be discarded, but it doesn't really matter because the stream APIs didn't let you receive data on a closed connection in the first place (Not relevant for dc-quic.)
Where this would be observable is if you have logic in the Event Subscriber like "on_frame_received" or "on_packet_received", you would notice it there, since these events would no longer trigger when OneRtt data arrives after a connection closure. I could plausibly think of a scenario where you wouldn't get as far into the dc-quic handshake because you aren't reading packets that arrive post-closure, i.e. you might be less likely to report dc_complete on the server side since you aren't reading packet acks post-closure. But in the event of a connection error you probably shouldn't try to report dc-complete, because something did legitimately go wrong.

--> I guess I should mention, this shouldn't affect connection error message. It's only after a connection closure has already happened that the behavior change occurs. So you should already know why the connection closes.

[Mark-Simulacrum]

I think we had a bug in roughly 1.71 or 1.72 s2n-quic where we were sending the MTU probing complete frame without getting transport parameters indicating the frame is expected from the peer. Can we confirm that users deploying that buggy code would not hit full handshake failure once they deploy this on the other side? E.g., because we would be closing the connection too early? I believe during the investigation into that issue we concluded that dc.complete is still reliably reached, because MTU probing can never complete within ~3 RTT that dc.complete does. But that case was one where we wanted dc.complete despite an error being (later) encountered.

I think in general this is the right thing to do and we should move ahead. I'm happy we're adding a new metric for this (packet_dropped.reason|CONNECTION_CLOSED), that will help us detect problems due to this in the wild if they occur.