Introduces PATH_CHALLENGE and PATH_RESPONSE frames #1086

janaiyengar · 2018-01-31T02:18:04Z

Replaces PING with Data and PONG frames with PATH_CHALLENGE and PATH_RESPONSE frames.

Closes #1091.

ianswett

One suggestion, otherwise LGTM

ianswett · 2018-01-31T03:12:04Z

draft-ietf-quic-transport.md

+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+A PATH_CHALLENGE frame MUST NOT elicit acknowledgements; the corresponding


Are we clear on the meaning of elicit here? We definitely intend for the packet containing the PATH_CHALLENGE frame to be acknowledged. Given that, I'd be inclined to just consider it a frame we acknowledge and remove this paragraph.

I'm with Ian here. I think that we can treat this frame as normal. I can see how you might want to exclude PATH_RESPONSE from acknowledgment, but even there I would just let the ACKs flow.

I didn't think that we previously intended a packet containing only a PATH_CHALLENGE frame to be acknowledged, but I think that could make sense.

Minor question though:
What if we get a PATH_RESPONSE, but no ACK? Do we then retransmit the PATH_CHALLENGE?

The previous comment about the packets potentially being sent on (or taking) different paths is interesting, because we could ACK along the current path, but send the PATH_RESPONSE along the same path the PATH_CHALLENGE came in on. This could provide valuable information to the endpoint about the viability of those two paths, although we still end up in the situation of a rapidly failing path causing the ACK to not be delivered, which is not the end of the world.

Also, given that we stated that a new transmission of a PATH_CHALLENGE frame due to a lack of a PATH_RESPONSE should include different random data, how does that interact with loss recovery due to ACKs being present/not being present?

(In other words, I can see an argument for either behavior, but we should make sure that we update below if we change this.)

I think we should always acknowledge all packets. I wouldn't be upset about including PATH_CHALLENGE/PATH_RESPONSE along with ACK in the context of "packets containing only ACKs don't trigger the sending of a ACK immediately, but are included the next time an ACK is generated."

I'll change the text, but the point here was that an ACK frame is basically useless for acknowledging receipt of a PATH_CHALLENGE frame. The entire point of this frame is to require an echo of the random bytes from the peer... if an ACK were adequate, a PING frame is enough. I don't think it matters whether the peer acks the frame or not, since the endpoint shouldn't use the received ACK as an acknowledgment. This problem exists with PING+Data as well, it just isn't stated.

I'll change this text.

martinthomson

This is definitely a much cleaner design than overloading PING as I did.

martinthomson · 2018-01-31T03:39:50Z

draft-ietf-quic-transport.md

+is received, the address is considered to be valid.  The PATH_RESPONSE frame can
+use any path on its return.  A PATH_CHALLENGE 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.


I think that we can remove this last sentence now that PATH_CHALLENGE is fixed size. FWIW, I think that we could drop back to 8 happily. 12 raised some eyebrows, and the handshake uses fewer octets. That said, 12 means that you could include 8 octets of randomness and a timestamp - or a timestamp and an 8 octet HMAC - so that you don't have to maintain state and you can still measure RTT.

Fixed 8 bytes SGTM.

I don't have an opinion on size at all. I'll make it 8 bytes to avoid confusion... also because I don't think stateless operation is much of a benefit here.

You can still do stateless in 8 bytes, I guess. It's just a weaker authenticator. (I think that stateless is a good strategy actually.)

martinthomson · 2018-01-31T03:42:00Z

draft-ietf-quic-transport.md

+
+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,
+additionally helping the endpoint make more accurate path measurements.


Split the last clause here into a new sentence: Using fresh values allows the endpoint to measure the time taken to respond to a challenge.

martinthomson · 2018-01-31T03:43:18Z

draft-ietf-quic-transport.md

+additionally helping the endpoint make more accurate path measurements.
+
+If the PATH_CHALLENGE frame is determined to be lost, a new PATH_CHALLENGE frame
+SHOULD be generated.  This PATH_CHALLENGE frame MUST include new data that is


