Subject: Re: [quicwg/base-drafts] handling of coalesced packets with decryption errors creates DoS opportunity (#2308)

From: Christian Huitema <[email-masked]>
Date: Tue, 07 May 2019 21:02:41 -0700

I am a bit worried when I hear suggestions of disabling coalescing with different encryption level. Initial(CH) + 0RTT is fairly common, and useful. So is Initial(SH) + handshake, Initial(ACK) + Handshake(ACK), or Handshake(Finished) + 1RTT. Yes, I can see attacks with "Christmas tree" packets containing too many coalesced segments. But mitigation is obvious -- implementations are free to drop packets for whatever reason. In the rare cases of false positive, when the funny coalesced packet is not in fact an attack, the peer will just resend after at time out.

-- 
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/2308#issuecomment-490338752

---

From: MikkelFJ <[email-masked]>
Date: Wed, 08 May 2019 00:01:44 -0700

> In the rare cases of false positive, when the funny coalesced packet is not in fact an attack, the peer will just resend after at time out.

What if it resends to the same "Christmas tree", for example for probing to get source CID?

-- 
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/2308#issuecomment-490372741

---

From: Christian Huitema <[email-masked]>
Date: Wed, 08 May 2019 07:49:09 -0700

I don't think this issue warrants changing the protocol specification. You may want to document the potential attack in the security section, and suggest mitigations. But IMHO this is one of the many ways in which a client can be nasty with a server, and vice versa. There are many other examples, such as acking packets one by one, sending multiple 1 byte stream frames, randomly alternating between multiple streams, etc. The generic pattern is to find some operation that is expensive for a particular server implementation, and then abuse it. You can't possibly document all the ways of doing 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/2308#issuecomment-490517076

---

From: MikkelFJ <[email-masked]>
Date: Wed, 08 May 2019 08:49:51 -0700

My point was that it is hard to define what is a valid abuse filter without risking not being able to connect with client which sends well intended but ill conceived "Christmass tree" packets. While you can always get into trouble, I think there are some difficulties here. 

-- 
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/2308#issuecomment-490541861

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 May 2019 16:52:03 -0700

@huitema 
> implementations are free to drop packets for whatever reason. In the rare cases of false positive, when the funny coalesced packet is not in fact an attack, the peer will just resend after at time out.

IIRC, one of the concerns regarding this approach was that we might see a deadlock if the peer has dropped the keys necessary for processing the the first packet within a datagram, and decides to drop the entire datagram due to not being able to process the first packet. Then, the endpoint might simply retransmit the coalesced datagram that starts from the same packet type as the previous datagram, and that datagram again gets dropped by the server, and ...

Therefore, it has been my view that it is beneficial to specify the minimum requirements that guarantees the connection to make progress. IIRC, my preference was to define a minimum number of packets within a single datagram that a receiver should process. But I am fine with what we have in the text right now, because it does not make the implementations complicated; a sensible sender would never send a datagram containing more than one packet at the same encryption level, detecting them on the receiver-side is an implementation choice.

-- 
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/2308#issuecomment-490692288

---

From: Christian Huitema <[email-masked]>
Date: Wed, 08 May 2019 17:14:39 -0700

@kazuho #2673 lands, we should have very few cases where _"the receiver has dropped the keys necessary for processing the first packet within a datagram"_. I imagine we could see some corner cases, but they most likely will be transient states, not leading to deadlocks. 

-- 
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/2308#issuecomment-490696764

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 May 2019 17:24:03 -0700

@huitema I agree that it's likely that we would not have deadlocks. But I think it's beneficial to specify how a server should not drop coalesced packets, because there would be performance issues if there is a disagreement about what is possible between the endpoints (one of the concerns being PMTU probes using coalesced packets (https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#rfc.section.14.3.1)).

-- 
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/2308#issuecomment-490698291

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 08 May 2019 18:45:24 -0700

Would a note suffice? Something like "an endpoint that coalesces more than a few packets risks having the peer not process all packets." I don't think we need stronger or RFC 2119 language here, just a note for sensible implementation is adequate I think.

-- 
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/2308#issuecomment-490711505

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 May 2019 19:58:54 -0700

@janaiyengar I personally think that that might be sufficient, though it is my understanding is that we already have a rough consensus on how to proceed; see https://github.com/quicwg/base-drafts/issues/2308#issuecomment-458778039.

Maybe I am incorrect in understanding the status, but based on this understanding, what I am wondering is if there's any new information that makes the WG reconsider.

-- 
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/2308#issuecomment-490724439

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 08 May 2019 20:37:13 -0700

Thanks @kazuho for reminding me that we discussed this in Tokyo and arrived at an agreement (it's not consensus per my read, it was agreement among the participants in the "room").

I'll gently poke @marten-seemann to write the PR.

-- 
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/2308#issuecomment-490730888

---

From: Christian Huitema <[email-masked]>
Date: Thu, 09 May 2019 04:11:04 +0000 (UTC)

I am a bit skeptical of the idea of using coalesced packets for PMTU discovery. If I receive a handshake packet after the handshake is finished, I will most probably ignore it. I don't have the keys anymore, I cannot remove header protection or packet protection. Packet will be treated as a decryption error, and whatever is coalesced with it will most likely be ignored.

I understand the goal of coalescing is to find a CID in the ICMP payload, but that seems somewhat brittle. Parsing the ICMP based on the 5 tuple would be much more robust, and does not require playing games with packet coalescing.

I may be a bit biased because I would rather just completely ignore all ICMP packets, and rely only on end-to-end ACKs. It removes the need to find a portable way to obtain ICMP messages through the socket API, and it also prevents attacks by ICMP spoofing.

-- 
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/2308#issuecomment-490736116

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 May 2019 21:18:57 -0700

@huitema 
> If I receive a handshake packet after the handshake is finished, I will most probably ignore it. I don't have the keys anymore, I cannot remove header protection or packet protection. Packet will be treated as a decryption error, and whatever is coalesced with it will most likely be ignored.

I would point out that the draft says don't. Quoting from [section 12.2](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#rfc.section.12.2): _if decryption fails (because the keys are not available or any other reason), the receiver MAY either discard or buffer the packet for later processing and MUST attempt to process the remaining packets._

> I understand the goal of coalescing is to find a CID in the ICMP payload, but that seems somewhat brittle. Parsing the ICMP based on the 5 tuple would be much more robust, and does not require play…

I agree that endpoints should rely on the 5 tuple whenever possible. However, if a stateless load balancer is being used, the CID becomes the only identifier for routing packets. That's why we have this mechanism.

-- 
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/2308#issuecomment-490737296

---

From: MikkelFJ <[email-masked]>
Date: Thu, 09 May 2019 00:10:35 -0700

I really wish QUIC did not use coalescing for any kind of semantics and only to avoid roundtrips.

-- 
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/2308#issuecomment-490772094

---

From: Igor Lubashev <[email-masked]>
Date: Thu, 09 May 2019 05:13:27 -0700

Coalescing for other-than-PMTU-discovery is to reduce the number of small datagrams. That's useful for reducing network bandwidth and CPU cost, especially for short connections.

So any kind of analysis for possible attacks should weigh a "Christmas Tree" coalesced packets vs. receiving each of the coalesced packets separately.  Coalescing makes sending such packets cheaper for the sender but it also makes it equally cheaper for the sever to receive.

My view is that packet coalescing is "neutral" in its effect on attack resilience of the protocol.  (One can actually say it is slightly beneficial, since it is harder to saturate network links with smaller datagrams.)

-- 
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/2308#issuecomment-490877936

---

From: MikkelFJ <[email-masked]>
Date: Thu, 09 May 2019 12:51:40 +0000 (UTC)

Yeah, meant reduce UDP packet count, not roundtrip.

-- 
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/2308#issuecomment-490890466

---

From: Lars Eggert <[email-masked]>
Date: Wed, 22 May 2019 06:22:18 -0700

Discussed in London; @marten-seemann will (still) make a PR to prohibit senders from coalescing packets of different encryption levels and to permit (but not require) receivers to enforce this constraint.

-- 
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/2308#issuecomment-494799700

---

From: Eric Kinnear <[email-masked]>
Date: Wed, 22 May 2019 06:24:57 -0700

Clarified in London: 
- Sender of a packet cannot coalesce multiple packets of the same type.
- If you receive multiple packets of the same type in the same datagram, you're not obligated to process all of them.

-- 
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/2308#issuecomment-494800763