Subject: Re: [quicwg/base-drafts] Padding requirement seems to be incorrect. (#3053)

From: David Schinazi <[email-masked]>
Date: Tue, 01 Oct 2019 12:41:53 -0700

I think the specification should mandate padding of server initial packets as well.

When I was traveling a couple weeks ago, I was on a network where, when using IPv6, it allowed 1200-byte UDP packets through from client to server but blackholed them from server to client. Since our implementation pads initials in both directions, we were able to detect this failure during the handshake and fall back to TCP/TLS. If the server didn't pad its initials, we would have succeeded at the handshake but never gotten an HTTP response.

-- 
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/3053#issuecomment-537196679

---

From: ekr <[email-masked]>
Date: Tue, 01 Oct 2019 13:44:32 -0700

> When I was traveling a couple weeks ago, I was on a network where, when using IPv6, it allowed 1200-byte UDP packets through from client to server but blackholed them from server to client. Since o…

This *definitely* seems like a design issue. The purpose of the padding rule is solely to limit amplification, not to establish a minimum MTU. If you want some sort of MTU rule, that should be discussed separately.

-- 
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/3053#issuecomment-537222069

---

From: David Schinazi <[email-masked]>
Date: Tue, 01 Oct 2019 13:56:04 -0700

This issue is marked as design. Are you suggesting I should open a separate 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/3053#issuecomment-537226713

---

From: MikkelFJ <[email-masked]>
Date: Tue, 01 Oct 2019 14:06:15 -0700

@ekr I believe it is dual purpose:

>From the current transport draft:

>The payload of a UDP datagram carrying the first Initial packet MUST be expanded to at least 1200 bytes, by adding PADDING frames to the Initial packet and/or by coalescing the Initial packet (see Section 12.2). Sending a UDP datagram of this size ensures that the network path supports a reasonable Maximum Transmission Unit (MTU), and helps reduce the amplitude of amplification attacks caused by server responses toward an unverified client address; see Section 8.

-- 
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/3053#issuecomment-537230448

---

From: ekr <[email-masked]>
Date: Tue, 01 Oct 2019 14:12:00 -0700

Yeah, that's dicta as far as I am concerned. I don't think we ever had consensus that we were going to enforce MTU 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/3053#issuecomment-537232577

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Oct 2019 19:11:57 -0700

The insight I had here was this:

If you are sending Initial, it is possible that this is the only packet in the datagram that can be processed by your peer.  If that is the case, then a client needs to ensure that it pads the datagram as though no other packets were present.

You can infer from the fact that you have Handshake keys that the server is able to process Handshake packets, and then extend that to say that if you include Handshake packets then the server won't need padding.  But then you can extend that inference to saying that there is no need to send any Initial packets (as the current draft does; currently, you are not required to keep Initial keys once you are sending with Handshake keys).

Either way, I figure that it is better to keep things simple and say "if Initial, pad".  In those cases where padding ends up being excessive, it's only excessive for a short while.

-- 
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/3053#issuecomment-540296849

---

From: ekr <[email-masked]>
Date: Wed, 09 Oct 2019 20:55:01 -0700

> If you are sending Initial, it is possible that this is the only packet in the datagram that can be processed by your peer. If that is the case, then a client needs to ensure that it pads the datag…

I don't know where you get this from. It's total datagram size that matters, not the contents of the rest of the datagram (which is why the neqo padding strategy works).

Anyway, the algorithm you propose seems fine, but it's not necessary, and therefore there's no need to specify it in 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/3053#issuecomment-540335959

---

From: MikkelFJ <[email-masked]>
Date: Thu, 10 Oct 2019 00:43:01 -0700

>  It's total datagram size that matters, not the contents of the rest of the datagram (which is why the neqo padding strategy works).

This was my thought too.

-- 
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/3053#issuecomment-540440293

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Oct 2019 12:20:18 -0700

> I don't know where you get this from. It's total datagram size that matters, not the contents of the rest of the datagram (which is why the neqo padding strategy works).

I'm not seeing a conflict between these statements.  If the packet contains an Initial, the total datagram size must be at least 1200 bytes.  You can achieve this by padding any packet in the payload; I don't think anyone is advocating for requiring the padding be in a particular spot.

-- 
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/3053#issuecomment-540739629

---

From: ekr <[email-masked]>
Date: Thu, 10 Oct 2019 14:34:24 -0700

I don't understand what "pads the datagram as though no other packets were present" is supposed to mean, then.

The way neqo does this is that it puts the packets in the datagram and then pads the rest, without any regard to what keys the server might have. 

-- 
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/3053#issuecomment-540809958

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Oct 2019 17:23:31 -0700

Imprecise wording on my part.  "pads the datagram as though no other packets were present" might be better as "as though no other packets can be processed by the server".

Let's keep in mind that the proposed changes is just striking "only" in "Clients MUST ensure that UDP datagrams containing ~~only~~ Initial packets have UDP payloads of at least 1200 bytes, [...]".

I agree that this will result in larger packets than are ideal in a couple of cases.  But the ACK-only Initial packets a client sent are most likely coalesced with other packets outside of cases where the padding is genuinely required.

-- 
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/3053#issuecomment-540850182

---

From: ekr <[email-masked]>
Date: Thu, 10 Oct 2019 17:52:11 -0700

I'm sorry, but I still don't see what benefit this change provides. Perhaps this would be easier next week.

-- 
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/3053#issuecomment-540855250

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Oct 2019 19:42:31 -0700

The change makes it clear that if you send Initial+0-RTT then you should still pad to 1200.  That's all.  Previously, you would not have been required to pad, which would have been bad.

Most of the debate and discussion is about the knock-on consequences of applying the broader rule, which I don't think are that important.  And yes, the whole "when do you discard Initial keys" thing does confound things a little.

-- 
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/3053#issuecomment-540875950

---

From: Lars Eggert <[email-masked]>
Date: Wed, 16 Oct 2019 14:08:14 -0700

Discussed in Cupertino. The word "only" is to be removed from the respective sentence. Moving to Editorial.

-- 
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/3053#issuecomment-542892358

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Oct 2019 15:20:44 -0700

Closed #3053 via #3051.

-- 
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/3053#event-2735019749