Rather than SHOULD here, I think that you need to talk about sending multiple to deal with the potential of loss, but limiting the total time or number of challenges and eventually giving up.

This text is the same as previous, with PING+Data. I intend to change this in the PR that addresses other migration concerns, but not here.

martinthomson · 2018-01-31T03:45:34Z

draft-ietf-quic-transport.md


-## PONG Frame {#frame-pong}
+Endpoints can use PATH_CHALLENGE frames (type=0x0e) to check reachability to the
+peer, to verify a new path's PMTU, and for address validation during connection


I think that you should drop the PMTU bit here. It's the packet size that determines PMTU validity anyway.

The point about PMTU is that you can send multiple PATH_CHALLENGEs on the new path in packets of different sizes and see which ones get through. However, I'd recommend putting that in a section on probing for PMTU than making an offhand reference here.

martinthomson · 2018-01-31T03:47:11Z

draft-ietf-quic-transport.md


-The PONG frame (type=0x0d) is sent in response to a PING frame that contains
-data.  Its format is identical to the PING frame ({{frame-ping}}).
+PATH_CHALLENGE frames contain a variable-length payload.


A fixed size is better. No chance of messing it up that way. As I said above, I think that 12 octets is pretty good (that's the size of the STUN transaction ID).

+1 for fixed size.

martinthomson · 2018-01-31T03:47:52Z

draft-ietf-quic-transport.md

-endpoint, the endpoint MAY generate a connection error of type UNSOLICITED_PONG.
+
+The sender of this frame MUST include at least one octet of data in the Data
+field.


To the above, one octet is of almost no value. You would need many PATH_RESPONSE frames.

martinthomson · 2018-01-31T03:50:09Z

draft-ietf-quic-transport.md

+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).


When? If the secondary value of PATH_CHALLENGE is to be realized, this should be done immediately. Or are we expecting the ACK frame to include ACK Delay and we expect challengers to use that to correct their view of path delay? With the potential for the ACK or response to follow a different path, it's hard to know how reliable this will be for measuring path RTT.

If we require the PATH_RESPONSE to be sent on the same path as the PATH_CHALLENGE, do we place any requirements on the ACK? And does such a requirement make sense?

The previous text says that the PATH_RESPONSE can be sent on any path, so currently we don't.

We should require PATH_RESPONSE to be sent to the same address as the PATH_CHALLENGE, though I'm open to arguments as to why not. That said, I'm not fixing migration bugs in this PR, just changing the PING+Data frame.

I think that it would be better to keep the special handling to a minimum. It is easier to generate a PATH_RESPONSE and send on any path than require use of the same path. That limits the special logic to the entity that is probing. I'm happy to keep this simple - you ACK these frames as normal and you generate the response where ever and whenever it is easiest to do so.

Again, not for this PR, but the idea would be to demonstrate two way reachability using PATH_CHALLENGE and PATH_RESPONSE. Otherwise, you may end up in a situation where, for instance, UDP packets are blocked on one direction, but migration happens anyways since PATH_RESPONSE was sent from an address which didn't block UDP packets.

martinthomson · 2018-01-31T03:50:56Z

draft-ietf-quic-transport.md

+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+A PATH_CHALLENGE frame MUST NOT elicit acknowledgements; the corresponding


I'm with Ian here. I think that we can treat this frame as normal. I can see how you might want to exclude PATH_RESPONSE from acknowledgment, but even there I would just let the ACKs flow.

martinthomson · 2018-01-31T03:51:43Z

draft-ietf-quic-transport.md

