Unable to reset connection with zero-length connection ID with stateless reset token about quic-go HOT 10 OPEN

Checking the stateless reset token before the connection ID is a pretty large performance penalty (2 instead of 1 map lookups), happening in the hot path of the transport, so that's a non-starter.

I could get behind your suggestion to add a OnDecryptionFailed callback. We'd need to be very careful about handling coalesced packets though, I'd argue that the callback should only be called if none of the packets can be decrypted.

Want to send us a PR?

from quic-go.

Can you explain why you think it should only be checked if every decryption fails?

A few thoughts on the RFC:

However, the comparison MUST be performed when the first packet in an incoming datagram either cannot be associated with a connection or cannot be decrypted.

not sure why only looking at the first packet in a datagram, as only the last one could be a stateless reset because it is short-header-like.

This design ensures that a Stateless Reset is -- to the extent possible -- indistinguishable from a regular packet with a short header.

a callback OnShortHeaderPacketDecryptionFailed to the transport should be sufficient

Senders MUST NOT coalesce QUIC packets with different connection IDs into a single UDP datagram

From the RFC it is not clear to me if coalesced packets can contain stateless resets.
From a network perspective, a stateless reset packet could look like a packet with the same connection ID.
If it does not start with the same connection ID, it think it is fine to drop it.

The receiver of coalesced QUIC packets MUST individually process each QUIC packet and separately acknowledge them, as if they were received as the payload of different UDP datagrams.

if decryption fails [...] the receiver [...] MUST attempt to process the remaining packets.

as I understand, a valid behavior would be to process the first packets in a datagram and then detect a stateless reset in the last packet because it cannot be decrypted. I am not sure why someone would send such a packet, but it would require checking only the last packet independently of the previous ones in the datagram.

Suggestion:

let the connection call back via OnShortHeaderPacketDecryptionFailed to the transport, if the decryption of any short header packet fails. The only exception is if the stateless reset is coalesced and it does not look like a short header packet with the same connection id as the preceding packets, then it will be dropped.

from quic-go.

A few thoughts on the RFC:

However, the comparison MUST be performed when the first packet in an incoming datagram either cannot be associated with a connection or cannot be decrypted.

not sure why only looking at the first packet in a datagram, as only the last one could be a stateless reset because it is short-header-like.

Some implementations satisfy the 1200 bytes minimum packet size requirement by appending some garbage data (or all 0s) to a coalesced packet. This is a valid implementation, and it shouldn't lead to a check for a stateless reset token.

let the connection call back via OnShortHeaderPacketDecryptionFailed to the transport, if the decryption of any short header packet fails. The only exception is if the stateless reset is coalesced and it does not look like a short header packet with the same connection id as the preceding packets, then it will be dropped.

Sounds good to me, modulo (maybe) the caveat for coalesced packets.

from quic-go.

Some implementations satisfy the 1200 bytes minimum packet size requirement by appending some garbage data (or all 0s) to a coalesced packet.

Ok appending random data makes sense to me, but still not why some packet of the coalesced packet has to be checked for a reset token if the first long header packet could not be decrypted.

This is a valid implementation, and it shouldn't lead to a check for a stateless reset token.

I don't see the problem, if by chance the random garbage data looks like a short header packet with the right connection id, the packet must even be tried to be decrypted, so checking for the reset token is negligible.

from quic-go.

This is a valid implementation, and it shouldn't lead to a check for a stateless reset token.

I don't see the problem, if by chance the random garbage data looks like a short header packet with the right connection id, the packet must even be tried to be decrypted, so checking for the reset token is negligible.

Yes, you're right about that. My argument is not a performance argument. My point is that a coalesced packet can never be a stateless reset, because that's what RFC 9000 specifies: The stateless reset is a packet on its own.

from quic-go.

Oh, now I see. And even stateless resets in datagrams starting with a long header must be detected. So it is true that a stateless reset can never be part of a coalesced packet. But technically, packets that look like coalesced packets can actually be a stateless reset packet. So OnShortHeaderPacketDecryptionFailed is not sufficient. Better would be OnFirstDatagramPacketDecryptionFailed, which then causes the transport to check the entire datagram.

From RFC 10.3:

A Stateless Reset uses an entire UDP datagram

Endpoints MUST send Stateless Resets formatted as a packet with a short header. However, endpoints MUST treat any packet ending in a valid stateless reset token as a Stateless Reset, as other QUIC versions might allow the use of a long header.

from quic-go.

We don't have to deal with this caveat as long as we don't support a QUIC version that allows long header stateless resets, do we?

That would simplify things, as there's no such QUIC version around, nor do I expect such a QUIC version to emerge anytime soon.

from quic-go.

In general, it conflicts with the statement in RFC 9000 about future versions of QUIC.
It also seems logical to me that stateless resets should be detected regardless of the used QUIC version, since the endpoint sending the token is unlikely to have any knowledge of the supported QUIC versions of the peer.
However, RFC 8999 does not mention anything about this.

So I think it should be fine for now to just check non-coalesced short-header packets.

from quic-go.

Sounds like a plan! Are you going to work on a PR?

from quic-go.

I will, but it's not super high on my priority list

from quic-go.