Subject: [quicwg/base-drafts] Do not allow ICMP PTB to increase PMTU (#2161)

From: Igor Lubashev <[email-masked]>
Date: Thu, 13 Dec 2018 13:33:19 -0800

The only time an ICMP PTB message can ever legitimately try to report a PMTU larger than an endpoint's estimate of a PMTU is when it is sent in response to a PMTU probe packet.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2161

-- Commit Summary --

  * Do not allow ICMP PTB to increase PMTU

-- File Changes --

    M draft-ietf-quic-transport.md (3)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2161.patch
https://github.com/quicwg/base-drafts/pull/2161.diff

-- 
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/pull/2161

---

From: Igor Lubashev <[email-masked]>
Date: Thu, 13 Dec 2018 21:52:57 +0000 (UTC)

Note: it would be wrong to increase PMTU merely due to a receipt of a validated ICMP PTB message received in response to a PMTU probe.  Only an ACK of a probe can increase PMTU to the probe's size.  But a receipt of such ICMP PTB message can trigger a new PMTU probe of the given size.

All of this is a subject to DPLPMTUD specification.  Hence, I did not include it in this 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/pull/2161#issuecomment-447133989

---

From: Martin Thomson <[email-masked]>
Date: Thu, 13 Dec 2018 20:59:34 -0800

martinthomson requested changes on this pull request.

> @@ -3139,7 +3139,8 @@ attacks that successfully guess the IP address 3-tuple and reduce the PMTU to a
 bandwidth-inefficient value.
 
 An endpoint MUST ignore an ICMP message that claims the PMTU has decreased below
-1280 bytes.
+1280 bytes.  And endpoint MUST ignore an ICMP message that attempts to increase
+PMTU, unless the ICMP message was received in response to a PMTU probe packet.

Not sure about the "unless" part here.

Say I expect PMTU to be 1280.  Then I send a probe for 2048.  I get back an ICMP PTB that says 1500.  This would allow the PMTU to be set to 1500, but I think that you want to probe at 1500 before increasing anything.

Thus, I would instead say that an ICMP message that claims a larger PMTU than the confirmed PMTU MUST be confirmed (by sending a packet at that size and having it acknowledged) before the PMTU can be increased.

-- 
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/pull/2161#pullrequestreview-184969037

---

From: Igor Lubashev <[email-masked]>
Date: Fri, 14 Dec 2018 06:55:42 -0800

igorlord commented on this pull request.

See the note above where I was wondering if people would read it the way you did.

The "unless" part only cancels "ignore the message" instruction.  It does not recommend using the message's MTU as PMTU w/o probing it first.

I did not want to get into PLDPMTUD probe size selection details, but, yet, you were confused by the text, so I will add a few more words...

-- 
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/pull/2161#discussion_r241780513

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 14 Dec 2018 07:13:28 -0800

gorryfair commented on this pull request.

It seems this thread is re-inventing machinery to do path probing.

I am really unsure why this would propose  a different approach than the TSVWG draft? - It would be useful to have a starting point to understand why QUIC would behave in a different way to other datagram transports. 

-- 
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/pull/2161#discussion_r241786324

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 14 Dec 2018 16:30:38 +0000 (UTC)

gloinul commented on this pull request.

Wouldn't the best thing here be to tweak the next paragraph to say that any reactions to ICMP messages should be validated and DPLPMTUD do specify how to do such validation both for increasing and decreasing the PMTU?

-- 
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/pull/2161#discussion_r241813264

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 14 Dec 2018 17:07:43 +0000 (UTC)

On 14/12/2018, 16:30, Magnus Westerlund wrote:
>
> *@gloinul* commented on this pull request.
>
> ------------------------------------------------------------------------
>
> In draft-ietf-quic-transport.md 
> <https://github.com/quicwg/base-drafts/pull/2161#discussion_r241813264>:
>
> Wouldn't the best thing here be to tweak the next paragraph to say 
> that any reactions to ICMP messages should be validated and DPLPMTUD 
> do specify how to do such validation both for increasing and 
> decreasing the PMTU?
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub 
> <https://github.com/quicwg/base-drafts/pull/2161#discussion_r241813264>, 
> or mute the thread 
> <https://github.com/notifications/unsubscribe-auth/AHC1koQJq3mAAm889Dnr6rCmDGrSvVfBks5u49IugaJpZM4ZSdss>.
>
I think terminology needs to be correct. It was really hard to get the 
terminology consistent in DPLPMTUD, and since QUIC has not (yet) adopted 
the same language, this may be tricky to wordsmith, but here is what I 
suggest:

Put simply, I would suggest a PTB message can NEVER increase the PLPMTU, 
even when it reports a value greater than the PLPMTU after a probe 
message. Unless someone explains different, I will continue to assert 
that PTB messages MUST be validated - at least the IP addresses, 
transport, and ports. Accepting arbitrary information to control the 
transport isn't a good start to building a useful protocol.

I have reasons to suggest that endpoints also not to increase their 
PLPMTU based on the PTB reported size:

(1) Because soemtimes the reported MTU size can be garbled, and I would 
encourage being conservative to avoid black-holing.
(2) Because the sender still doesn't know an actual segment larger than 
the PLPMTU would transit the end to end path.
(3) A path that is incosnistent in forwarding would result in more 
corner cases if you start reacting to PTB signals, relying on actual 
probes is simpler.

Therefore, I would trigger a probe for the reported size. That's what 
DPLPMTUD currentl says (you could refer to that spec to explain this.)

Gorry

-- 
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/pull/2161#issuecomment-447388870

---

From: Igor Lubashev <[email-masked]>
Date: Fri, 14 Dec 2018 10:54:02 -0800

@gloinul, DPLPMTUD is not a requirement for QUIC.

This text was to apply to all implementations -- ones performing DPLPMTUD or traditional PMTUD or something else.  It is possible that we just reject this PR and assume people will follow directions specified in whatever PMTUD scheme they are implementing.

-- 
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/pull/2161#discussion_r241854827

---

From: Igor Lubashev <[email-masked]>
Date: Fri, 14 Dec 2018 18:59:09 +0000 (UTC)

@gorryfair, I like this suggestion -- not talking about ignoring ICMP messages but about not using ICMP messages to increase PMTU.  That would be very clear and concise.

Validation with at least IP addresses, transport (implicit in a QUIC spec), and ports is a matter for #2109, and it looks like it will be resolved exactly this way.

-- 
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/pull/2161#issuecomment-447421214

---

From: Igor Lubashev <[email-masked]>
Date: Fri, 14 Dec 2018 12:24:52 -0800

@igorlord pushed 1 commit.

6b3b9882b5807709a294f30d261ca95c5519f151  Do not talk about ignoring ICMP that increases PMTU. Talk about disallowing PMTU increases.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2161/files/a90fb86a36d7582637ee093a61dae3c0fa216d45..6b3b9882b5807709a294f30d261ca95c5519f151

---

From: Igor Lubashev <[email-masked]>
Date: Fri, 14 Dec 2018 12:28:05 -0800

@martinthomson, @gorryfair, is this better now?  (See updated diff.)

-- 
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/pull/2161#issuecomment-447468194

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Dec 2018 18:00:38 -0800

martinthomson approved this pull request.

Seems OK to me.

-- 
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/pull/2161#pullrequestreview-185419307

---

From: Magnus Westerlund <[email-masked]>
Date: Mon, 17 Dec 2018 05:26:52 -0800

I think that text is basically fine. 

What isn't that clear is this section's normative language interactions with the next section that discusses DPLPMTUD. I really think it should be clear her that
1. If you do something basic follow these things.
2. If you want to do something that can provide some benefits but also create more complexities then follow this specficiation [DPLPMTUD], and in case there are inconsistencies with the text for 1) then 2) overrides such limiations. 

-- 
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/pull/2161#issuecomment-447844777

---

From: ianswett <[email-masked]>
Date: Tue, 18 Dec 2018 15:25:12 -0800

ianswett approved this pull request.

-- 
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/pull/2161#pullrequestreview-186329459

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 15:25:23 -0800

Merged #2161 into master.

-- 
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/pull/2161#event-2034369520