Subject: Re: [quicwg/base-drafts] Mandatory distinction between token types (#3128)

From: Jana Iyengar <[email-masked]>
Date: Fri, 01 Nov 2019 15:02:26 -0700

janaiyengar commented on this pull request.

A few comments. 

> @@ -1623,6 +1623,14 @@ also constrained in what they can send by the limits set by the congestion
 controller.  Clients are only constrained by the congestion controller.

+### Token Construction
+
+Tokens sent in NEW_TOKEN frames or Retry packets MUST be constructed in a way

```suggestion
A token sent in a NEW_TOKEN frame or a Retry packet MUST be constructed in a way
```

> @@ -1675,22 +1683,20 @@ one.  The client MUST NOT use the token provided in a Retry for future
 connections. Servers MAY discard any Initial packet that does not carry the
 expected token.
 
-A token SHOULD be constructed in a way that allows the server to distinguish it
-from tokens that are sent in Retry packets as they are carried in the same
-field.
-
-The token MUST NOT include information that would allow it to be linked by an
-on-path observer to the connection on which it was issued.  For example, it
-cannot include the connection ID or addressing information unless the values are
-encrypted.
-
 Unlike the token that is created for a Retry packet, there might be some time
 between when the token is created and when the token is subsequently used.
 Thus, a token SHOULD have an expiration time, which could be either an explicit

Make this a MUST too? I don't see why this is a SHOULD.

>  Unlike the token that is created for a Retry packet, there might be some time
 between when the token is created and when the token is subsequently used.
 Thus, a token SHOULD have an expiration time, which could be either an explicit
 expiration time or an issued timestamp that can be used to dynamically calculate
 the expiration time.  A server can store the expiration time or include it in an
 encrypted form in the token.
 
+A token issued with NEW_TOKEN MUST NOT include information that would allow
+values to be linked by an on-path observer to the connection on which it was
+issued.  For example, it cannot include the previous connection ID or addressing
+information unless the values are encrypted.  Information that allows the server

Move this "unless" clause up to the first sentence. The unless applies to the 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/3128#pullrequestreview-310713842

---

From: Marten Seemann <[email-masked]>
Date: Fri, 01 Nov 2019 18:46:52 -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/3128#pullrequestreview-310750165

---

From: Martin Thomson <[email-masked]>
Date: Sun, 03 Nov 2019 15:39:52 -0800

@martinthomson pushed 1 commit.

311d87043248d5862f3e181e9045f90bdf73239d  Jana's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3128/files/6b6273302247ffb91797391218850f95dae0b4af..311d87043248d5862f3e181e9045f90bdf73239d

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 04 Nov 2019 17:24:58 -0800

ping

-- 
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/3128#discussion_r342347676

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 04 Nov 2019 17:25:49 -0800

One suggestion to change a normative to a MUST, on existing text.

-- 
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/3128#pullrequestreview-311458217

---

From: Martin Thomson <[email-masked]>
Date: Mon, 04 Nov 2019 18:02:31 -0800

martinthomson commented on this pull request.

Problems with existing text can be taken up on a new 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/3128#discussion_r342354671

---

From: Martin Thomson <[email-masked]>
Date: Thu, 28 Nov 2019 17:04:39 -0800

Merged #3128 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/3128#event-2840092529