+## Since draft-ietf-quic-transport-09
+
+- Added PATH_CHALLENGE and PATH_RESPONSE frames to replace PING with Data and
+  PONG frame. Changed ACK frame type from 0x0e to 0x0d. (#000)


Can you open an issue for this, so we can track it?

Yes, good point. Filed #1091.

erickinnear

A few minor nits, but otherwise looks good!
Thanks for putting this together.

erickinnear · 2018-01-31T09:05:04Z

draft-ietf-quic-transport.md

-~~~
-
-Length:
+The receiver of a PING frame simply need to acknowledge the packet containing


Nit: need -> needs

erickinnear · 2018-01-31T09:06:59Z

draft-ietf-quic-transport.md

@@ -2352,17 +2337,56 @@ Application Error Code:
  {{app-error-codes}}).


+## PATH_CHALLENGE Frame {#frame-path-challenge}


Should these fall after ACK frames in terms of ordering?

I don't believe we have specified packet ordering if that's what you're referring to, so I'd prefer not to start now.

Ah no, sorry, this is much more minor: I'm referring to ordering within the document itself (although that might not be a concern at this point), which no longer matches the table above.

We've already done one reorder of the document to match the table -- we'll probably need to do another as part of the grand editorial pass being discussed in a few months.

erickinnear · 2018-01-31T09:11:27Z

draft-ietf-quic-transport.md

+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+A PATH_CHALLENGE frame MUST NOT elicit acknowledgements; the corresponding


I didn't think that we previously intended a packet containing only a PATH_CHALLENGE frame to be acknowledged, but I think that could make sense.

Minor question though:
What if we get a PATH_RESPONSE, but no ACK? Do we then retransmit the PATH_CHALLENGE?

The previous comment about the packets potentially being sent on (or taking) different paths is interesting, because we could ACK along the current path, but send the PATH_RESPONSE along the same path the PATH_CHALLENGE came in on. This could provide valuable information to the endpoint about the viability of those two paths, although we still end up in the situation of a rapidly failing path causing the ACK to not be delivered, which is not the end of the world.

erickinnear · 2018-01-31T09:12:20Z

draft-ietf-quic-transport.md

+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).


If we require the PATH_RESPONSE to be sent on the same path as the PATH_CHALLENGE, do we place any requirements on the ACK? And does such a requirement make sense?

erickinnear · 2018-01-31T09:15:09Z

draft-ietf-quic-transport.md

+the content of a PATH_CHALLENGE frame previously sent by the endpoint, the
+endpoint MAY generate a connection error of type UNSOLICITED_PATH_RESPONSE.
+
+A PATH_RESPONSE frame MUST NOT elicit an acknowledgement.


 ## ACK Frame {#frame-ack}


Minor: This is now type 0x0d

Not minor -- you can create chaos doing this. Ask me how I know. 😉

yup, this would not be happy. fixed.

erickinnear · 2018-01-31T09:18:22Z

draft-ietf-quic-transport.md

+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+A PATH_CHALLENGE frame MUST NOT elicit acknowledgements; the corresponding


Also, given that we stated that a new transmission of a PATH_CHALLENGE frame due to a lack of a PATH_RESPONSE should include different random data, how does that interact with loss recovery due to ACKs being present/not being present?

(In other words, I can see an argument for either behavior, but we should make sure that we update below if we change this.)

nibanks

Looks pretty good. Just a few comments/questions.

nibanks · 2018-01-31T16:39:19Z

draft-ietf-quic-transport.md

-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


Why should the connection be terminated if the response wasn't received? What if the new path was just being interrogated for viability and it just wasn't viable (too much loss, blocked in one direction or got disconnected)? Should that really cause the good path to be killed as well?

This is specific to address validation, I think.
So, if you're just probing new paths for viability then you're not doing this and it's totally fine to consider those packets lost and move on.
If you're doing address validation and the remote side cannot prove that it actually owns that address, we need to not send it anything more.

In practice, this is determined by what role you're playing (are you the one changing addresses or are you the one seeing the remote endpoint change).

Let's imagine a client is on path A and probes path B with a challenge. The server receives this challenge and then responds with the response and a challenge of its own. Then, for some reason, that packet never makes it (too much loss, blocked in that direction, or client got disconnected). The both sides won't receive their challenge's response. The current spec text indicates both sides would kill the connection on path A now. I am just questioning if that is the best course of action.

