Subject: [quicwg/base-drafts] Pad path validation in both directions (#4241)

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Oct 2020 17:08:21 -0700

This enacts the proposal in [@kazuho&#39;s comment](https://github.com/quicwg/base-drafts/issues/4216#issuecomment-709664513) on #4216.  Padding PATH_RESPONSE and sending on the same path as PATH_CHALLENGE helps ensure that the path MTU in both directions of the path is sufficient for the protocol to function.

This is a nasty change; I&#39;m personally quite unhappy about the prospect of implementing it, but it seems like the best option we have.

Closes #4216.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Pad when validating addresses
  * A big caveat
  * Datagrams, not packets
  * Revert automated formatting of tables, wrap
  * Pad PATH_RESPONSE too

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/4241.patch
https://github.com/quicwg/base-drafts/pull/4241.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/4241

---

From: ianswett <[email-masked]>
Date: Sun, 18 Oct 2020 19:02:24 -0700

@ianswett commented on this pull request.

LG, but a few suggestions

> +A PATH_RESPONSE frame MUST be sent on the network path where the PATH_CHALLENGE
+was received.  This requirement MUST NOT be enforced by the endpoint that
+initiates path validation as that would enable an attack on migration; see
+{{off-path-forward}}.

I think this is a SHOULD not a MUST both because it's unenforceable and because future multicast use cases may need to not follow it.

```suggestion
A PATH_RESPONSE frame SHOULD be sent on the network path where the PATH_CHALLENGE
was received.  This ensures the path is functional in both directions.  This requirement
MUST NOT be enforced by the endpoint that initiates path validation as that would enable
an attack on migration; see {{off-path-forward}}.
```

> +least the smallest allowed maximum datagram size of 1200 bytes. This in
+combination with sending on the same path allows the endpoint that initiated
+path validation to verify that the path is able to carry datagrams of this size
+in both directions.

```suggestion
least the smallest allowed maximum datagram size of 1200 bytes. This verifies
that the path is able to carry datagrams of this size in both directions.
```

> +the data that was sent in a previous PATH_CHALLENGE frame.  This PATH_RESPONSE
+frame can be received on any network path.  This validates the path on which
+the PATH_CHALLENGE was sent.

```suggestion
the data that was sent in a previous PATH_CHALLENGE frame.  A PATH_RESPONSE
frame received on any network path validates the path on which the
PATH_CHALLENGE was sent.
```

-- 
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/4241#pullrequestreview-511366824

---

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Oct 2020 20:12:15 -0700

@martinthomson pushed 1 commit.

663465c6c3679870ac476964ce79adbdd7a73d08  Tweaks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4241/files/b3ce9e647ba7e0344f852662c200c161a5030537..663465c6c3679870ac476964ce79adbdd7a73d08

---

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Oct 2020 20:35:37 -0700

80cbb76dcb5f1873a00047ed147282682ab53df3  Wrap; make the other a SHOULD too

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4241/files/663465c6c3679870ac476964ce79adbdd7a73d08..80cbb76dcb5f1873a00047ed147282682ab53df3

---

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Oct 2020 20:36:06 -0700

If we're going to go with a SHOULD on using the same path, then it makes sense to use a SHOULD on the padding piece as well.

-- 
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/4241#issuecomment-711492569

---

From: ianswett <[email-masked]>
Date: Mon, 19 Oct 2020 07:36:43 -0700

@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/4241#pullrequestreview-511835206

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 19 Oct 2020 17:22:19 -0700

@janaiyengar commented on this pull request.

> +PATH_CHALLENGE was received.  This ensures the path is functional in both
+directions.  This requirement MUST NOT be enforced by the endpoint that

This justification could be a bit more precise.
```
This ensures that the peer's path validation succeeds only if the path is functional
in both directions.
```

>  frame unless constrained by congestion control.
 
+A PATH_RESPONSE frame SHOULD be sent on the network path where the
+PATH_CHALLENGE was received.  This ensures the path is functional in both
+directions.  This requirement MUST NOT be enforced by the endpoint that
+initiates path validation as that would enable an attack on migration; see
+{{off-path-forward}}.
+
+An endpoint SHOULD expand datagrams that contain a PATH_RESPONSE frame to at

Why is this optional?
```suggestion
An endpoint MUST expand datagrams that contain a PATH_RESPONSE frame to at
```

>  frame unless constrained by congestion control.
 
+A PATH_RESPONSE frame SHOULD be sent on the network path where the

We can require this from the endpoint, while at the same time requiring the peer to not enforce it. We've done unenforceable MUSTs in other parts of the specs. What is the justification for allowing this wiggle room?

```suggestion
A PATH_RESPONSE frame MUST be sent on the network path where the
```

-- 
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/4241#pullrequestreview-512245928

---

From: ianswett <[email-masked]>
Date: Mon, 19 Oct 2020 17:44:26 -0700

I requested it be a SHOULD for two reasons:
 1) It's unenforceable, and unenforceable MUSTs are unfortunate.
 2) If we ever did multipath, it may make sense to send PATH_RESPONSE on a different path than the PATH_CHALLENGE, and a SHOULD makes that possible.

-- 
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/4241#discussion_r508142179

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Oct 2020 17:45:24 -0700

