Subject: [quicwg/base-drafts] Change text that describes version negotiation in terms of crypto keys. (#141)

From: janaiyengar <[email-masked]>
Date: Wed, 11 Jan 2017 23:11:50 -0800

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Change text that describes version negotiation in terms of crypto keys.

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Thu, 12 Jan 2017 09:01:32 -0800

ianswett commented on this pull request.

I support the change Martin was trying to make, but I think it could use a bit more clarification, as the existing text is a bit confusing, because it doesn't explain why the version must be specified, even though version negotiation has technically completed.

As such, I'd support a merge of the functional change Martin made with some combined text?

> @@ -411,8 +411,7 @@ have the VERSION bit set.  This bit is always set on packets that are sent prior
 to connection establishment.  When receiving a packet that is not associated
 with an existing connection, packets without a VERSION bit MUST be discarded.
 
-While there might be similarities between different versions of this protocol,
-implementations have to assume that a version that it does not support uses a
+Implementations have to assume that a version that it does not support uses a

nit: How about "Implementations must assume an unsupported version may use a different packet format."?

> @@ -656,13 +655,15 @@ version. The resent packets MUST use new packet numbers.  These packets MUST
 continue to have the VERSION flag set and MUST include the new negotiated
 protocol version.
 
-The client MUST include its selected version on all packets until it starts
-protecting packets with 1-RTT keys.  Only unprotected packets and 0-RTT
-protected packets can include a version.  A client MUST NOT change the version
-it uses unless it is in response to a version negotiation packet from the
-server.
+The client MUST set the VERSION flag on all packets until version negotiation 

Is this section just a rollback?

-- 
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/141#pullrequestreview-16403844

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Jan 2017 10:02:04 -0800

janaiyengar commented on this pull request.

Done, with a minor edit.

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Jan 2017 10:03:58 -0800

My goal is to separate this functional change into a separate smaller PR, separate from the additions to the draft that have happened in the other PR. Technically, if we aren't requiring TLS in the QUIC document, talking about 0-RTT and 1-RTT keys pre-supposes a specific mechanism, which, if universal, ought to be documented in the "Crypto Handshake Requirements", or should be removed. Either ways, this should happen separately, so I'm changing the text back to what it described, modulo Martin's nice editorializing.

-- 
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/141#issuecomment-272236277

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Jan 2017 10:03:53 -0800

@janaiyengar pushed 1 commit.

eecb8d9  Address Ian's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/141/files/99946d15e0102c2c344e7d603fc4fa63122fae73..eecb8d907341a203030078436e4eaf5e3f954a92

---

From: ianswett <[email-masked]>
Date: Thu, 12 Jan 2017 10:20:01 -0800

ianswett approved this pull request.

Thanks Jana, one minor tweak, but otherwise it LGTM.

-The client MUST include its selected version on all packets until it starts
-protecting packets with 1-RTT keys.  Only unprotected packets and 0-RTT
-protected packets can include a version.  A client MUST NOT change the version
-it uses unless it is in response to a version negotiation packet from the
-server.
+The client MUST set the VERSION flag on all packets until version negotiation 
+concludes. Version negotiation successfully concludes when the client receives a
+packet from the server with the VERSION flag unset. All subsequent packets sent
+by the client MUST have the VERSION flag unset.

Now that I've discussed this more, how about just changing this to SHOULD, since the MUST is not enforcable anyway.

-- 
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/141#pullrequestreview-16421025

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Jan 2017 10:20:30 -0800

5a636ef  MUST -> SHOULD

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/141/files/eecb8d907341a203030078436e4eaf5e3f954a92..5a636ef3338e17a5cc94f3acdaefc893ecbaf2e1

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Jan 2017 10:21:52 -0800

Merged #141.

-- 
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/141#event-921154856

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Jan 2017 16:33:41 -0800

martinthomson commented on this pull request.

It is absolutely enforceable.  Unless your point is that the presence of absence of the version field isn't authenticated, in which case I guess I'd have to concede that you don't want an attacker ADDING a version to your packet so that you would kill the connection.

If we are going with a SHOULD here, we need to explain why (and the consequences if the SHOULD is not respected, I'll open a small PR).