This should probably be clarified in the text: I read this as only the server would be performing address validation for this example (since the client isn't seeing the server's address change) and therefore would be the only one required to kill the connection if the client cannot demonstrate that it owns the new address.

In the scenario you mentioned, the client should be sending additional PATH_CHALLENGES when it doesn't get the response (and I think it's possible and probably a good idea for a server implementation to send a new one as well instead of waiting for the client to retransmit, if it wishes).

I agree with your general point, however -- I think we should clarify that the server must not send additional data to the new client's address rather than closing the connection as a whole. Otherwise I can trivially get the server to close your connection just by failing address validation on your behalf.

I think the language should be around not considering that path viable. If that was your only path, that means you'll have to kill the connection, yes. If you think you still have the old path but you're wrong, you'll discover that via timeout soon enough.

The strategy that I'd like to suggest is to go back to the last best path. That said, this text is the same as it was previously with PING+Data and I'd like to not change migration behavior. I'd like to change this behavior but in a different PR that fixes migration. This PR simply replaces PING+Data / PONG with new frames.

Yes, let's just keep this constrained to replacement as much as possible.

nibanks · 2018-01-31T16:40:47Z

draft-ietf-quic-transport.md


-The PONG frame (type=0x0d) is sent in response to a PING frame that contains
-data.  Its format is identical to the PING frame ({{frame-ping}}).
+PATH_CHALLENGE frames contain a variable-length payload.


+1 for fixed size.

nibanks · 2018-01-31T16:45:01Z

draft-ietf-quic-transport.md

-: An endpoint received a PONG frame that did not correspond to any PING frame
-  that it previously sent.
+: An endpoint received a PATH_RESPONSE frame that did not correspond to any
+  PATH_CHALLENGE frame that it previously sent.


Do we really need to error out if a response is received for a challenge we don't have saved at the moment? What if the sender of the challenge ended up timing out the packet too aggressively and threw away its state for it? If we really need to reliability validate this, how long would the sender of the challenge need to keep this state around then?

Defining the error code doesn't create a mandate to enforce and send it. Implementations that know they have accounting for every PATH_CHALLENGE they've ever sent (e.g. because they've never sent any) can enforce this, while implementations which have discarded state can let an unknown PATH_RESPONSE pass without comment.

mikkelfj · 2018-01-31T17:44:44Z

I must be missing something fundamental but ...

What is the real gain of a path challenge compared to just sending a packet on the desired path with an empty ping on a new path and see if an ACK gets through? Is to to enforce the ACK (aka path response) to follow the same path, and if so, I don't see any requirement that the path response is sent on a particular path?

Why must must the challenge be hard to guess if it happens on an encrypted channel? It would seem that the packet number with an auth tag would be sufficient, unless challenges can happen early on stream 0, which I don't think is the intention.

Is there a risk that the recipient is not cooperating, and hence the need for something unguessable? This then leans up against optimitistic ACK attacks.

Could this challenge thing be integrated with PMTU probing, considering that you need that knowledge to use the path, of course starting with the minimum size probe?

nibanks · 2018-01-31T17:47:52Z

@mikkelfj As I understand it, the challenge response doesn't have to be on a particular path. The reason it needs to be hard to guess is because you are doing source address validation. You are trying to make sure the client isn't trying to dump traffic on some unsuspecting IP. If the client can guess the challenge and reply with the appropriate response on its own path, it could make the server think it does own that other IP, and start sending traffic to it.

mikkelfj · 2018-01-31T17:49:31Z

OK, so the fundamental thing I was missing (which dawned after hitting return of course), is that it is not necessarily the client proping for a new path, but the server not trusting the peer to be on the path that it claims to be. So the sending a challenge is already following a return path.

@nibanks explained this while I was writing the above.

MikeBishop

Generally fine.

MikeBishop · 2018-01-31T18:49:26Z

draft-ietf-quic-transport.md

-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


I think the language should be around not considering that path viable. If that was your only path, that means you'll have to kill the connection, yes. If you think you still have the old path but you're wrong, you'll discover that via timeout soon enough.

MikeBishop · 2018-01-31T18:50:43Z

draft-ietf-quic-transport.md

+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'd question this piece, too. Rather than "does not want," the implementation should be assuming that the other side cannot receive any more packets on the path being probed. If you have another path, sending a CONNECTION_CLOSE or even maintaining the connection seems quite reasonable.

Agreed, again not changing text here for what the draft currently states, since I'd like to simply get rid of PING+Data for now. This will change with fixing the rest of migration.

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

MikeBishop · 2018-01-31T18:52:15Z

draft-ietf-quic-transport.md

+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.


How about a recommendation to probe both old and new addresses to guard against this?

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

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

MikeBishop · 2018-01-31T18:54:38Z

draft-ietf-quic-transport.md

@@ -2352,17 +2337,56 @@ Application Error Code:
  {{app-error-codes}}).


