Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ekr editorial 17 3 #2164

Closed
wants to merge 6 commits into from
Closed

Ekr editorial 17 3 #2164

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ef973fe
some more editorial work
ekr Dec 12, 2018
1dcac01
Some more cleanup
ekr Dec 13, 2018
841e8c3
More editorial work
ekr Dec 13, 2018
ade5b40
Update draft-ietf-quic-transport.md
mikkelfj Dec 13, 2018
cc14d24
Apply suggestions from code review
mikkelfj Dec 13, 2018
ceba5d1
Review comments
ekr Dec 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
193 changes: 102 additions & 91 deletions draft-ietf-quic-transport.md
View file
Open in desktop
Expand Up @@ -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 because of the
anti-amplification limit and the client has no data to send. In order
to prevent a handshake deadlock as a result of this situation, 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.

A server might wish to validate the client address before starting the
cryptographic handshake. QUIC uses a token in the Initial packet to provide
Expand Down Expand Up @@ -1600,14 +1603,14 @@ carry the expected token.

Unlike the token that is created for a Retry packet, there might be some time
between when the token is created and when the token is subsequently used.
Thus, a resumption token SHOULD include an expiration time. The server MAY
Thus, a token SHOULD include an expiration time. The server MAY
include either an explicit expiration time or an issued timestamp and
dynamically calculate the expiration time. It is also unlikely that the client
port number is the same on two different connections; validating the port is
therefore unlikely to be successful.

A resumption token SHOULD be constructed to be easily distinguishable from
tokens that are sent in Retry packets as they are carried in the same field.
A token SHOULD be constructed to be easily distinguishable from tokens
that are sent in Retry packets as they are carried in the same field.

If the client has a token received in a NEW_TOKEN frame on a previous connection
to what it believes to be the same server, it can include that value in the
Expand All @@ -1616,7 +1619,9 @@ 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. A token obtained in a Retry packet must be used immediately
during the connection attempt and cannot be used in subsequent connection
attempts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

A client SHOULD NOT reuse a token in different connections. Reusing a token
allows connections to be linked by entities on the network path
Expand Down Expand Up @@ -1679,9 +1684,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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not packets, and the position of the parenthetical phrase is inconsistent between the two options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

it validates that the packets received from the migrating endpoint do
not carry a spoofed source address.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Path validation can be used at any time by either endpoint. For instance, an
endpoint might check that a peer is still in possession of its address after a
Expand Down Expand Up @@ -1716,8 +1722,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 corresponding
PATH_CHALLENGE.


## Path Validation Responses
Expand All @@ -1738,26 +1745,25 @@ 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.
that meets the following criteria:

- It contains the 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.

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 was sent 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.

Additionally, the PATH_RESPONSE frame MUST be received on the same local address
from which the corresponding PATH_CHALLENGE was sent. An endpoint considers the
path to be valid when a PATH_RESPONSE frame is received on the same path with
the same payload as the PATH_CHALLENGE frame.
- It was received on the same local address from which the corresponding
PATH_CHALLENGE was sent.

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 does not result in path
validation failure, as it might be a result of a forwarded packet (see
{{off-path-forward}}) or misrouting. It is possible that a valid
PATH_RESPONSE might be received in the future.


## Failed Path Validation
Expand Down Expand Up @@ -2205,12 +2211,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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.



## Idle Timeout
Expand Down Expand Up @@ -2256,11 +2263,12 @@ increase the time between packets.

Note:

: Allowing retransmission of a packet contradicts other advice in this document
that recommends the creation of new packet numbers for every packet. Sending
new packet numbers is primarily of advantage to loss recovery and congestion
control, which are not expected to be relevant for a closed connection.
Retransmitting the final packet requires less state.
: Allowing retransmission of a closing packet contradicts other advice
in this document that recommends the creation of new packet numbers
for every packet. Sending new packet numbers is primarily of
advantage to loss recovery and congestion control, which are not
expected to be relevant for a closed connection. Retransmitting the
final packet requires less state.

New packets from unverified addresses could be used to create an amplification
attack (see {{address-validation}}). To avoid this, endpoints MUST either limit
Expand Down Expand Up @@ -2334,18 +2342,19 @@ This design ensures that a stateless reset packet is - to the extent possible -
indistinguishable from a regular packet with a short header.

