Subject: [quicwg/base-drafts] Split error code space (#722)

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 05:02:25 +0000 (UTC)

This creates two orthogonal spaces for error codes.  Application error codes
can be used for both connection and stream errors and are under the control of
the application protocol.  Transport error codes are QUIC-controlled, but can
only terminate the connection.

To fix this, I had to add IANA considerations for error codes, plus a few extra
tweaks.  I think that the error code space could be narrowed to 16 bits after
this, if only to keep things sane.

Closes #132.

(I've stacked this on top of #721 because I don't like managing conflicts and this touches the same general area.)
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Refactor the section on connection termination
  * Split error code space
  * s/Public Reset/Stateless Reset/

-- File Changes --

    M draft-ietf-quic-tls.md (41)
    M draft-ietf-quic-transport.md (323)

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 05:07:20 +0000 (UTC)

@martinthomson pushed 1 commit.

bf6c5d9  Get codes consistent

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/4edb255eb98bcc1fff17dd517863303f97b872f5..bf6c5d960364642ee51d651c862b2a00b436f92e

---

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 05:15:32 +0000 (UTC)

1f00110  Make STOP_SENDING application-specific as well

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/bf6c5d960364642ee51d651c862b2a00b436f92e..1f001108b2a9af83d7804ec1f2ec2dca0ddda338

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Aug 2017 22:16:36 -0700

fc7ced7  Split errors in HTTP too

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/1f001108b2a9af83d7804ec1f2ec2dca0ddda338..fc7ced7cd62f414e10086ca767c477f7727f5808

---

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 05:22:09 +0000 (UTC)

@martinthomson pushed 2 commits.

9ca0728  Refine description of HTTP codes
50594bf  Move TLS errors closer

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/fc7ced7cd62f414e10086ca767c477f7727f5808..50594bf5bcb50cf08762f405d3d47692b44c4df6

---

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 05:29:56 +0000 (UTC)

5538d44  Match the names
a26a1fb  Add HTTP_NO_ERROR

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/50594bf5bcb50cf08762f405d3d47692b44c4df6..a26a1fbcdf481e32120a755d1dd47336f053acdd

---

From: janaiyengar <[email-masked]>
Date: Tue, 15 Aug 2017 00:02:39 +0000 (UTC)

janaiyengar commented on this pull request.

Comments inline. Can you also please not stack these PRs? Github is really difficult to use for review of large changes like this one, and stacking PRs makes it even harder. I understand the pains of merge conflicts, but honestly, that's a one-time pain during merge, not a continual pain during review.

-The portion of the QUIC error code space allocated for the crypto handshake is
-0xC0000000-0xFFFFFFFF. The following error codes are defined when TLS is used
-for the crypto handshake:
+This document defines error codes from the error code space used in
+{{QUIC-TRANSPORT}}.

This document --> This section.
Also, editorial nit: I like the mention of "crypto handshake when TLS is used for it".

 Error Code:
 
 : A 32-bit error code which indicates the reason for closing this connection.
+  TRANSPORT_CLOSE uses codes from the space defined in {{error-codes}};
+  APPLICATION_CLOSE uses codes from the application protocol error code space
+  ({{app-error-codes}}).

Seems unnecessary to mention APPLICATION_CLOSE in the definitoion of TRANSPORT_CLOSE.

> @@ -2608,11 +2654,10 @@ discarded upon receipt.  This avoids potential ambiguity about which STREAM
 frames count toward flow control.
 
 Upon receipt of a STOP_SENDING frame on a stream in the "open" or "half-closed
-(remote)" states, an endpoint MUST send a RST_STREAM with an error code of
-QUIC_RECEIVED_RST.  If the STOP_SENDING frame is received on a stream that is
-already in the "half-closed (local)" or "closed" states, a RST_STREAM frame MAY
-still be sent in order to cancel retransmission of previously-sent STREAM
-frames.
+(remote)" states, an endpoint MUST send a RST_STREAM frame.  If the STOP_SENDING

What's the proposed error code in the RST_STREAM now? If there's a requirement for QUIC to generate and send this frame, the error code has to be specified.

-An endpoint that receives an invalid CONNECTION_CLOSE frame MUST NOT signal the
-existence of the error to its peer.
+An endpoint that chooses not to retransmit packets containing TRANSPORT_CLOSE or
+APPLICATION_CLOSE risks a peer missing the first such packet.  The only
+mechanism available to an endpoint that continues to receive data for a
+terminated connection is to use the stateless reset process
+({{stateless-reset}}).

editorial nit: I would merge this para with the one above, since this one follows from the earlier.

-An endpoint that receives an invalid CONNECTION_CLOSE frame MUST NOT signal the
-existence of the error to its peer.
+An endpoint that chooses not to retransmit packets containing TRANSPORT_CLOSE or
+APPLICATION_CLOSE risks a peer missing the first such packet.  The only

Not just the first packet, but any packets lost in the network.

> @@ -2923,115 +2975,102 @@ Stream 0 is critical to the functioning of the entire connection.  If stream 0
 is closed with either a RST_STREAM or STREAM frame bearing the FIN flag, an
 endpoint MUST generate a connection error of type PROTOCOL_VIOLATION.
 
-Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT

I would drop this first sentence -- it's a bit imprecise and does not add value.

> -Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT
+be generated by the QUIC transport layer.  Resetting a stream at the transport
+layer could cause unrecoverable loss of application protocol state.  Application
+protocols might require certain streams to be reliably delivered in order to
+guarantee consistent state between endpoints.  For this reason, all errors that
+can be detected at the transport layer result in connection errors.

This last sentence does not belong in this section, and is unnecessary. I would drop it.

-Error codes are 32 bits long, with the first two bits indicating the source of
-the error code:
+An endpoint that detects a stream error MAY choose to treat the error as a
+connection error and send an APPLICATION_CLOSE frame in place of RST_STREAM.

This is confusing. What is the "endpoint"? QUIC, or HTTP/QUIC? If QUIC is no longer generating RST_STREAM, then which stream errors are detected by QUIC? I suspect the answer is that the endpoint is HTTP/QUIC, in which case I would remove this sentence from here entirely.

 : An endpoint detected an error in a specific frame type.  The frame type is
   included as the last octet of the error code.  For example, an error in a
-  MAX_STREAM_ID frame would be indicated with the code (0x80000106).
+  MAX_STREAM_ID frame would be indicated with the code (0x106).
+
+See {{iana-error-codes}} for details of registering new error codes.
+
+
+## Application Protocol Error Codes {#app-error-codes}
+
+Application protocol error codes are 32-bits long, but the management of
+application error codes are left to application protocols.  Application protocol
+error codes are used for the RST_STREAM ({{frame-rst-stream}}) and
+APPLICATION_CLOSE ({{frame-application-close}}) frames.
+
+Application protocols SHOULD define an error codes for indicating no error and

typo: remove "an"

 : An endpoint detected an error in a specific frame type.  The frame type is
   included as the last octet of the error code.  For example, an error in a
-  MAX_STREAM_ID frame would be indicated with the code (0x80000106).
+  MAX_STREAM_ID frame would be indicated with the code (0x106).
+
+See {{iana-error-codes}} for details of registering new error codes.
+
+
+## Application Protocol Error Codes {#app-error-codes}
+
+Application protocol error codes are 32-bits long, but the management of

16-bits

> @@ -1596,7 +1622,7 @@ The RST_STREAM frame is as follows:
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                        Stream ID (32)                         |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                        Error Code (32)                        |
+| App Protocol Error Code (16)  |

This is still 32 bits in this PR, right?

> @@ -2923,115 +2975,102 @@ Stream 0 is critical to the functioning of the entire connection.  If stream 0
 is closed with either a RST_STREAM or STREAM frame bearing the FIN flag, an
 endpoint MUST generate a connection error of type PROTOCOL_VIOLATION.
 
-Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT
+be generated by the QUIC transport layer.  Resetting a stream at the transport

drop "transport layer". 

Also, it's odd to say QUIC MUST not generate a RST_STREAM, since it is a QUIC frame and QUIC does in fact generate it. How about "RST_STREAM MUST be instigated by the application and MUST carry an application error code."?

-- 
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/722#pullrequestreview-56230010

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Aug 2017 00:33:51 +0000 (UTC)

martinthomson commented on this pull request.

I haven't seen a review for #721 yet.  I apologize for stacking like this, but #721 is pretty self-contained, so the amount of leakage from that is small.  That said, I tried removing it from this changeset and found that a lot of the text is intertwined.  I've started stacking PRs differently now, so the reviewing should be easier.

Yes, I must have split this from the other PR poorly.

 Error Code:

I considered not doing that, but it seemed more helpful to highlight the distinction here given that I wasn't repeating the format stuff in the description of APPLICATION_CLOSE.

Actually, the point here is that the *application* sends the RST_STREAM.  The transport can't send RST_STREAM.  The transport can't do that without risking putting the application into a bad state.  I will change this to make it clearer.

Actually, I'm going to raise this on the list.  It seems like STOP_SENDING could be made an application-layer construct and we could avoid that complexity at the transport layer entirely.  I'm trying to see how visibility at the transport layer would be of value, but I'm drawing a blank.

The crypto handshake bit is on the very next line.  The point of separation is that the codes are consumed even if TLS isn't used.

I'll remove this sentence, reword the remaining sentence, and merge.

-- 
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/722#pullrequestreview-56237490

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Aug 2017 01:21:18 +0000 (UTC)

3f1867a  Review comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/722/files/a26a1fbcdf481e32120a755d1dd47336f053acdd..3f1867a100da12fa58b2e21acb0031e545aa64dc