+## PATH_CHALLENGE Frame {#frame-path-challenge}


We've already done one reorder of the document to match the table -- we'll probably need to do another as part of the grand editorial pass being discussed in a few months.

MikeBishop · 2018-01-31T18:56:03Z

draft-ietf-quic-transport.md


-## PONG Frame {#frame-pong}
+Endpoints can use PATH_CHALLENGE frames (type=0x0e) to check reachability to the
+peer, to verify a new path's PMTU, and for address validation during connection


The point about PMTU is that you can send multiple PATH_CHALLENGEs on the new path in packets of different sizes and see which ones get through. However, I'd recommend putting that in a section on probing for PMTU than making an offhand reference here.

MikeBishop · 2018-01-31T18:57:46Z

draft-ietf-quic-transport.md

+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).


The previous text says that the PATH_RESPONSE can be sent on any path, so currently we don't.

MikeBishop · 2018-01-31T19:00:24Z

draft-ietf-quic-transport.md

+PATH_CHALLENGE frame containing an empty payload MUST generate a connection
+error of type FRAME_ERROR, indicating the PATH_CHALLENGE frame (that is, 0x0e).
+
+A PATH_CHALLENGE frame MUST NOT elicit acknowledgements; the corresponding


I think we should always acknowledge all packets. I wouldn't be upset about including PATH_CHALLENGE/PATH_RESPONSE along with ACK in the context of "packets containing only ACKs don't trigger the sending of a ACK immediately, but are included the next time an ACK is generated."

MikeBishop · 2018-01-31T19:01:29Z

draft-ietf-quic-transport.md

+the content of a PATH_CHALLENGE frame previously sent by the endpoint, the
+endpoint MAY generate a connection error of type UNSOLICITED_PATH_RESPONSE.
+
+A PATH_RESPONSE frame MUST NOT elicit an acknowledgement.


 ## ACK Frame {#frame-ack}


Not minor -- you can create chaos doing this. Ask me how I know. 😉

MikeBishop · 2018-01-31T19:03:57Z

draft-ietf-quic-transport.md

-: An endpoint received a PONG frame that did not correspond to any PING frame
-  that it previously sent.
+: An endpoint received a PATH_RESPONSE frame that did not correspond to any
+  PATH_CHALLENGE frame that it previously sent.


