Subject: [quicwg/base-drafts] stateless reset detection for undecryptable packets might be hard to implement (#4124)

From: Marten Seemann <[email-masked]>
Date: Tue, 22 Sep 2020 01:28:36 -0700

Regarding stateless reset detection, we currently say:

> This comparison can be performed for every inbound datagram. Endpoints MAY skip this check if any packet from a datagram is successfully processed. 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.

This might be hard to implement if you have a clear separation between the demuxer and individual connections. The demuxer would only look at the CID of an incoming packet, and pass a packet to the corresponding connection. The connection would then (possibly asynchronously) process that packet.
The current text requires the connection to somehow pass the packet back to the demuxer, so that it can be checked for stateless reset tokens. I'd argue that there's little harm in just dropping this packet in this case.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124

---

From: ianswett <[email-masked]>
Date: Tue, 22 Sep 2020 09:10:41 -0700

Practical question: How does one reset a connection if you can't associate the packet with a connection?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-696821930

---

From: Marten Seemann <[email-masked]>
Date: Tue, 22 Sep 2020 09:15:04 -0700

Using the stateless reset token. The demuxer basically has two hash maps, one CID -> conn and one token -> conn.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-696824967

---

From: Mike Bishop <[email-masked]>
Date: Tue, 22 Sep 2020 15:23:07 -0700

This is covered already.  The text says that you "MAY skip this check."  The code you describe makes it difficult to know whether to skip the check, so that implementation probably wouldn't.  The demuxer will always check for stateless reset tokens.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-697013659

---

From: Lars Eggert <[email-masked]>
Date: Wed, 23 Sep 2020 00:48:58 -0700

@marten-seemann do you see a need for any further changes with @MikeBishop's explanation? 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-697196113

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 00:54:29 -0700

I don't think "this check" refers to the check I'm referring to.

What we're saying is that when receiving a packet, you SHOULD do the following:
1. check your stateless reset token table. If you find the token in the table, close the conn. If you don't find this token in that table, then
2. check your CID table, and pass the packet to the connection

I interpret the "MAY skip this check" as saying that you can do 2. before 1. This reduces the number of hash map lookups in the common case to 1 instead of 2 per packet, so that's a worthwhile optimization.
However, the spec now says that if implement this optimization, you MUST do another lookup in the stateless reset token table if the connection was not able to unprotect the packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-697198648

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Sep 2020 02:52:10 -0700

> This comparison can be performed for every inbound datagram. Endpoints MAY skip this check if any packet from a datagram is successfully processed. However, the comparison MUST be performed when th…

It says that you can check every datagram.  This is easy to implement and can be done at any stage of your pipeline.  You can stop there.

It says that you MAY skip the check if you successfully process a packet.  This is harder to implement if you have separate demux and processing points.  This is especially so because the last sentence says that you can't skip this check if you didn't process any packets.  That is, you can apply an optimization on the condition that you don't apply the optimization uniformly.

For an integrated implementation, especially one that doesn't demux because it has a dedicated socket, it is easy to take advantage of the "MAY".  You try to process everything and check for a stateless reset if you can't process anything.

For a layered implementation that does stateless reset processing at the demux point, it is trickier, and could involve some back and forth.  Or you could just look for a stateless reset on every packet.  The odds that a packet is both a stateless reset and valid is small enough that you don't need to bother.

The design I think we had in mind was a demux point (a load balancer) that does strict demux and stateless resets generated by the next layer (the instance).  In that case, it's much more like the integrated implementation.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-697259227

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 23 Sep 2020 18:40:31 -0700

> However, the spec now says that if implement this optimization, you MUST do another lookup in the stateless reset token table if the connection was not able to unprotect the packet.

Right, but if you do the optimization, what other choice do you have? Meaning, how else would you detect the token? With the implementation strategy you suggest, and with this optimization, you have no choice but to get back to the demuxer.

Could you explain how you think we might fix this, if you have a fix in mind?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698061276

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 21:03:31 -0700

@janaiyengar Sure. The fix is to drop the undecryptable packet.

Assuming you're using a very short CID of just 4 bytes, the chance that the CID bytes match a connection you're handling is 2^-32 * N, where N is the number of connection you're handling. That should be a negligible chance for all practical purposes, even if you somehow manage to handle a million connections on the same machine.

Dropping a valid stateless reset is not the end of the world. You'll get another one for the next retransmission / probe you send to that endpoint, giving you another chance of detecting it this time.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698097934

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Sep 2020 21:27:46 -0700

There is no one stopping you from doing that, but I don't think that it is good to permit that in the specification.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698104176

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 22:15:38 -0700

Seems like you don't disagree that this is a reasonable way to implement stateless reset detection. If this is a reasonable implementation choice, why is it explicitly forbidden by the spec?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698117050

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 23 Sep 2020 22:50:16 -0700

@marten-seemann I might point out that the goal of stateless reset is to quickly notify the peer that the connection cannot be used. Waiting for the second packet goes against the purpose. Note also that, to avoid looping, endpoints are expected to limit the number of stateless reset packets they send.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698127750

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 22:58:46 -0700

@kazuho If you're handling 65336 connections (or rather, CIDs) and you're using a 4 byte CID, the probability of a random collision between the random data in the stateless reset packet and one of the connections you're serving is 1:65336. If you're handling that many connections, you'd probably be using longer CIDs anyway, which brings down the collision probability even further.
This is a minuscule collision probability, and a reasonable implementation wouldn't bother to optimize this corner case of a corner case (stateless reset is a corner case in itself).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698130820

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Sep 2020 23:13:45 -0700

A collision occurs with ~50% probability at that point.  Does that help make Kazuho's point?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698136419

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 23:18:37 -0700

I don't think this is a correct application of the birthday bound here. The server has a fixed set of these 2^16 connection IDs (which, by construction, are collision-free).
To get a collision with any of them, the random value, for which there are 2^32, would have to be match one of those 2^16 CIDs. That's a collision probability of 2^-16.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698138215

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Sep 2020 23:21:46 -0700

Ah, of course.  You aren't picking 2^16 times.  Still, this isn't about collision probability.  You probably have higher drop rates than that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698139398

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 23:24:10 -0700

> You probably have higher drop rates than that.

Exactly my point. If the stateless reset is much more likely to be dropped in the network than to cause a collision, I won't make things a lot worse by just dropping it myself in case of a collision.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698140366

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Sep 2020 23:28:15 -0700

Well, I think that it makes the counterpoint too.  Why come here and ask for permission when you can just pretend something got lost?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698141987

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Sep 2020 23:31:57 -0700

I'm not asking for permission here, and I have no intention to change the way I implemented stateless reset detection in quic-go.
I just don't think that the spec should prohibit a reasonable way to implement this mechanism.

How about we change the text to this:
> However, the comparison MUST be performed when the first packet in an incoming datagram either cannot be associated with a connection. The comparison MAY also be performed when a short header packet associated with a connection cannot be decrypted.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698143521

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 23 Sep 2020 23:38:34 -0700

> The comparison MAY also be performed when a short header packet associated with a connection cannot be decrypted.

I am not sure if I like this text. Depending on the size of the CID and the number of connections that an endpoint handles, this has to be MUST or SHOULD to achieve the goal of stateless reset.

IMO the question is if it's worth spending the cost to seek consensus on the acceptable rate of losing a stateless reset within an endpoint. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698146202

---

From: Mike Bishop <[email-masked]>
Date: Thu, 24 Sep 2020 13:30:16 -0700

I would be content with that change as a SHOULD.  However, I believe that would be a design change.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698572285

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 24 Sep 2020 14:17:38 -0700

It's an unenforceable MUST. While I understand the issue, I am going to argue that this is fine as it is, but it's not going to be ideal for all implementers. I would be fine with a SHOULD here too, but I do not think that the change is worth the delay this will cause.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698593243

---

From: ianswett <[email-masked]>
Date: Thu, 24 Sep 2020 16:02:40 -0700

Most of the changes I've come up with don't end up making the text any better, so I'd prefer to keep it as is.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698631203

---

From: David Schinazi <[email-masked]>
Date: Thu, 24 Sep 2020 16:07:42 -0700

I would also prefer to keep things as-is on this one.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698632713

---

From: Lucas Pardue <[email-masked]>
Date: Thu, 24 Sep 2020 16:34:56 -0700

I'm not seeing much interest in a design change and we are late in the process.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698640287

---

From: Lars Eggert <[email-masked]>
Date: Thu, 24 Sep 2020 23:29:39 -0700

Agreed. I will close this. If @marten-seemann really wants to push for changes, he can do so during IETF LC.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#issuecomment-698748024

---

From: Lars Eggert <[email-masked]>
Date: Thu, 24 Sep 2020 23:29:41 -0700

Closed #4124.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/4124#event-3806208059