Subject: [quicwg/base-drafts] Replace use of the TLS presentation language with QUIC encoding (#3108)

From: David Schinazi <[email-masked]>
Date: Thu, 17 Oct 2019 09:20:02 -0700

This PR is purely editorial and does not change the protocol in any way.

When we discussed #3020 in Cupertino, it emerged that there was consensus for this editorial change but not for all topics in that issue, so I split this out to its own PR.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Make transport parameters varint
  * Merge branch &#39;master&#39; into no_tls_pres
  * Revert non-editorial changes
  * fix

-- File Changes --

    M draft-ietf-quic-tls.md (6)
    M draft-ietf-quic-transport.md (119)

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Fri, 18 Oct 2019 00:14:27 -0700

So the variable length integers have a 2 byte length prefix followed by a two bit length encoding?

-- 
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/3108#issuecomment-543560674

---

From: Nick Banks <[email-masked]>
Date: Fri, 18 Oct 2019 08:34:21 -0700

nibanks approved this pull request.

LGTM. I'd also be for removing the unnecessary, prefixed `Sequence Length` and fine with changing to varint encoding where applicable.

-- 
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/3108#pullrequestreview-303972000

---

From: ianswett <[email-masked]>
Date: Fri, 18 Oct 2019 20:15:12 -0700

ianswett approved this pull request.

-- 
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/3108#pullrequestreview-304193043

---

From: Martin Thomson <[email-masked]>
Date: Mon, 21 Oct 2019 17:40:11 -0700

martinthomson approved this pull request.

SGTM

> @@ -1336,7 +1336,7 @@ protocol incompatible with the protocol version being used.
 ## QUIC Transport Parameters Extension {#quic_parameters}
 
 QUIC transport parameters are carried in a TLS extension. Different versions of
-QUIC might define a different format for this struct.
+QUIC might define a different format for them.

"them" is a little vague.

```suggestion
QUIC might define a different method for negotiating transport configuration.
```

> +|  Transport Parameter ID (16)  |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|  Transport Param Length (16)  |

This should be a multi-line suggestion, but thanks to GitHub, it doesn't work that way...

```suggestion
|  Transport Parameter ID (16)  |  Transport Param Length (16)  |
```

 ~~~
-   struct {
-     opaque ipv4Address[4];
-     uint16 ipv4Port;
-     opaque ipv6Address[16];
-     uint16 ipv6Port;
-     opaque connectionId<0..20>;
-     opaque statelessResetToken[16];
-   } PreferredAddress;
+ 0                   1                   2                   3
+ 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                       IPv4 Address (32)                       |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|         IPv4 Port (16)        |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

You could wrap this differently...

```
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|         IPv4 Port (16)        |                               |
+-------------------------------+                               +
|                                                               |
+                                                               +
|                      IPv6 Address (128)                       |
+                                                               +
|                                                               |
+                               +-------------------------------+
|                               |         IPv6 Port (16)        |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
```

> +| CID Length (8)|
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                      Connection ID (*)                      ...

```suggestion
| CID Length (8)|      Connection ID (*)                      ...
```

-- 
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/3108#pullrequestreview-304921499

---

From: David Schinazi <[email-masked]>
Date: Mon, 21 Oct 2019 19:05:35 -0700

@DavidSchinazi pushed 1 commit.

79a9a7c9f34d3482fcd61265ca3c10e5dcf365ba  MT inline comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3108/files/0b7f3ddf94b58a9348e1e59ac9aa927f1ddb474b..79a9a7c9f34d3482fcd61265ca3c10e5dcf365ba

---

From: David Schinazi <[email-masked]>
Date: Mon, 21 Oct 2019 19:06:32 -0700

DavidSchinazi commented on this pull request.

 ~~~
-   struct {
-     opaque ipv4Address[4];
-     uint16 ipv4Port;

That looks sad :(

-- 
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/3108#discussion_r337309654

---

From: David Schinazi <[email-masked]>
Date: Mon, 21 Oct 2019 19:07:21 -0700

I'd rather follow the format we've been using in the definition of long headers

-- 
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/3108#discussion_r337309737

---

From: David Schinazi <[email-masked]>
Date: Mon, 21 Oct 2019 19:07:54 -0700

8641db7f92831e309c04ae9b181134535adbdff9  MT multiline comment fixup

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3108/files/79a9a7c9f34d3482fcd61265ca3c10e5dcf365ba..8641db7f92831e309c04ae9b181134535adbdff9

---

From: Rui Paulo <[email-masked]>
Date: Wed, 23 Oct 2019 12:31:09 -0700

Are we doing https://github.com/quicwg/base-drafts/issues/2944 here or 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/3108#issuecomment-545600652

---

From: David Schinazi <[email-masked]>
Date: Wed, 23 Oct 2019 20:30:50 +0000 (UTC)

@rpaulo this is an editorial PR that does not change any wire encoding. We are not doing #2944 here.

-- 
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/3108#issuecomment-545622801

---

From: Rui Paulo <[email-masked]>
Date: Fri, 25 Oct 2019 11:01:13 -0700

This  not marked as editorial.  It was decided in Cupertino to either do #2944 here or to close #2944. 

-- 
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/3108#issuecomment-546452894

---

From: David Schinazi <[email-masked]>
Date: Fri, 25 Oct 2019 11:51:03 -0700

@rpaulo that is not my recollection. Either way, this is a PR, we can discuss non-editorial changes on the appropriate issue.

-- 
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/3108#issuecomment-546470038

---

From: Marten Seemann <[email-masked]>
Date: Mon, 28 Oct 2019 23:23:35 -0700

marten-seemann approved this pull request.

-- 
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/3108#pullrequestreview-308293667

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 28 Oct 2019 23:32:17 -0700

kazuho commented on this pull request.

 ~~~
-   struct {
-     opaque ipv4Address[4];
-     uint16 ipv4Port;

Seriously, I believe the diagram should be left as-is.

IIUC, our convention is to wrap the lines in cases where the following field is bigger than 32-bits. Or do we want to wrap the Version field of long header packets?

-- 
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/3108#discussion_r339910157

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 29 Oct 2019 15:34:02 -0700

janaiyengar approved this pull request.

 ~~~
-   struct {
-     opaque ipv4Address[4];
-     uint16 ipv4Port;

I second @martinthomson's suggestion.

Same here.

-- 
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/3108#pullrequestreview-306331829

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Oct 2019 15:34:11 -0700

Merged #3108 into master.

-- 
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/3108#event-2754037309