Defining the error code doesn't create a mandate to enforce and send it. Implementations that know they have accounting for every PATH_CHALLENGE they've ever sent (e.g. because they've never sent any) can enforce this, while implementations which have discarded state can let an unknown PATH_RESPONSE pass without comment.

janaiyengar

High-level point: The goal of this PR is to replace PING+Data with new frames, to avoid overloaded semantics. I'd like to not fix migration bugs here, since I'd like to fix them in a separate PR (upcoming).

janaiyengar · 2018-02-01T00:01:18Z

draft-ietf-quic-transport.md

+is received, the address is considered to be valid.  The PATH_RESPONSE frame can
+use any path on its return.  A PATH_CHALLENGE 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.


I don't have an opinion on size at all. I'll make it 8 bytes to avoid confusion... also because I don't think stateless operation is much of a benefit here.

janaiyengar · 2018-02-01T00:54:14Z

draft-ietf-quic-transport.md

+
+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,
+additionally helping the endpoint make more accurate path measurements.


janaiyengar · 2018-02-01T01:10:07Z

draft-ietf-quic-transport.md

+additionally helping the endpoint make more accurate path measurements.
+
+If the PATH_CHALLENGE frame is determined to be lost, a new PATH_CHALLENGE frame
+SHOULD be generated.  This PATH_CHALLENGE frame MUST include new data that is


This text is the same as previous, with PING+Data. I intend to change this in the PR that addresses other migration concerns, but not here.

janaiyengar · 2018-02-01T01:13:03Z

draft-ietf-quic-transport.md

-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


The strategy that I'd like to suggest is to go back to the last best path. That said, this text is the same as it was previously with PING+Data and I'd like to not change migration behavior. I'd like to change this behavior but in a different PR that fixes migration. This PR simply replaces PING+Data / PONG with new frames.

janaiyengar · 2018-02-01T01:13:34Z

draft-ietf-quic-transport.md

+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.


Agreed, again not changing text here for what the draft currently states, since I'd like to simply get rid of PING+Data for now. This will change with fixing the rest of migration.

janaiyengar · 2018-02-01T01:16:23Z

draft-ietf-quic-transport.md

@@ -2352,17 +2337,56 @@ Application Error Code:
  {{app-error-codes}}).


+## PATH_CHALLENGE Frame {#frame-path-challenge}


janaiyengar · 2018-02-01T01:17:08Z

draft-ietf-quic-transport.md


-## PONG Frame {#frame-pong}
+Endpoints can use PATH_CHALLENGE frames (type=0x0e) to check reachability to the
+peer, to verify a new path's PMTU, and for address validation during connection


janaiyengar · 2018-02-01T01:19:40Z

draft-ietf-quic-transport.md

+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).


We should require PATH_RESPONSE to be sent to the same address as the PATH_CHALLENGE, though I'm open to arguments as to why not. That said, I'm not fixing migration bugs in this PR, just changing the PING+Data frame.

janaiyengar · 2018-02-01T01:22:12Z

draft-ietf-quic-transport.md

+the content of a PATH_CHALLENGE frame previously sent by the endpoint, the
+endpoint MAY generate a connection error of type UNSOLICITED_PATH_RESPONSE.
+
+A PATH_RESPONSE frame MUST NOT elicit an acknowledgement.


 ## ACK Frame {#frame-ack}


yup, this would not be happy. fixed.

janaiyengar · 2018-02-01T01:25:53Z

draft-ietf-quic-transport.md

+## Since draft-ietf-quic-transport-09
+
+- Added PATH_CHALLENGE and PATH_RESPONSE frames to replace PING with Data and
+  PONG frame. Changed ACK frame type from 0x0e to 0x0d. (#000)


Yes, good point. Filed #1091.

martinthomson · 2018-02-01T01:30:54Z

draft-ietf-quic-transport.md

-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


Yes, let's just keep this constrained to replacement as much as possible.

martinthomson · 2018-02-01T01:34:13Z

draft-ietf-quic-transport.md

+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).


I think that it would be better to keep the special handling to a minimum. It is easier to generate a PATH_RESPONSE and send on any path than require use of the same path. That limits the special logic to the entity that is probing. I'm happy to keep this simple - you ACK these frames as normal and you generate the response where ever and whenever it is easiest to do so.

martinthomson · 2018-02-01T01:36:40Z

draft-ietf-quic-transport.md

+
+: This 8-byte field contains arbitrary data.
+
+A PATH_CHALLENGE frame containing at least 8 randomly generated {{?RFC4086}}


I think that we're making this too hard. Can we just say hard to guess? How they are generated can be left to the challenger. (If TCP is our benchmark, then 32 bits of entropy is probably enough.)

