Subject: [quicwg/base-drafts] Discard inconsistent packets (#2431)

From: Mike Bishop <[email-masked]>
Date: Wed, 06 Feb 2019 19:06:29 +0000 (UTC)

Fixes #2397 by discarding packets which are "inconsistent with the state of the connection."  Previously, this was only a failure to remove packet protection; incorrect version is added as another example inconsistency.

Note that this phrasing doesn't rule out upward-VN like #1773, since receiving an expected upgrade is not inconsistent with your local state.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Discard inconsistent packets

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Wed, 06 Feb 2019 12:26:05 -0800

ianswett 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/2431#pullrequestreview-200787572

---

From: MikkelFJ <[email-masked]>
Date: Wed, 06 Feb 2019 13:35:25 -0800

mikkelfj commented on this pull request.

> @@ -1010,8 +1010,10 @@ Endpoints can send a Stateless Reset ({{stateless-reset}}) for any packets that
 cannot be attributed to an existing connection. A stateless reset allows a peer
 to more quickly identify when a connection becomes unusable.
 
-Packets that are matched to an existing connection, but for which the endpoint
-cannot remove packet protection, are discarded.
+Packets that are matched to an existing connection are discarded if the packets
+are inconsistent with the state of that connection -- for example, if they
+indicate a different protocol version than that of the connection, or if the
+endpoint cannot remove packet protection.

During handshake, reordering can lead to temporarily undecryptable content which may be discarded or buffered. This is technically not inconsistent with state, but the above formulation could suggest to always drop such 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/2431#pullrequestreview-200816939

---

From: Mike Bishop <[email-masked]>
Date: Wed, 06 Feb 2019 16:42:54 -0800

MikeBishop commented on this pull request.

Perhaps "if removal of packet protection fails" would exclude the case where it can't be attempted yet?

-- 
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/2431#discussion_r254510382

---

From: Mike Bishop <[email-masked]>
Date: Wed, 06 Feb 2019 16:46:03 -0800

@MikeBishop pushed 1 commit.

105dd3213812cde18c65240efc7ae1f339fae65b  Removal is unsuccessful

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2431/files/e94bdc2cafb07ee711b91281591dc45637467bae..105dd3213812cde18c65240efc7ae1f339fae65b

---

From: Martin Thomson <[email-masked]>
Date: Wed, 06 Feb 2019 18:22:48 -0800

martinthomson commented on this pull request.

Two sentences > 1.

-- 
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/2431#pullrequestreview-200897801

---

From: MikkelFJ <[email-masked]>
Date: Wed, 06 Feb 2019 23:15:12 -0800

+removal of packet protection is unsuccessful.

```suggestion
removal of packet protection is unsuccessful once the expected keys are available.
```

-- 
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/2431#pullrequestreview-200944522

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Feb 2019 15:55:41 -0800

5b3abdedf454cccec4e2e0bbb1b5dcc3de261b46  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/2431/files/105dd3213812cde18c65240efc7ae1f339fae65b..5b3abdedf454cccec4e2e0bbb1b5dcc3de261b46

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Feb 2019 15:57:21 -0800

5369ff2cf70a461899817aec983ac22e23684f15  Sentence split

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2431/files/5b3abdedf454cccec4e2e0bbb1b5dcc3de261b46..5369ff2cf70a461899817aec983ac22e23684f15

---

From: MikkelFJ <[email-masked]>
Date: Fri, 08 Feb 2019 23:31:15 -0800

I'm wondering if there is a difference between failed decryption and version number.

If the versions differ, it doesn't really make sense to try to read a subsequent coalesced packet, unless you want to do trial and error since the concepts of coalescing and length fields are not defined in the general case. Even if you understand the version, two version can not (or should not?) exist in the same datagram.

If packet protection fails on the same version, it might just be a timing issue, or the (to me) dreaded path probe with prefixed knowingly invalid handshake.

-- 
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/2431#issuecomment-462021997

---

From: Martin Thomson <[email-masked]>
Date: Sun, 10 Feb 2019 16:25:34 -0800

Merged #2431 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/2431#event-2129515936

---

From: Mike Bishop <[email-masked]>
Date: Mon, 11 Feb 2019 16:17:48 -0800

That's an interesting question.  The Length field is version-specific.  If you include it, you can have no expectation that a peer which doesn't speak that version will even be able to find anything further in the datagram -- as far as it knows, the remainder of the datagram is that connection.  But that actually has a potential use.

On receipt, if the logic first attempts to chunk up the datagram and the subsequent packet is the correct version for the connection, then acknowledging that second coalesced packet implies that you support the first packet's version.

-- 
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/2431#issuecomment-462550705