Subject: [quicwg/base-drafts] Optimize definition IP addr in TP PreferredAddress (#1533)

From: Dmitri Tikhonov <[email-masked]>
Date: Fri, 06 Jul 2018 13:21:34 -0700

There are two reasons for this change.

First, having the IP address stored as an opaque string 4 to 255 octets
in length does not make it clear in what format the IP address is stored.
The choice of 4 and 16 bytes makes the format obvious.

Second, the longer of the two possibilities is 16 bytes.  There is no
reason to allow for longer values.  Only allowing 4 and 16 bytes make
the checks both stricter and simpler and implementations more efficient.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Optimize definition IP addr in TP PreferredAddress

-- File Changes --

    M draft-ietf-quic-transport.md (13)

-- Patch Links --

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

---

From: Mike Bishop <[email-masked]>
Date: Fri, 06 Jul 2018 15:27:41 -0700

Technically, it's not a string, it's a length-prefixed sequence of bytes.  I think all this actually does is to make the length implicit from the IP version.  @martinthomson had suggested having the explicit length prefix, so I'll invoke him to defend it.  ;-)

Making it implicit saves a byte, if nothing else. If you see an IP version you don't recognize (and therefore don't know how long the following field is), you're probably bailing on the entire TP anyway.  Fortunately, the value is also length-prefixed and could be skipped as a whole.

-- 
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/1533#issuecomment-403161657

---

From: Dmitri Tikhonov <[email-masked]>
Date: Sat, 07 Jul 2018 04:12:21 -0700

The proposed change does more than simply make the IP address length implicit.  Tying length to the IP version makes it apparent what the format of the IP address is without any additional verbiage.  Such verbiage is not present in the current version of the Draft.

As it stands, one could imagine (however perversely) that the IP address is a NUL-terminated ASCII string, such as "::1" (hence the minimum of 4 bytes), "192.168.100.25", or "2001:0db8:85a3:0000:0000:8a2e:0370:7334".

-- 
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/1533#issuecomment-403208120

---

From: MikkelFJ <[email-masked]>
Date: Sat, 07 Jul 2018 04:53:00 -0700

Like http://doc.cat-v.org/plan_9/4th_edition/papers/net/?

-- 
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/1533#issuecomment-403210438

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jul 2018 17:55:21 -0700

martinthomson commented on this pull request.

> @@ -1444,9 +1444,20 @@ language from Section 3 of {{!I-D.ietf-tls-tls13}}.
       TransportParameter parameters<22..2^16-1>;
    } TransportParameters;
 
+   struct {
+     uint8[4] addr;
+   } IPv4addr;
+
+   struct {
+     uint8[16] addr;
+   } IPv6addr;

You can use a typedef for this:

```
typedef uint8[16] IPv6addr;
```

-- 
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/1533#pullrequestreview-135253675

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jul 2018 17:58:45 -0700

You can't assume that you know the IP address.  The server might want to advertise a preferred address in a version other than the current one.  For instance, you reach the server over v4, but it would rather you use v6.  You can probably assume that people understand that v6 means 16 octets, but is that true for IPv10?

Yes, you can ignore the entire transport parameter if you hit an unknown version.  That's a little unusual for TLS 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/1533#issuecomment-403332000

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 09 Jul 2018 05:58:05 -0700

> You can probably assume that people understand that v6 means 16 octets, but is that true for IPv10?

This reasoning seems far-fetched.  By the same logic, we should not limit the IP version to 15 (see the enum maximum value), nor the port number to 65535.

If the idea is to be able to skip unknown IP address blobs, so to speak, then the format of the IPv4 and IPv6 blobs should be specified somehow -- either using the TLS presentation language or English.

-- 
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/1533#issuecomment-403469609

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 09 Jul 2018 06:00:50 -0700

dtikhonov commented on this pull request.

+   struct {
+     uint8[4] addr;
+   } IPv4addr;
+
+   struct {
+     uint8[16] addr;
+   } IPv6addr;

There are no typedefs in the [TLS 1.3 draft](https://tools.ietf.org/html/draft-ietf-tls-tls13-28).

-- 
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/1533#discussion_r200992572

---

From: ekr <[email-masked]>
Date: Mon, 09 Jul 2018 07:22:59 -0700

ekr commented on this pull request.

+   struct {
+     uint8[4] addr;
+   } IPv4addr;
+
+   struct {
+     uint8[16] addr;
+   } IPv6addr;

The way you say this in TLS is:

```
uint8 IPv6addr[16];
```

-- 
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/1533#discussion_r201020325

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 09 Jul 2018 15:37:03 +0000 (UTC)

@dtikhonov pushed 1 commit.

c2647e3  Drop IPv4addr and IPv6addr structs in favor of vectors

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

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 09 Jul 2018 08:38:13 -0700

+   struct {
+     uint8[4] addr;
+   } IPv4addr;
+
+   struct {
+     uint8[16] addr;
+   } IPv6addr;

Ah yes, this is much simpler, thanks.  Done in c2647e37ea6af229d201b542bb2a8f372f872440.

-- 
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/1533#discussion_r201048895

---

From: Mike Bishop <[email-masked]>
Date: Mon, 09 Jul 2018 10:37:17 -0700

@dtikhonov, the IP version is limited to 15 because that's the space in which 4 and 6 are allocated.  https://www.iana.org/assignments/version-numbers/version-numbers.xhtml.  However, I'm sympathetic to the argument that it should be implicit because if you don't know the specified version, being able to figure out how long the address is seems pointless.

Or are you suggesting that a future version might have a variable-length address, for which an explicit length would be required?

-- 
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/1533#issuecomment-403559681

---

From: Dmitri Tikhonov <[email-masked]>
Date: Tue, 10 Jul 2018 06:23:19 -0700

I don't understand why the build [failed](https://circleci.com/gh/quicwg/base-drafts/6042?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link).  Works for me when I `make` 😕 .

-- 
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/1533#issuecomment-403820087

---

From: Dmitri Tikhonov <[email-masked]>
Date: Tue, 10 Jul 2018 06:28:29 -0700

How can we move this forward?  The problem this PR is trying to solve is the lack of the IP address format description in the document.  There are two ways to solve it:
1. Describe the format using the TLS Presentation Language; or
1. Describe the format using English.
My preference is the former.  I am OK with English as well -- describing the address format outside of the TP figure.

-- 
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/1533#issuecomment-403821806

---

From: MikkelFJ <[email-masked]>
Date: Tue, 10 Jul 2018 06:38:42 -0700

Just say that the encoding is big endian in same encoding as the UDP/4 and UDP/6 header formats with 4 and 16 bytes respetively.

-- 
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/1533#issuecomment-403825187

---

From: Mike Bishop <[email-masked]>
Date: Tue, 10 Jul 2018 12:10:50 -0700

Build failure isn't you.  One of the references in TLS has stopped working.  It probably works for you either because you're specifically building just this doc or because your .refcache contains a version of the reference that's sufficiently fresh that it's not trying to refresh.

-- 
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/1533#issuecomment-403933248