SGTM. I kept futzing around with saying the right thing, and it's probably adequate to say "hard to guess". Done.

ianswett

Some suggested text on validation and acking.

ianswett · 2018-02-01T15:32:38Z

draft-ietf-quic-transport.md

+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.

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.

ianswett · 2018-02-01T15:34:34Z

draft-ietf-quic-transport.md

+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.

ianswett · 2018-02-01T15:35:54Z

draft-ietf-quic-transport.md

+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.

ianswett · 2018-02-01T15:38:08Z

draft-ietf-quic-transport.md

+
+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

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

ianswett · 2018-02-01T15:38:56Z

draft-ietf-quic-transport.md

+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

ianswett · 2018-02-01T15:40:21Z

draft-ietf-quic-transport.md

-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

ianswett · 2018-02-01T15:42:35Z

draft-ietf-quic-transport.md

+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.

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.

ianswett · 2018-02-01T15:45:28Z

draft-ietf-quic-transport.md

-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)

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

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.

ianswett · 2018-02-01T17:22:53Z

draft-ietf-quic-transport.md

+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.

as is = as it was

Removed offending text. Many things need fixing in migration.

janaiyengar · 2018-02-01T23:16:57Z

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

Use PATH_CHALLENGE and PATH_RESPONSE frames.

d14922c

janaiyengar requested review from martinthomson and MikeBishop January 31, 2018 02:18

janaiyengar changed the title ~~Introduce PATH_CHALLENGE and PATH_RESPONSE frames~~ Introduces PATH_CHALLENGE and PATH_RESPONSE frames Jan 31, 2018

ianswett reviewed Jan 31, 2018

View reviewed changes

martinthomson requested changes Jan 31, 2018

View reviewed changes

erickinnear reviewed Jan 31, 2018

View reviewed changes

nibanks reviewed Jan 31, 2018

View reviewed changes

MikeBishop reviewed Jan 31, 2018

View reviewed changes

MikeBishop mentioned this pull request Feb 1, 2018

Mitigate amplification attacks during handshake #1090

Merged

janaiyengar commented Feb 1, 2018

View reviewed changes

responses to comments

9cc0cab

martinthomson approved these changes Feb 1, 2018

View reviewed changes

Jana Iyengar added 3 commits January 31, 2018 17:51

not rtt, response time

b96bd5d

hard to guess

5c3c0d6

fix broken refs

4c470d8

ianswett reviewed Feb 1, 2018

View reviewed changes

ian's comments

5ce6149

ianswett reviewed Feb 1, 2018

View reviewed changes

Jana Iyengar added 2 commits February 1, 2018 15:12

removed offending text about acks

fd34af6

f'ing whitespace

d7a4c77

janaiyengar merged commit c35948b into master Feb 1, 2018

marten-seemann mentioned this pull request Apr 22, 2018

change ACK frame type byte quic-go/quic-go#1314

Closed

martinthomson deleted the path-challenge branch May 29, 2018 02:52

		@@ -2352,17 +2337,56 @@ Application Error Code:
		{{app-error-codes}}).


		## PATH_CHALLENGE Frame {#frame-path-challenge}


		: This 8-byte field contains arbitrary data.

		A PATH_CHALLENGE frame containing at least 8 randomly generated {{?RFC4086}}

Introduces PATH_CHALLENGE and PATH_RESPONSE frames #1086

Introduces PATH_CHALLENGE and PATH_RESPONSE frames #1086

Conversation

janaiyengar commented Jan 31, 2018 • edited Loading

ianswett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erickinnear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nibanks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nibanks Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nibanks Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikkelfj commented Jan 31, 2018

nibanks commented Jan 31, 2018

mikkelfj commented Jan 31, 2018

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janaiyengar commented Jan 31, 2018 •

edited

Loading

nibanks Jan 31, 2018 •

edited

Loading

nibanks Jan 31, 2018 •

edited

Loading