Subject: [quicwg/base-drafts] Ekr editorial 17 3 (#2164)

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 14:12:47 -0800

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * some more editorial work
  * Some more cleanup
  * More editorial work

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Thu, 13 Dec 2018 14:45:44 -0800

mikkelfj commented on this pull request.

> @@ -1533,12 +1533,15 @@ Once a client has received an acknowledgment for a Handshake packet it MAY send
 smaller datagrams.  Sending padded datagrams ensures that the server is not
 overly constrained by the amplification restriction.
 
-In order to prevent a handshake deadlock as a result of the server being unable
-to send, clients SHOULD send a packet upon a handshake timeout, as described in
-{{QUIC-RECOVERY}}.  If the client has no data to retransmit and does not have
-Handshake keys, it SHOULD send an Initial packet in a UDP datagram of at least
-1200 bytes.  If the client has Handshake keys, it SHOULD send a Handshake
-packet.
+Packet loss, e.g., of the server's Handshake packet, can cause a
+situation in which the server cannot send becaus of the

```suggestion
situation in which the server cannot send because of the
```

> @@ -1616,7 +1619,8 @@ Token field of its Initial packet.
 A token allows a server to correlate activity between the connection where the
 token was issued and any connection where it is used.  Clients that want to
 break continuity of identity with a server MAY discard tokens provided using the
-NEW_TOKEN frame.  Tokens obtained in Retry packets MUST NOT be discarded.
+NEW_TOKEN frame.  Tokens obtained in Retry packets MUST NOT be discarded
+during connection establishment (they will not be used with new connections).

How about a positive: A token obtained in a Retry packet must be used immediately during the connection attempt and cannot be used in subsequent connection attempts.

> @@ -1679,9 +1683,10 @@ peer from a new local address.  In path validation, endpoints test reachability
 between a specific local address and a specific peer address, where an address
 is the two-tuple of IP address and port.
 
-Path validation tests that packets can be both sent to and received from a peer
-on the path.  Importantly, it validates that the packets received from the
-migrating endpoint do not carry a spoofed source address.
+Path validation tests that packets (PATH_CHALLENGE) can be both sent
+to and received (PATH_RESPONSE) from a peer on the path.  Importantly,
+it validates that the packets received from the migrating endpoint do
+not carry a spoofed source address.

But that isn't exactly true since MITM can spoof the source address of these packets if it can guess the packet content (as you pointed out).

> @@ -1716,8 +1721,9 @@ loss.  An endpoint SHOULD NOT send a PATH_CHALLENGE more frequently than it
 would an Initial packet, ensuring that connection migration is no more load on a
 new path than establishing a new connection.
 
-The 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.
+The endpoint MUST use unpredictable data in every PATH_CHALLENGE frame
+so that it can associate the peer's response with the causative

```suggestion
so that it can associate the peer's response with the corresponding
```

> @@ -1737,27 +1743,26 @@ to the same remote address from which the PATH_CHALLENGE was received.
 
 ## Successful Path Validation
 
-A new address is considered valid when a PATH_RESPONSE frame is received
-containing data that was sent in a previous PATH_CHALLENGE. Receipt of an
-acknowledgment for a packet containing a PATH_CHALLENGE frame is not adequate
-validation, since the acknowledgment can be spoofed by a malicious peer.
+A new address is considered valid when A PATH_RESPONSE frame is received
+that meets the following criteria:
+
+- It contains that was sent in a previous PATH_CHALLENGE. Receipt of an

```suggestion
- It contains the data that was sent in a previous PATH_CHALLENGE. Receipt of an
```

-For path validation to be successful, a PATH_RESPONSE frame MUST be received
-from the same remote address to which the corresponding PATH_CHALLENGE was
-sent. If a PATH_RESPONSE frame is received from a different remote address than
-the one to which the PATH_CHALLENGE was sent, path validation is considered to
-have failed, even if the data matches that sent in the PATH_CHALLENGE.
+- It is from the same remote address as that to which the
+  corresponding PATH_CHALLENGE was sent. If a PATH_RESPONSE frame is

It was received from the remote address that the corresponding PATH_CHALLENGE was sent to.

-For path validation to be successful, a PATH_RESPONSE frame MUST be received
-from the same remote address to which the corresponding PATH_CHALLENGE was
-sent. If a PATH_RESPONSE frame is received from a different remote address than
-the one to which the PATH_CHALLENGE was sent, path validation is considered to
-have failed, even if the data matches that sent in the PATH_CHALLENGE.
+- It is from the same remote address as that to which the
+  corresponding PATH_CHALLENGE was sent. If a PATH_RESPONSE frame is
+  received from a different remote address than the one to which the
+  PATH_CHALLENGE was sent, path validation is considered to have failed,
+  even if the data matches that sent in the PATH_CHALLENGE.

```suggestion
  even if the PATH_RESPONSE data matches the PATH_CHALLENGE data.
```

-If a PATH_RESPONSE frame is received on a different local address than the one
-from which the PATH_CHALLENGE was sent, path validation is not considered to be
-successful, even if the data matches the PATH_CHALLENGE.  This doesn't result in
-path validation failure, as it might be a result of a forwarded packet (see
-{{off-path-forward}}) or misrouting.
+Note that receipt on a different local address doesn't result in path

```suggestion
Note that receipt on a different local address does not result in path
```

> @@ -2338,14 +2345,15 @@ of the packet header.  The remainder of the first byte and an arbitrary
 number of random bytes following it are set to unpredictable values.  The last
 16 bytes of the datagram contain a Stateless Reset Token.
 
-A stateless reset will be interpreted by a recipient as a packet with a short
-header.  For the packet to appear as valid, the Random Bits field needs to
-include at least 182 bits of random or unpredictable values (or 24 bytes, less
-the two fixed bits).  This is intended to allow for a destination connection ID
-of the maximum length permitted, with a minimal packet number, and payload.  The
-Stateless Reset Token corresponds to the minimum expansion of the packet
-protection AEAD.  More random bytes might be necessary if the endpoint could
-have negotiated a packet protection scheme with a larger minimum AEAD expansion.
+A stateless reset will be interpreted by a recipient as a packet with
+a short header.  For the packet to appear as valid, the Random Bits
+field needs to include at least 182 bits of data (or 24 bytes, less
+the two fixed bits). This is intended to allow for a destination

```suggestion
the two fixed bits). This is intended to allow for a Destination
```

> @@ -2338,14 +2345,15 @@ of the packet header.  The remainder of the first byte and an arbitrary
 number of random bytes following it are set to unpredictable values.  The last
 16 bytes of the datagram contain a Stateless Reset Token.
 
-A stateless reset will be interpreted by a recipient as a packet with a short
-header.  For the packet to appear as valid, the Random Bits field needs to
-include at least 182 bits of random or unpredictable values (or 24 bytes, less
-the two fixed bits).  This is intended to allow for a destination connection ID
-of the maximum length permitted, with a minimal packet number, and payload.  The
-Stateless Reset Token corresponds to the minimum expansion of the packet
-protection AEAD.  More random bytes might be necessary if the endpoint could
-have negotiated a packet protection scheme with a larger minimum AEAD expansion.
+A stateless reset will be interpreted by a recipient as a packet with
+a short header.  For the packet to appear as valid, the Random Bits
+field needs to include at least 182 bits of data (or 24 bytes, less
+the two fixed bits). This is intended to allow for a destination
+connection ID of the maximum length permitted, with a minimal packet

```suggestion
Connection ID of the maximum length permitted, with a minimal packet
```

> @@ -4931,10 +4943,10 @@ Frame Type:
 
 Reason Phrase Length:
 
-: A variable-length integer specifying the length of the reason phrase in bytes.
-  Note that a CONNECTION_CLOSE frame cannot be split between packets, so in
-  practice any limits on packet size will also limit the space available for a
-  reason phrase.
+: A variable-length integer specifying the length of the reason phrase
+  in bytes.  Because CONNECTION_CLOSE frame cannot be split between

```suggestion
  in bytes.  Because a CONNECTION_CLOSE frame cannot be split between
```

-- 
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/2164#pullrequestreview-184895292

---

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 14:47:37 -0800

@ekr pushed 1 commit.

ade5b402feeaba56872862fe66ee12d62b560cc5  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2164/files/841e8c39b02e588cb2bf2f82d750727787860d00..ade5b402feeaba56872862fe66ee12d62b560cc5

---

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 22:50:29 +0000 (UTC)

@ekr pushed 1 commit.

cc14d2437e9f5beb357fd718e869fcc6f4413289  Apply suggestions from code review

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

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 22:51:35 +0000 (UTC)

MikeBishop commented on this pull request.

Mostly improvements or neutral.  As a general comment, you appear to be rewrapping your changed paragraphs to a different width than we've been using for these documents, which makes more appear to have changed than actually did.

These are not packets.

 

I think you a word out here.

> @@ -2294,7 +2301,7 @@ coalesced (see {{packet-coalesce}}) to facilitate retransmission.
 
 ## Stateless Reset {#stateless-reset}
 
-A stateless reset is provided as an option of last resort for an endpoint that
+A stateless reset is provided as an option of last resort for a server that

No, SR is bilateral, even if we expect servers to use them more frequently.

I appreciate that you're removing the suggestion that these should be "random" despite the name of the field.  However, it seems worthwhile to keep "unpredictable" at least.

> @@ -2447,9 +2455,11 @@ Note that Stateless Reset packets do not have any cryptographic protection.
 
 ### Looping {#reset-looping}
 
-The design of a Stateless Reset is such that it is indistinguishable from a
-valid packet.  This means that a Stateless Reset might trigger the sending of a
-Stateless Reset in response, which could lead to infinite exchanges.
+The design of a Stateless Reset is such that without knowing the
+stateless reset token it is indistinguishable from a valid packet.
+This means that if a server sends a Stateless Reset to another server,
+that might trigger the sending of a Stateless Reset in response, which

Can we get rid of one of these "that"s?

> @@ -3732,9 +3743,9 @@ ID that is chosen by the recipient of the packet; the Source Connection ID
 includes the connection ID that the sender of the packet wishes to use (see
 {{negotiating-connection-ids}}).
 
-The first Handshake packet sent by a server contains a packet number of 0.
-Handshake packets are their own packet number space.  Packet numbers are
-incremented normally for other Handshake packets.
+

Extra line seems unnecessary here.

> @@ -3732,9 +3743,9 @@ ID that is chosen by the recipient of the packet; the Source Connection ID
 includes the connection ID that the sender of the packet wishes to use (see
 {{negotiating-connection-ids}}).
 
-The first Handshake packet sent by a server contains a packet number of 0.
-Handshake packets are their own packet number space.  Packet numbers are
-incremented normally for other Handshake packets.
+
+Handshake packets are their own packet number space, and thus
+the first Handshake packet sent by a server contains a packet number of 0.

Is this a requirement?  Last I knew, some implementations were choosing to just keep a counter incrementing across all packet spaces, and that was okay.  The key point is that (Initial,X) and (Handshake,X) are different packets, but IIRC there's no requirement that each space start at zero.

> @@ -3847,10 +3858,11 @@ MUST include the value of the Original Destination Connection ID field of the
 Retry packet (that is, the Destination Connection ID field from the client's
 first Initial packet) in the transport parameter.
 
-If the client received and processed a Retry packet, it validates that the
-original_connection_id transport parameter is present and correct; otherwise, it
-validates that the transport parameter is absent.  A client MUST treat a failed
-validation as a connection error of type TRANSPORT_PARAMETER_ERROR.
+If the client received and processed a Retry packet, it MUST validate
+that the original_connection_id transport parameter is present and
+correct; otherwise, it validates that the transport parameter is
+absent.  A client MUST treat a failed validation as a connection error
+of type TRANSPORT_PARAMETER_ERROR.

Adding the MUST to the validate-is-present fork seems unnecessary, but if you're going to, isn't it also a MUST to validate-is-absent?

-- 
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/2164#pullrequestreview-184901101

---

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 14:59:54 -0800

ekr commented on this pull request.

Sure.. I didn't change this text, I just added the parentheticals to make it clearer. This PR is essentially editorial

I think this text is fine

I think this is fine

Prefer as-is

I'll let MT weigh in on this. This is existing text.

 

"A client MUST NOT include an original connection ID, a stateless reset token, or
a preferred address.  A server MUST treat receipt of any of these transport
parameters as a connection error of type TRANSPORT_PARAMETER_ERROR."

It does appear that a client can send one in NCID, but that's a real inconsistency.

The previous graf dicates the contents. This graf is about how the recipient will interpret them, and the recipient has no good way of knowing if they are random or unpredictable or whatever.

 

feel free to suggest.

"   The CRYPTO frame can be sent in different packet number spaces.  The
   sequence numbers used by CRYPTO frames to ensure ordered delivery of
   cryptographic handshake data start from zero in each packet number
   space.
"

yes I agree. What I'm trying to do is remove language which is apparently descriptive but is actually normative. I.e., "the agent does X". That's a requirement

-- 
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/2164#discussion_r241590333

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 23:06:46 +0000 (UTC)

Our convention is that the term "connection ID" is not capitalized, but the fields named "Connection ID" are.

-- 
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/2164#discussion_r241594596

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 15:11:53 -0800

- OCID is only applicable to servers because it's about Retry
- Preferred Address is only applicable to servers and contains an SRT
- SRT is useless if it's ever exchanged in the clear, which the client's TPs are

After the initial CID, the client can provide an SRT and the behavior is bilateral.  There was a discussion about enabling the client to supply an SRT for the initial CID after the handshake, but there wasn't great appetite for the added complexity.  If the server cares about getting an SRT, it just starts using the next CID as soon as it gets 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/pull/2164#discussion_r241595759

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 23:13:28 +0000 (UTC)

"This means that if a server sends a Stateless Reset to another server, it might receive another Stateless Reset in response.  As a result...."

-- 
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/2164#discussion_r241596071

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 15:14:06 -0800

That's the offset of the CRYPTO stream, not the packet numbers.

-- 
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/2164#discussion_r241596188

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Dec 2018 23:14:52 +0000 (UTC)

Then "otherwise, it MUST validate 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/2164#discussion_r241596360

---

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 15:45:39 -0800

Ugh.

-- 
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/2164#discussion_r241602632

---

From: ekr <[email-masked]>
Date: Thu, 13 Dec 2018 15:47:16 -0800

My mistake. Here:

https://tools.ietf.org/html/draft-ietf-quic-transport-16#section-12.3

" This enforces cryptographic separation between the data sent in the
   different packet sequence number spaces.  Each packet number space
   starts at packet number 0.  Subsequent packets sent in the same
   packet number space MUST increase the packet number by at least 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/pull/2164#discussion_r241602951

---

From: Martin Thomson <[email-masked]>
Date: Thu, 13 Dec 2018 21:19:01 -0800

martinthomson commented on this pull request.

Thanks for these.  Nits only.

What @mikkelfj said.

The parenthetical doesn't really make sense to me - it's true, but I don't see how it is connected to the primary statement.

I agree, this likely confuses things more than it helps.

@janaiyengar insists on avoiding contractions.

> @@ -2205,12 +2210,13 @@ An endpoint is not expected to handle key updates when it is closing or
 draining.  A key update might prevent the endpoint from moving from the closing
 state to draining, but it otherwise has no impact.
 
-An endpoint could receive packets from a new source address, indicating a client
-connection migration ({{migration}}), while in the closing period. An endpoint
-in the closing state MUST strictly limit the number of packets it sends to this
-new address until the address is validated (see {{migrate-validate}}). A server
-in the closing state MAY instead choose to discard packets received from a new
-source address.
+While in the closing period, an endpoint could receive packets from a
+new source address, indicating a client connection migration
+({{migration}}). An endpoint in the closing state MUST strictly limit
+the number of packets it sends to this new address until the address
+is validated (see {{migrate-validate}}). A server in the closing state
+MAY instead choose to discard packets received from a new source
+address.

The change to the fill-column is making this harder to review than I'd like.  I guess that's just a consequence of insisting on wrapping at <80...

 

Yep :)

