Subject: Re: [quicwg/base-drafts] Introduces PATH_CHALLENGE and PATH_RESPONSE frames (#1086)

From: ianswett <[email-masked]>
Date: Thu, 01 Feb 2018 15:46:21 +0000 (UTC)

ianswett commented on this pull request.

Some suggested text on validation and acking.

> -If the PING frame is determined to be lost, a new PING frame SHOULD be
-generated.  This PING frame MUST include a new Data field that is similarly
-difficult to guess.
+endpoint validates a remote address by sending a PATH_CHALLENGE frame containing
+a payload that is hard to guess.  This frame MUST be sent in a packet that is
+sent to the new address.  Once a PATH_RESPONSE frame containing the same payload
+is received, the address is considered to be valid.  The PATH_RESPONSE frame can
+use any path on its return.
+
+An endpoint MAY send multiple PATH_CHALLENGE frames to handle packet loss or to
+make additional measurements on a new network path.
+
+An endpoint MUST use fresh random data in every PATH_CHALLENGE frame so that it
+can associate the peer's response with the causative PATH_CHALLENGE.  Using
+fresh values allows the endpoint to measure the time taken to respond to a
+challenge.

This gets a bit weird if the PATH_RESPONSE doesn't come back via the new path.

 If validation of the new remote address fails, after allowing enough time for
-possible loss and recovery of packets carrying PING and PONG frames, the
-endpoint MUST terminate the connection.  When setting this timer,
-implementations are cautioned that the new path could have a longer round trip
-time than the original.  The endpoint MUST NOT send a CONNECTION_CLOSE frame in
-this case; it has to assume that the remote peer does not want to receive any
-more packets.
+recovering from possible loss of packets carrying PATH_CHALLENGE and
+PATH_RESPONSE frames, the endpoint MUST terminate the connection.  When setting
+this timer, implementations are cautioned that the new path could have a longer
+round trip time than the original.  The endpoint MUST NOT send a
+CONNECTION_CLOSE frame in this case; it has to assume that the remote peer does
+not want to receive any more packets.

I would change this to "cannot" from "does not want to" as Mike suggested.

> @@ -1619,10 +1627,10 @@ the legitimate copy will be dropped as a duplicate.
 
 After a spurious migration, validation of the source address will fail because
 the entity at the source address does not have the necessary cryptographic keys
-to read or respond to the PING frame that is sent to it, even if it wanted to.
-Such a spurious connection migration could result in the connection being
-dropped when the source address validation fails.  This grants an attacker the
-ability to terminate the connection.
+to read or respond to the PATH_CHALLENGE frame that is sent to it, even if it
+wanted to.  Such a spurious connection migration could result in the connection
+being dropped when the source address validation fails.  This grants an attacker
+the ability to terminate the connection.

I think Jana's planning to improve this in a follow up, so I'm ok with it as is.

> +|                                                               |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+
+~~~
+
+Data:
+
+: This 8-byte field contains arbitrary data.
+
+A PATH_CHALLENGE frame containing 8 octets that are hard to guess is sufficient
+to ensure that it is easier to receive the packet than it is to guess the value
+correctly.
+
+The recipient of this frame MUST generate a PATH_RESPONSE frame
+({{frame-path-response}}) containing the same Data.  An endpoint that receives a
+PATH_CHALLENGE frame containing an empty payload MUST generate a connection

There's no length, so I don't believe you can have an empty PATH_CHALLENGE frame

> +to ensure that it is easier to receive the packet than it is to guess the value
+correctly.
+
+The recipient of this frame MUST generate a PATH_RESPONSE frame
+({{frame-path-response}}) containing the same Data.  An endpoint that receives a
+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+
+## PATH_RESPONSE Frame {#frame-path-response}
+
+The PATH_RESPONSE frame (type=0x0f) is sent in response to a PATH_CHALLENGE
+frame.  Its format is identical to the PATH_CHALLENGE frame
+({{frame-path-challenge}}).
+
+An endpoint that receives a PATH_RESPONSE frame containing an empty payload MUST

Same comment: It can no longer be empty

> @@ -2692,10 +2709,19 @@ implementation decision, and an implementation should be careful to delay
 conservatively, since any delay is likely to increase application-visible
 latency.
 
-Regular QUIC packets are "containers" of frames; a packet is never retransmitted
-whole.  How an endpoint handles the loss of the frame depends on the type of the
-frame.  Some frames are simply retransmitted, some have their contents moved to
-new frames, and others are never retransmitted.
+Regular QUIC packets are "containers" of frames.  When an endpoint receives an
+ACK frame for one or more transmitted packets, all frames in the acknowledged
+packets are considered to have been delivered to the peer, with one exception.

How about "received and processed by" instead of "delivered to", since processing is as important as receipt

> @@ -2692,10 +2709,19 @@ implementation decision, and an implementation should be careful to delay
 conservatively, since any delay is likely to increase application-visible
 latency.
 
-Regular QUIC packets are "containers" of frames; a packet is never retransmitted
-whole.  How an endpoint handles the loss of the frame depends on the type of the
-frame.  Some frames are simply retransmitted, some have their contents moved to
-new frames, and others are never retransmitted.
+Regular QUIC packets are "containers" of frames.  When an endpoint receives an
+ACK frame for one or more transmitted packets, all frames in the acknowledged
+packets are considered to have been delivered to the peer, with one exception.
+A PATH_CHALLENGE frame (see {{frame-path-challenge}}) is used to validate a

I would prefer to leave this text out because it's more confusing than helpful.  It's ok to consider it delivered to the peer when it's acknowledged, the key is that one must receive a PATH_RESPONSE before the path has been verified, which you say above.

> @@ -1562,25 +1563,31 @@ consider the possibility that packets are sent without congestion feedback.
 
 Once a connection is established, address validation is relatively simple (see
 {{address-validation}} for the process that is used during the handshake).  An
-endpoint validates a remote address by sending a PING frame containing a payload
-that is hard to guess.  This frame MUST be sent in a packet that is sent to the
-new address.  Once a PONG frame containing the same payload is received, the
-address is considered to be valid.  The PONG frame can use any path on its
-return.  A PING frame containing 12 randomly generated {{?RFC4086}} octets is
-sufficient to ensure that it is easier to receive the packet than it is to guess
-the value correctly.
-
-If the PING frame is determined to be lost, a new PING frame SHOULD be
-generated.  This PING frame MUST include a new Data field that is similarly
-difficult to guess.
+endpoint validates a remote address by sending a PATH_CHALLENGE frame containing
+a payload that is hard to guess.  This frame MUST be sent in a packet that is
+sent to the new address.  Once a PATH_RESPONSE frame containing the same payload

How about "The new address is not validated until a PATH_RESPONSE frame containing the same payload is received, even if the packet containing the PATH_RESPONSE frame is acknowledged."  (and then remove the text below I think is a bit confusing)

-- 
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/1086#pullrequestreview-93322922

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 08:23:56 -0800

@janaiyengar pushed 1 commit.

5ce6149  ian's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1086/files/4c470d89681c8bd961c2bc85647049fcb4e4b160..5ce614938e8fa3644d08c13a413b182fe599a527

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 16:24:08 +0000 (UTC)

janaiyengar commented on this pull request.

Added. Which text is weird? The text about measuring time taken? It's caused me enough trouble so I'll remove it.

Yup, and validation gets weird when PATH_RESPONSE is sent via any path too. Again, I don't want to fix the connection migration mechanism here, so leaving it as is. But removed text about measuring time taken.

It depends on what the semantics of delivery are. What I say above is that an ACK implies that the frame has been received and processed by the peer. I don't think that's a safe assumption with PATH_CHALLENGE, since the entire point of it is to get something besides an ACK.

Done.

Yup. Happy to take this comment in the follow-up.

Good point. Removed the text, since it seems redundant now.

+correctly.
+

Done.

Done.

-- 
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/1086#discussion_r165405265

---

From: ianswett <[email-masked]>
Date: Thu, 01 Feb 2018 17:16:08 +0000 (UTC)

I can't find my comment, but I thought it was weird to take an RTT sample of one path in one direction and a different path in another, which is what happens if PATH_RESPONSE comes back on a different path from the one PATH_CHALLENGE is sent on.

-- 
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/1086#discussion_r165425493

---

From: ianswett <[email-masked]>
Date: Thu, 01 Feb 2018 17:22:58 +0000 (UTC)

> @@ -2692,10 +2708,19 @@ implementation decision, and an implementation should be careful to delay
 
-Regular QUIC packets are "containers" of frames; a packet is never retransmitted
-whole.  How an endpoint handles the loss of the frame depends on the type of the
-frame.  Some frames are simply retransmitted, some have their contents moved to
-new frames, and others are never retransmitted.
+Regular QUIC packets are "containers" of frames.  When an endpoint receives an
+ACK frame for one or more transmitted packets, all frames in the acknowledged
+packets are considered to have been received and processed by the peer, with one
+exception.  A PATH_CHALLENGE frame (see {{frame-path-challenge}}) is used to
+validate a peer's ownership of its address.  An ACK frame received for a
+PATH_CHALLENGE frame is not adequate to indicate that the PATH_CHALLENGE was in
+fact received.  A PATH_CHALLENGE is considered acknowledged only when the
+corresponding PATH_RESPONSE (see {{frame-path-response}}) is received for it.

I still think this new text is more confusing than helpful.  You're going to have to write some new text that's more complete than this for connection migration, so I would keep this section as is for now.

-- 
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/1086#pullrequestreview-93365654

---

From: ianswett <[email-masked]>
Date: Thu, 01 Feb 2018 17:23:09 +0000 (UTC)

as is = as it was

-- 
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/1086#discussion_r165427473

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 15:11:58 -0800

Removed offending text. Many things need fixing in migration.

-- 
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/1086#discussion_r165517036

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 23:12:51 +0000 (UTC)

fd34af6  removed offending text about acks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1086/files/5ce614938e8fa3644d08c13a413b182fe599a527..fd34af6df17613257ea3239562331237256bc63d

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 23:14:26 +0000 (UTC)

d7a4c77  f'ing whitespace

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1086/files/fd34af6df17613257ea3239562331237256bc63d..d7a4c77c00dd11265445818c5da4811887faa40e

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 15:17:23 -0800

Merged #1086.

-- 
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/1086#event-1454096397

---

From: janaiyengar <[email-masked]>
Date: Thu, 01 Feb 2018 23:17:00 +0000 (UTC)

I'm going to merge this PR, please feel free to comment on the issue if you want to see any further changes.

-- 
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/1086#issuecomment-362434254