Subject: [quicwg/base-drafts] Version 0 is Version Negotiation (#968)

From: Mike Bishop <[email-masked]>
Date: Wed, 29 Nov 2017 20:51:27 +0000 (UTC)

Fixes #963.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * 0 is Version Negotiation

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Nov 2017 21:23:45 +0000 (UTC)

martinthomson requested changes on this pull request.

I think that we need to be clearer about the use of the long header here.  I don't think that we should say that it has a long header, only that it sets the bit (so that recipients will be able to look for the version field).  And we need to explain further about how this is expected to be identified by a recipient.

But then I don't think we should use the long header at all, and drop the randomized packet number field.

-The connection ID and version fields echo corresponding values from the
-triggering client packet.  This allows clients some assurance that the
-server received the packet and that the Version Negotiation packet was not
-carried in a packet with a spoofed source address.
+The Connection ID field echos the corresponding value from the triggering client

echoes

-- 
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/968#pullrequestreview-80001199

---

From: janaiyengar <[email-masked]>
Date: Wed, 29 Nov 2017 15:29:34 -0800

janaiyengar commented on this pull request.

I agree with Martin -- I don't think this needs to be a long header at all, but only needs to have the bit set. If you do this, you can eliminate the packet number field.

-The connection ID and version fields echo corresponding values from the
-triggering client packet.  This allows clients some assurance that the
-server received the packet and that the Version Negotiation packet was not
-carried in a packet with a spoofed source address.
+The Connection ID field echos the corresponding value from the triggering client
+packet.  This allows clients some assurance that the server received the packet
+and that the Version Negotiation packet is in fact from the server.  Values for
+the Packet Number and Type fields are selected at random.

Nit: I would require this: "Values for ... MUST be randomly chosen."

-- 
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/968#pullrequestreview-80033829

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 00:06:45 +0000 (UTC)

@MikeBishop pushed 1 commit.

00e0218  Version Negotiation is its own format

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/968/files/5a6ec3a337b076e718062a45f1497e9055489c96..00e0218d34fe8fcdeea775160f050d105ee4542e

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 00:07:31 +0000 (UTC)

a43d55e  echos vs. echoes

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/968/files/00e0218d34fe8fcdeea775160f050d105ee4542e..a43d55e29ba7ec7c61a6a581f827dc31800d9206

---

From: Martin Thomson <[email-masked]>
Date: Thu, 30 Nov 2017 00:18:08 +0000 (UTC)

martinthomson commented on this pull request.

I think that we should say something about how this is identified by a client.  Maybe:

"A Version Negotiation packet that is received by a client will be initially identified as a packet with a long header based on the high bit of the first octet.  Checking the Version field will reveal the packet to be a Version Negotiation packet."

 ~~~
  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
++-+-+-+-+-+-+-+-+
+|1|   Type (7)  |

Rather than enshrine this as a "Type" field, I think that I'd prefer that this be marked "Unused" instead.

-- 
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/968#pullrequestreview-80043433

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 00:22:36 +0000 (UTC)

70d4edf  No type, unused

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/968/files/a43d55e29ba7ec7c61a6a581f827dc31800d9206..70d4edf88e6b1d92a4adda43ec4d914bc456c961

---

From: Marten Seemann <[email-masked]>
Date: Wed, 29 Nov 2017 17:27:17 -0800

marten-seemann commented on this pull request.

> @@ -543,6 +544,17 @@ the server supports, as shown below.
 ~~~
 {: #version-negotiation-format title="Version Negotiation Packet"}
 
+The value in the Unused field is selected randomly by the server. The Connection

This will make it impossible to determine the packet type by looking at the type byte.

-- 
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/968#pullrequestreview-80054289

---

From: janaiyengar <[email-masked]>
Date: Wed, 29 Nov 2017 17:31:06 -0800

~~~

That is true, but you can look at the type byte and version fields together to determine it. Also note that this is only sent on the server -> client direction, so you're not going to hit boxes that need to scale this sort of check.

-- 
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/968#discussion_r153965230

---

From: Marten Seemann <[email-masked]>
Date: Thu, 30 Nov 2017 01:35:40 +0000 (UTC)

~~~

True, but why grease it at all? Note that with this proposal, you can't trust the long header type byte **at all**, since every reserved value could also just be the random value of the version negotiation packet.
This will make parsing long header packets quite painful, you will always have to look at the other fields to determine the true packet byte.
For middle boxes, the version negotiation packet will be clearly distinguishable anyway, so I don't see the need to play tricks with the type byte.

-- 
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/968#discussion_r153965794

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Nov 2017 20:12:46 -0800

~~~

Removing version negotiation from consideration makes it possible to grease the type field completely.  Creating a carve-out for version negotiation makes it a little harder.

Long header packets are rare, so it seems like hitting the slow path is tolerable.

-- 
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/968#discussion_r153981991

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Nov 2017 20:13:04 -0800

@MikeBishop did you miss the start of my comment?

-- 
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/968#issuecomment-348077688

---

From: Marten Seemann <[email-masked]>
Date: Wed, 29 Nov 2017 20:22:43 -0800

~~~

We could exclude the code points defined for Long Header types from the random values that can be used for the greased type byte of the version negotiation packet. That way, we get greasing and easy parsing at the same time.

-- 
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/968#discussion_r153982825

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 11:02:21 -0800

The PR now says:

> A Version Negotiation packet [...] does not use the packet headers defined above. Upon receipt by a client, it will appear to be a packet using the long header, but will be identified as a Version Negotiation packet based on the Version field.

Is there something specific about the use of the long header that needs to be clearer?

-- 
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/968#issuecomment-348287249

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 19:09:29 +0000 (UTC)

MikeBishop commented on this pull request.

 ~~~

"the other fields" => version.  Since the type byte is version-specific, you should always confirm that the version is what you expect before attempting to interpret the type byte.  (Should we perhaps mention this in the details of handling incoming 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/968#discussion_r154171766

---

From: Marten Seemann <[email-masked]>
Date: Thu, 30 Nov 2017 19:46:23 +0000 (UTC)

~~~

Ok, I didn't think about multiple version. Since the type byte version dependent, I'd argue that we shouldn't grease it at all for version negotiation packets.This would simplify the parsing logic, since you could identify the version negotiation packet by just looking at it's type byte (which is a QUIC invariant). For all other packets, you'd know the version anyway (because they're received after version negotiation).
If we want to grease, we could define a greasing mechanism for 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/968#discussion_r154181661

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 19:55:40 +0000 (UTC)

~~~

But the greasing mechanism for long-header packets would likely be version-specific.  I think the point here is that "version 0" doesn't actually define any semantics for the Type field because it doesn't define any types; it defines a single type, and the value in that field is irrelevant once you've seen the version.

So the parsing logic would be:

```
if( in_handshake ) {
  switch(packet.version) {
    what_i_sent:  switch(packet.type) { ... }
    0:  version_negotiation(packet)
    default:  return;
  }
else {
  if(packet.version != connection.version) return;
  ...
}
```

-- 
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/968#discussion_r154184185

---

From: MikkelFJ <[email-masked]>
Date: Thu, 30 Nov 2017 12:14:36 -0800

Is bit 6 in first byte of short headers guaranteed to always indicate presence/absence of connection ID in short headers across all versions?
Is bit 7 in first byte guaranteed to alwats indicate long headers across all versions?

A version specific change could relate to co-existence with other protocols. This means very little should be decided about the first byte across versions. But enough to detect version negotiation.

Since detecting presence of version field puts restraints on the first byte it might be good to be very specific about which bits in the first byte remain unaffected across versions.

One the use of random values for greasing, I'm concerned that a dominating implementation in some environments might be sloppy about randomness and just put in the value 0, or 4 always, or by careless with PRNG exposure. I'd prefer some sort of hashing. It could be something simple like FNV-1a or Fletcher-16 seeded by 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/968#issuecomment-348307828

---

From: Marten Seemann <[email-masked]>
Date: Thu, 30 Nov 2017 20:19:38 +0000 (UTC)

~~~

The principle I'm advocating here (as well as in #426) is that a packet header should be identifiable by just looking at its type byte. So the parsing logic would be:

```
ParseHeader(our_version) {
  if(packet.type_byte == kTypeByteVersionNegotiation) {
    // parse version negotiation packet
  }
  else {
    // version dependent parsing of the packet header (version is our_version)
  }
}
```

Note that the **only** thing in the packet you have to look at to correctly parse the header is the type byte.

-- 
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/968#discussion_r154189840

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 20:56:21 +0000 (UTC)

~~~

We should move this to #963, since this is a discussion about whether the issue in #963 is a problem or a feature.

-- 
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/968#discussion_r154198668

---

From: Mike Bishop <[email-masked]>
Date: Thu, 30 Nov 2017 21:01:17 +0000 (UTC)

Martin has a draft with limited circulation so far pulling out the invariants into a separate document that hopefully addresses some of these concerns, @mikkelfj.  The current proposal in that draft is close to what's in 5.8, though:

- The long/short header bit
- The Connection ID and Version fields in long headers
- The Connection ID bit, and the Connection ID field if present, in short 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/968#issuecomment-348319481

---

From: Marten Seemann <[email-masked]>
Date: Thu, 30 Nov 2017 21:04:29 +0000 (UTC)

~~~

Thanks. I must have missed that issue. I'll write up my argument there.

-- 
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/968#discussion_r154200587

---

From: Martin Thomson <[email-masked]>
Date: Thu, 30 Nov 2017 17:48:37 -0800

martinthomson 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/968#pullrequestreview-80387016

---

From: Christian Huitema <[email-masked]>
Date: Thu, 30 Nov 2017 17:56:34 -0800

huitema commented on this pull request.

 ~~~

This looks like a gratuitous change. What exactly is the benefit of not having a version renegotiation packet?

-- 
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/968#discussion_r154252173

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 17:57:59 -0800

I like this PR, but there's ongoing discussion on the issue, so please wait before committing.

-- 
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/968#pullrequestreview-80388226

---

From: janaiyengar <[email-masked]>
Date: Fri, 01 Dec 2017 01:58:56 +0000 (UTC)

~~~

@huitema: can we discuss on #963 ?

-- 
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/968#discussion_r154252403

---

From: Martin Thomson <[email-masked]>
Date: Thu, 30 Nov 2017 18:10:11 -0800

I looks like the only point of contention is the value we put in the type field.  Hopefully the draft I'm about to publish will help clarify.  That draft assumes that this PR is merged.  Switching based on the type byte is possible, and it won't appreciably change the logic.

There is no case where you don't check version before handling a packet with a long header.  So there aren't any more or fewer checks, just that they are in a different place.

-- 
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/968#issuecomment-348381158

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 21:38:13 -0800

@martinthomson : Agreed, but if you remove that question, i.e., assume that a type is assigned to a VN packet, then the only thing left in this PR is the removal of packet number from the VN packet. I am hearing pushback from the wg on the question of whether the VN type byte should be version-dependent, which is important to this particular PR, and we should discuss on #963 first. It's ok for your draft to assume this PR in the meanwhile.

-- 
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/968#issuecomment-348406236