This is a field, so the capitalization is correct.

 

One fewer: "If a server sends a Stateless Reset to another server, it might receive another Stateless Reset in response."

```suggestion
```

I think that we're operating on the assumption that you start sending from 0, but there is no strict requirement because it's unenforceable.  Loss.

I'm OK with the existing or as Mike suggests.

> @@ -5326,7 +5338,7 @@ An accompanying transport parameter registration (see
 specification needs to describe the format and assigned semantics of any fields
 in the frame.
 
-Expert(s) are encouraged to be biased towards approving registrations unless
+Expert(s) SHOULD be biased towards approving registrations unless

Not really an interoperability requirement.

```suggestion
Expert(s) are encouraged to be biased towards approving registrations unless
```

-- 
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/2164#pullrequestreview-184970263

---

From: Mike Bishop <[email-masked]>
Date: Fri, 14 Dec 2018 16:28:22 -0800

@MikeBishop pushed 1 commit.

5f6331f9ca01eb91aa50dc73263dbb92154c24e4  Rewrap

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

---

From: Mike Bishop <[email-masked]>
Date: Fri, 14 Dec 2018 16:30:54 -0800

I tried rewrapping it for him, but that makes all the comments outdated, unfortunately.  I think this will need a rewrap just before merge instead, unfortunately.

-- 
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/2164#discussion_r241927225

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:31:50 -0800

janaiyengar commented on this pull request.

+1. I do.

-- 
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/2164#discussion_r241927305

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:35:56 -0800

```suggestion
correct; otherwise, it MUST validate that the transport parameter is
```

-- 
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/2164#pullrequestreview-185330874

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:36:23 -0800

Hah, yes. Suggested above.

-- 
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/2164#discussion_r241927749

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:38:30 -0800

```suggestion
A new address is considered valid when a PATH_RESPONSE frame is received
```

-- 
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/2164#pullrequestreview-185331186

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:39:46 -0800

```suggestion
- It is sent from the same remote address as that to which 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/2164#pullrequestreview-185331338

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 16:44:28 -0800

It'd all be a lot simpler if we just went with client and server :-)

-- 
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/2164#discussion_r241928516

---

From: ekr <[email-masked]>
Date: Sat, 15 Dec 2018 06:41:33 -0800

@ekr pushed 1 commit.

ceba5d15a5563b87494dc3a7cced575ec071d0d8  Review comments

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 22:13:05 -0800

Merged manually.

-- 
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/2164#issuecomment-448481549

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 22:13:07 -0800

Closed #2164.

-- 
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/2164#event-2034823812