A stateless reset uses an entire UDP datagram, starting with the first two bits
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.
of the packet header. The remainder of the first byte and an arbitrary number
of bytes following it that 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 data (or 24 bytes, less
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

An endpoint SHOULD NOT send a stateless reset that is significantly larger than
the packet it receives. Endpoints MUST discard packets that are too small to be
Expand Down Expand Up @@ -2447,9 +2456,10 @@ 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. If a server sends a
Stateless Reset to another server, it might receive another Stateless Reset in
response, could lead to infinite exchanges.

An endpoint MUST ensure that every Stateless Reset that it sends is smaller than
the packet which triggered it, unless it maintains state sufficient to prevent
Expand Down Expand Up @@ -2562,7 +2572,7 @@ and integrity protection. Details of packet protection are found in
{{QUIC-TLS}}; this section includes an overview of the process.

Initial packets are protected using keys that are statically derived. This
packet protection is not effective confidentiality protection, it only exists to
packet protection is not effective confidentiality protection; it only exists to
ensure that the sender of the packet is on the network path. Any entity that
receives the Initial packet from a client can recover the keys necessary to
remove packet protection or to generate packets that will be successfully
Expand All @@ -2577,9 +2587,10 @@ handshake ensures that only the communicating endpoints receive the
corresponding keys.

The packet number field contains a packet number, which has additional
confidentiality protection that is applied after packet protection is applied
(see {{QUIC-TLS}} for details). The underlying packet number increases with
each packet sent, see {{packet-numbers}} for details.
confidentiality protection that is applied after packet protection is
applied (see {{QUIC-TLS}} for details). The underlying packet number
increases with each packet sent in a given packet number space, see
{{packet-numbers}} for details.


## Coalescing Packets {#packet-coalesce}
Expand Down Expand Up @@ -2758,7 +2769,7 @@ A sender can minimize per-packet bandwidth and computational costs by bundling
as many frames as possible within a QUIC packet. A sender MAY wait for a short
period of time to bundle multiple frames before sending a packet that is not
maximally packed, to avoid sending out large numbers of small packets. An
implementation may use knowledge about application sending behavior or
implementation MAY use knowledge about application sending behavior or
heuristics to determine whether and for how long to wait. This waiting period
is an implementation decision, and an implementation should be careful to delay
conservatively, since any delay is likely to increase application-visible
Expand Down Expand Up @@ -2808,7 +2819,7 @@ Packets containing PADDING frames are considered
to be in flight for congestion control purposes {{QUIC-RECOVERY}}. Sending only
PADDING frames might cause the sender to become limited by the congestion
controller (as described in {{QUIC-RECOVERY}}) with no acknowledgments
forthcoming from the receiver. Therefore, a sender should ensure that other
forthcoming from the receiver. Therefore, a sender SHOULD ensure that other
frames are sent in addition to PADDING frames to elicit acknowledgments from the
receiver.

Expand Down Expand Up @@ -3628,7 +3639,7 @@ that are added to the Long Header before the Length field.
{: #initial-format title="Initial Packet"}

These fields include the token that was previously provided in a Retry packet or
NEW_TOKEN frame:
a NEW_TOKEN frame:

Token Length:

Expand Down Expand Up @@ -3674,10 +3685,10 @@ subsequent to the first do not need to fit within a single UDP datagram.

<!-- TODO: delete this section after confirming that it is redundant -->

The first Initial packet sent by either endpoint contains a packet number of
0. The packet number MUST increase monotonically thereafter. Initial packets
are in a different packet number space to other packets (see
{{packet-numbers}}).
The first Initial packet sent by either endpoint MUST contain a packet
number of 0. The packet number MUST increase monotonically thereafter.
Initial packets are in a different packet number space to other
packets (see {{packet-numbers}}).

### 0-RTT Packet Numbers {#retry-0rtt-pn}

Expand Down Expand Up @@ -3732,9 +3743,8 @@ 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


The payload of this packet contains CRYPTO frames and could contain PADDING, or
ACK frames. Handshake packets MAY contain CONNECTION_CLOSE frames. Endpoints
Expand Down Expand Up @@ -3847,10 +3857,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 MUST validate that the transport parameter is
absent. A client MUST treat a failed validation as a connection error
of type TRANSPORT_PARAMETER_ERROR.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then "otherwise, it MUST validate that...."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, yes. Suggested above.


A Retry packet does not include a packet number and cannot be explicitly
acknowledged by a client.
Expand Down Expand Up @@ -4183,9 +4194,6 @@ Block describes progressively lower-numbered packets. As long as contiguous
ranges of packets are small, the variable-length integer encoding ensures that
each range can be expressed in a small number of bytes.

The ACK frame uses the least significant bit (that is, type 0x03) to indicate
ECN feedback and report receipt of QUIC packets with associated ECN codepoints
of ECT(0), ECT(1), or CE in the packet's IP header.

~~~
0 1 2 3
Expand Down Expand Up @@ -4266,8 +4274,11 @@ Additional ACK Block (repeated):

### ECN section

The ECN section should only be parsed when the ACK frame type is 0x03. The ECN
section consists of 3 ECN counters as shown below.
The ACK frame uses the least significant bit (that is, type 0x03) to
indicate that ECN feedback follows the ACK blocks. This feedback
reports receipt of QUIC packets with associated ECN codepoints of
ECT(0), ECT(1), or CE in the packet's IP header. The ECN section
consists of 3 ECN counters as shown below.

~~~
0 1 2 3
Expand Down Expand Up @@ -4430,8 +4441,8 @@ FIN bit.

## NEW_TOKEN Frame {#frame-new-token}

A server sends a NEW_TOKEN frame (type=0x07) to provide the client a token to
send in the header of an Initial packet for a future connection.
A server sends a NEW_TOKEN frame (type=0x07) to provide the client with
a token to send in the header of an Initial packet for a future connection.

The NEW_TOKEN frame is as follows:

Expand Down Expand Up @@ -4525,9 +4536,9 @@ Stream Data:
When a Stream Data field has a length of 0, the offset in the STREAM frame is
the offset of the next byte that would be sent.

The first byte in the stream has an offset of 0. The largest offset delivered
on a stream - the sum of the re-constructed offset and data length - MUST be
less than 2^62.
The first byte in the stream has an offset of 0. The largest offset
delivered on a stream - the sum of the offset and data length - MUST
be less than 2^62.


## MAX_DATA Frame {#frame-max-data}
Expand Down Expand Up @@ -4931,10 +4942,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 a CONNECTION_CLOSE frame cannot be split between
packets, any limits on packet size will also limit the space
available for a reason phrase.

Reason Phrase:

Expand Down Expand Up @@ -5326,7 +5337,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
they are abusive, frivolous, or actively harmful (not merely aesthetically
displeasing, or architecturally dubious).

Expand Down