Re: [quicwg/base-drafts] Specify behavior for post-handshake CRYPTO messages (#2524)

[Martin Thomson <[e-mail]>]

martinthomson requested changes on this pull request.

Thanks for writing this up Nick.

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to
+discard all subsequent CRYPTO frames, the packets containing these CRYPTO frames
+are still ACKed. An endpoint in this condition MAY tear down state for its

```suggestion
are still ackknowledged. An endpoint in this condition MAY tear down state for its
```

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to

```suggestion
connection with an INTERNAL_ERROR code. If an endpoint chooses to
```

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to
+discard all subsequent CRYPTO frames, the packets containing these CRYPTO frames
+are still ACKed. An endpoint in this condition MAY tear down state for its
+cryptographic handshake, keeping only the material needed for the current
+encryption keys and any future key updates.

I would drop the last sentence here.  It's an optimization that is sufficiently obvious.

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.

I think that this needs a new section heading.  Section 7.4 "After Handshake Completion" perhaps, after the transport parameters, could be what it is, but it might be better to say "Cryptographic Message Buffering" and discuss the handshake a little (more below).

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.

Does this requirement apply during the handshake?  Perhaps document some recommendations.

Being unable to buffer CRYPTO frames during the handshake leads to connection failure. Endpoints MAY choose to allow more data to be buffered during the handshake and reduce buffering once the handshake completes.  A larger limit during the handshake could allow for larger keys or credentials to be exchanged.

-- 
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/2524#pullrequestreview-215332111