Subject: [quicwg/base-drafts] Immediately close with INVALID_TOKEN (#3107)

From: ianswett <[email-masked]>
Date: Wed, 16 Oct 2019 19:41:07 -0700

If the Retry token is known to be invalid by the server, then the server can close the connection with INVALID_TOKEN instead of waiting for a timeout.

Came out of discussion of Issue #3014
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Immediately close with INVALID_TOKEN

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Wed, 16 Oct 2019 19:43:19 -0700

@ianswett pushed 1 commit.

24d4b635a6348565fe16e8ee20b0e0e34ad5b590  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3107/files/1a563f23d8de25376eb56fcb54133968fe72a88d..24d4b635a6348565fe16e8ee20b0e0e34ad5b590

---

From: ianswett <[email-masked]>
Date: Wed, 16 Oct 2019 19:48:26 -0700

8c4691476c3cd896f74ff58402b48c650ac032e6  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3107/files/24d4b635a6348565fe16e8ee20b0e0e34ad5b590..8c4691476c3cd896f74ff58402b48c650ac032e6

---

From: Nick Banks <[email-masked]>
Date: Wed, 16 Oct 2019 21:07:09 -0700

You need to update the "Transport Error Codes" section too.

-- 
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/3107#issuecomment-542990466

---

From: ianswett <[email-masked]>
Date: Wed, 16 Oct 2019 21:40:20 -0700

Thanks @nibanks I did that in a subsequent update.  Refresh and tell me if it looks correct.

-- 
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/3107#issuecomment-542997845

---

From: Nick Banks <[email-masked]>
Date: Thu, 17 Oct 2019 08:08:49 -0700

@ianswett you're missing the section that gives some text to each error code `#error-codes`.

-- 
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/3107#issuecomment-543220311

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 17 Oct 2019 10:16:17 -0700

kazuho commented on this pull request.

> @@ -1641,6 +1641,14 @@ of connection establishment.  By giving the client a different connection ID to
 use, a server can cause the connection to be routed to a server instance with
 more resources available for new connections.
 
+If a server receives a client Initial with a unverifiable Retry token,

Maybe s/unverifiable/invalid/?

To me it seems that "unverifiable" might have the implication of the decryption process (or the integrity check) of the token failing. But a rejection of Retry token happens in other cases too, e.g., when the Retry token is outdated.

-- 
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/3107#pullrequestreview-303415556

---

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

martinthomson requested changes on this pull request.

+If a server receives a client Initial with a unverifiable Retry token,
+it knows the client will not accept another Retry token.  It can either
+proceed with the handshake without verifying the token or immediately close

I don't think that we should allow the server to continue.  Why not make the INVALID_TOKEN mandatory?

+If a server receives a client Initial with a unverifiable Retry token,
+it knows the client will not accept another Retry token.  It can either
+proceed with the handshake without verifying the token or immediately close
+({{immediate-close}}) the connection with an connection error of
+INVALID_TOKEN to cause the handshake to fail quickly instead of waiting
+for the client to timeout.  When a client receives a CONNECTION_CLOSE
+with an error of INVALID_TOKEN, it MAY create a new connection.

This isn't a very useful "MAY", because we couldn't stop a client from doing this.  We never say anything about making other connections anywhere else.

-- 
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/3107#pullrequestreview-304925455

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 21 Oct 2019 19:27:01 -0700

If we are to make this a mandatory requirement, under the assumption that the servers would act like that, should we also suggest that a server should issue tokens in a way that a Retry token and a NEW_TOKEN token are distinguishable from each other?

Unless we recommend servers building a token in that way, I do not think making this a MUST has benefits.

-- 
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/3107#discussion_r337311953

---

From: Marten Seemann <[email-masked]>
Date: Mon, 21 Oct 2019 19:30:35 -0700

marten-seemann commented on this pull request.

If we assume that servers only require Retries when they’re under attack, they might not bother checking the validity of a token when they’re not under attack.

-- 
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/3107#discussion_r337312373

---

From: Martin Thomson <[email-masked]>
Date: Mon, 21 Oct 2019 19:31:01 -0700

martinthomson commented on this pull request.

> 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.

-- [Section 8.1.2](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#validate-future)

We could strengthen that to MUST if you like.  It probably should be 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/3107#discussion_r337312437

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:19:33 +0000 (UTC)

ianswett commented on this pull request.

Done.

-- 
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/3107#discussion_r337318435

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:21:42 +0000 (UTC)

The server could decide it's not under heavy load and proceed with the handshake, even if the token doesn't constitute address validation.  ie: The token could expire on a shorter timeline than the session ticket/etc, so 0-RTT could succeed, but address validation would fail.

-- 
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/3107#discussion_r337318691

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:22:41 +0000 (UTC)

6cf5de8b6bce8470f614918cf149429ed327cf1a  Kazuho and MTs comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3107/files/8c4691476c3cd896f74ff58402b48c650ac032e6..6cf5de8b6bce8470f614918cf149429ed327cf1a

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:22:58 +0000 (UTC)

SG, sentence removed.

-- 
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/3107#discussion_r337318836

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Oct 2019 03:23:20 +0000 (UTC)

I'm persuaded by Marten's comment here (which I missed at first, I think because we commented at almost 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/3107#discussion_r337318870

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:25:47 +0000 (UTC)

b43b5bfb6962bfa87054ae8ed0eb96e18ce5be2a  Define the error

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3107/files/6cf5de8b6bce8470f614918cf149429ed327cf1a..b43b5bfb6962bfa87054ae8ed0eb96e18ce5be2a

---

From: ianswett <[email-masked]>
Date: Tue, 22 Oct 2019 03:28:16 +0000 (UTC)

f827ca30b70e4ccffcd5f94ce884b0c26584d37f  whitespace

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

---

From: Nick Banks <[email-masked]>
Date: Tue, 22 Oct 2019 07:07:32 -0700

nibanks 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/3107#pullrequestreview-305234780

---

From: Mike Bishop <[email-masked]>
Date: Thu, 24 Oct 2019 11:30:33 -0700

MikeBishop commented on this pull request.

Depending how the server distinguishes them, a change of method / confusion about what server you're sending it to could make you appear to be presenting a Retry token instead of a regular token.  This error code enables the client to recover and retry quickly, but there's no language about what the client should do when it receives the code.

I still think the requirement to make them distinguishable should be a MUST.  Otherwise, a change of token generation method could make an old NEW_TOKEN token appear to be an invalid Retry token.

-- 
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/3107#pullrequestreview-306776279

---

From: ianswett <[email-masked]>
Date: Thu, 24 Oct 2019 11:48:55 -0700

@MikeBishop do you have a suggestion about what the client should do?  Clearly it could start a new connection, but it would't want to do that an unlimited number of times, and it may depend upon whether a TCP fallback is available.  Given that, it wasn't clear I should recommend something.

And I agree this should be a MUST, as stated in #3128

-- 
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/3107#issuecomment-546052605

---

From: Martin Thomson <[email-masked]>
Date: Thu, 24 Oct 2019 14:35:48 -0700

I think that the best thing we can say is "the connection attempt failed".  The first version of the PR strongly suggested that making another connection attempt was the right thing to do, but that gets into lots of difficult questions about identifying why.  Given that this is an error that only happens if a Retry is spoofed, then I think that all we can say is:

![it's a trap](https://i.kym-cdn.com/entries/icons/original/000/000/157/itsatrap.jpg)

-- 
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/3107#issuecomment-546113385