Subject: [quicwg/base-drafts] Use CONNECTION_CLOSE rather than CRYPTO_CLOSE (#1477)

From: Martin Thomson <[email-masked]>
Date: Mon, 25 Jun 2018 18:42:07 -0700

More frame types aren't really necessary here.

(This will conflict with the rename of CRYPTO_HS, but I can manage that.)
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Use CONNECTION_CLOSE rather than CRYPTO_CLOSE

-- File Changes --

    M draft-ietf-quic-tls.md (18)
    M draft-ietf-quic-transport.md (94)

-- Patch Links --

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

---

From: ekr <[email-masked]>
Date: Mon, 25 Jun 2018 20:49:11 -0700

ekr commented on this pull request.

Not sure how I feel about this, but noticed some glitches you should probably correct pre-landing

> @@ -602,10 +602,16 @@ HelloRetryRequest is still used for incorrect key shares.
 
 ## TLS Errors
 
-If TLS experiences an error, it MUST generate an appropriate alert
-as defined in {{TLS13}}; Section 6) and then provide it to QUIC,
-which sends the alert in a CRYPTO_CLOSE frame. All such alerts are
-"fatal" (see {{TLS13}}, Section 6.2.
+If TLS experiences an error, it generates an appropriate alert as defined in
+Section 6 of {{TLS13}}.
+
+A TLS alert is turned into a QUIC connection error by converting the alert
+description into a QUIC error code.  The alert description is added to 0x200 to

Probably worth observing that this is a one-byte field.

> @@ -602,10 +602,16 @@ HelloRetryRequest is still used for incorrect key shares.
 
 ## TLS Errors
 
-If TLS experiences an error, it MUST generate an appropriate alert
-as defined in {{TLS13}}; Section 6) and then provide it to QUIC,
-which sends the alert in a CRYPTO_CLOSE frame. All such alerts are
-"fatal" (see {{TLS13}}, Section 6.2.
+If TLS experiences an error, it generates an appropriate alert as defined in
+Section 6 of {{TLS13}}.
+
+A TLS alert is turned into a QUIC connection error by converting the alert
+description into a QUIC error code.  The alert description is added to 0x200 to
+produce a QUIC error code from the range reserved for CRYPTO_ERROR.  The
+resulting value is sent in a QUIC CONNECTION_CLOSE frame.
+
+The alert level of all TLS alerts is "fatal", a TLS stack does not generate
+alerts at the "warning" level.

This is a bit confusing because stacks can in fact do that. Maybe MUST NOT?

-- 
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/1477#pullrequestreview-131856622

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 Jun 2018 22:03:11 -0700

martinthomson commented on this pull request.

 ## TLS Errors

I thought that this was OK.  TLS 1.3 prohibits use of warning for anything but close_notify, and we don't allow TLS to close.  I guess that MUST NOT isn't a problem though.

-- 
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/1477#discussion_r198014372

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 Jun 2018 22:03:41 -0700

@martinthomson pushed 1 commit.

9b80c45  one-octet description; MUST NOT warning alert

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1477/files/0acc8f8b945ffc6199492f12fa6d0c5059101175..9b80c45f58b43ede57b9876e21df30dfa963f49b

---

From: Rui Paulo <[email-masked]>
Date: Tue, 26 Jun 2018 00:13:19 -0700

I really like this simplification.

-- 
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/1477#issuecomment-400204210

---

From: ianswett <[email-masked]>
Date: Tue, 26 Jun 2018 06:04:07 -0700

ianswett approved this pull request.

Some small nits, but mostly looks good to me.

> @@ -1032,7 +1032,6 @@ explained in more detail as they are referenced later in the document.
 | 0x0f        | PATH_RESPONSE     | {{frame-path-response}}     |
 | 0x10 - 0x17 | STREAM            | {{frame-stream}}            |
 | 0x18        | CRYPTO_HS         | {{frame-crypto}}            |
-| 0x19        | CRYPTO_CLOSE      | {{frame-crypto-close}}      |
 | 0x20        | NEW_TOKEN         | {{frame-new-token}}         |

Renumber NEW_TOKEN to 0x19

 Application protocols can signal application-specific protocol errors
 using the APPLICATION_CLOSE frame.  Errors that are specific to the
 transport, including all those described in this document, are carried
 in a CONNECTION_CLOSE frame.  Other than the type of error code they
 carry, these frames are identical in format and semantics.
 
-A CONNECTION_CLOSE, CRYPTO_CLOSE, or APPLICATION_CLOSE frame could be
-sent in a packet that is lost.  An endpoint SHOULD be prepared to
-retransmit a packet containing either frame type if it receives more
-packets on a terminated connection.  Limiting the number of
-retransmissions and the time over which this final packet is sent
+A CONNECTION_CLOSE, or APPLICATION_CLOSE frame could be sent in a packet that is

"," unnecessary

-- 
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/1477#pullrequestreview-131980571

---

From: Nick Banks <[email-masked]>
Date: Tue, 26 Jun 2018 06:21:35 -0700

nibanks approved this pull request.

Looks good to me.

-- 
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/1477#pullrequestreview-132014704

---

From: janaiyengar <[email-masked]>
Date: Tue, 26 Jun 2018 12:16:42 -0700

janaiyengar approved this pull request.

Yes, definitely.

-- 
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/1477#pullrequestreview-132167155

---

From: Mike Bishop <[email-masked]>
Date: Tue, 26 Jun 2018 13:53:18 -0700

MikeBishop approved this pull request.

Yes please.

 ## TLS Errors
 
-If TLS experiences an error, it MUST generate an appropriate alert
-as defined in {{TLS13}}; Section 6) and then provide it to QUIC,
-which sends the alert in a CRYPTO_CLOSE frame. All such alerts are
-"fatal" (see {{TLS13}}, Section 6.2.
+If TLS experiences an error, it generates an appropriate alert as defined in
+Section 6 of {{TLS13}}.
+
+A TLS alert is turned into a QUIC connection error by converting the one-octet
+alert description into a QUIC error code.  The alert description is added to
+0x200 to produce a QUIC error code from the range reserved for CRYPTO_ERROR.
+The resulting value is sent in a QUIC CONNECTION_CLOSE frame.
+
+The alert level of all TLS alerts is "fatal", a TLS stack MUST NOT generate

Comma splice.  Separate sentences or `;`.

No objection, but also seems unnecessary.  Leave 0x19 for the next frame to be defined, and if there are gaps in the space we'll defrag before we ship.

> @@ -2200,10 +2199,9 @@ before the packet is received.
 
 ### Immediate Close
 
-An endpoint sends a closing frame, (CONNECTION_CLOSE, CRYPTO_CLOSE or
-APPLICATION_CLOSE) to terminate the connection immediately.  Any
-closing frame causes all streams to immediately become closed; open
-streams can be assumed to be implicitly reset.
+An endpoint sends a closing frame, (CONNECTION_CLOSE or APPLICATION_CLOSE) to

Unnecessary comma

-- 
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/1477#pullrequestreview-132199013

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 Jun 2018 15:18:59 -0700

a77b066  Typos, renumber

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1477/files/9b80c45f58b43ede57b9876e21df30dfa963f49b..a77b0669f52aaa155dc6fb15f97d3aaf0237a8bf

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 Jun 2018 15:19:49 -0700

d768347  commas

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 Jun 2018 16:07:29 -0700

d4ec8a5  comma splice

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 Jun 2018 16:09:13 -0700

Merged #1477.

-- 
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/1477#event-1702665298