abde090670ad22def944702d63ecde09600bd88b  Peer path validation...

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4241/files/80cbb76dcb5f1873a00047ed147282682ab53df3..abde090670ad22def944702d63ecde09600bd88b

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Oct 2020 17:46:42 -0700

@martinthomson commented on this pull request.

I was happy to accommodate Ian's request, but I think that MUST is slightly better, even if it is unenforceable (I'm sure that Marten will be happy to enforce it in practice.)

-- 
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/4241#discussion_r508142832

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 19 Oct 2020 18:32:42 -0700

@kazuho commented on this pull request.

LGTM, with a note regarding MUST vs. SHOULD for padding.

As I pointed out in #4253, datagram size is not authenticated, and therefore receivers can never enforce the use of padding (though they MAY discard packets that do not meet the padding requirement).

If we go with a MUST, we should be clear about 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/pull/4241#pullrequestreview-512268768

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 19 Oct 2020 21:01:50 -0700

A SHOULD suggests there are potentially good reasons where an implementation might deviate from the recommendation. If there aren't any, then it's a MUST. It doesn't have to be enforceable, since not doing it means the protocol might not work as intended. That is the case here -- not following the MUST means that the protocol might not work.

In terms of the value in deviating from this recommendation, I don't see this degree of freedom being useful for multipath, though it's likely I'm not seeing what you're thinking. That said, I don't think we should design multipath 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/pull/4241#discussion_r508195566

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Oct 2020 23:44:05 -0700

1e95cb2f928835f5f932ea3ebdce2784a9b1fd0c  MUST

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4241/files/abde090670ad22def944702d63ecde09600bd88b..1e95cb2f928835f5f932ea3ebdce2784a9b1fd0c

---

From: Magnus Westerlund <[email-masked]>
Date: Tue, 20 Oct 2020 01:37:52 -0700

@gloinul approved this pull request.

I think this is an improvement and is clear. So I am fine with going forward with this 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/pull/4241#pullrequestreview-512464858

---

From: ianswett <[email-masked]>
Date: Tue, 20 Oct 2020 05:33:37 -0700

-- 
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/4241#pullrequestreview-512654709

---

From: mirjak <[email-masked]>
Date: Tue, 20 Oct 2020 05:43:20 -0700

@mirjak commented on this pull request.

> @@ -2224,8 +2237,9 @@ PATH_RESPONSE frames.
 ### Successful Path Validation
 
 Path validation succeeds when a PATH_RESPONSE frame is received that contains
-the data that was sent in a previous PATH_CHALLENGE frame. This validates the
-path on which the PATH_CHALLENGE was sent.
+the data that was sent in a previous PATH_CHALLENGE frame.  A PATH_RESPONSE
+frame received on any network path validates the path on which the
+PATH_CHALLENGE was sent.

I guess it actually only validates one direction of that path but I guess that nit picking...

-- 
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/4241#pullrequestreview-512663209

---

From: Christian Huitema <[email-masked]>
Date: Tue, 20 Oct 2020 12:24:18 -0700

The proposed PR bakes the assumption that paths should be symmetric. This might be an OK restriction for V1, but I hate seeing that baked in the code. There are several scenarios in which data can be send one way on a broadcast channel (e.g., satellite or DVB), while the other way is through a slower but symmetric channel.

Supporting asymmetry requires testing each direction. The server, instead of changing the address at which it sends merely because it received data from the client on that new path, should perform its own Challenge/Response to validate the path. Since challenges have to be padded, this tests both continuity and MTU. The server learns that a path may exist by receiving packets on it; it only uses that path for sending if the path is validated.

I guess this is something we will need to revisit as part of support for multipath.

-- 
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/4241#issuecomment-713086130

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 20 Oct 2020 12:27:26 -0700

@huitema : It does not assume path symmetry, it tests for our MTU requirement for QUIC of 1200 bytes (https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#section-14-4).

-- 
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/4241#issuecomment-713087719

---

From: ianswett <[email-masked]>
Date: Tue, 20 Oct 2020 12:29:31 -0700

@huitema I agree with your concerns, which is why I asked for SHOULDs instead of MUST, but there may be other ways forward as well.

That being said, the current proposal seems quite reliable, it's just not very flexible.

-- 
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/4241#issuecomment-713088803

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 20 Oct 2020 12:31:33 -0700

@janaiyengar 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/4241#pullrequestreview-513065811

---

From: Christian Huitema <[email-masked]>
Date: Tue, 20 Oct 2020 12:31:44 -0700

@janaiyengar By symmetry, I mean the assumption that the pair of source and destination addresses and ports used by the server is exactly the symmetric of the pair used by the client. This is not a completely natural assumption in QUIC, because peers are able to process packets received from any address and port, as long as the crypto works 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/pull/4241#issuecomment-713089937

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 20 Oct 2020 12:44:10 -0700

@huitema -- ah, ok, I follow. Yes, this is definitely limited to the scope of problem that we have right now.  As you say, we might need to revisit this if we are to add use cases that we haven't yet considered.

-- 
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/4241#issuecomment-713096347

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 20 Oct 2020 13:13:27 -0700

@larseggert @LPardue: Merging this, since the plan is to do consensus as part of 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/pull/4241#issuecomment-713111783

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 20 Oct 2020 13:13:38 -0700

Merged #4241 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/4241#event-3900598912