Subject: Re: [quicwg/base-drafts] Duplicates are dropped (#1624)

From: Martin Thomson <[email-masked]>
Date: Wed, 01 Aug 2018 00:23:39 -0700

I'm going to hold the line on MUST.  The protocol doesn't make any sense if you get (valid) duplicates and don't discard them.  Given that you have to track what you are acknowledging, then the only additional cost is the threshold, which is nearly zero.  It can even *be* zero if you use the minimum value from what you are tracking.

What do you gain by having anything less than MUST?  That is, other than the potential for crazy non-determinism.

-- 
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/1624#issuecomment-409476533

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 01 Aug 2018 00:49:50 -0700

> The protocol doesn't make any sense if you get (valid) duplicates and don't discard them.

I think we have different opinions here. My understanding is that processing of a “valid” duplicate (i.e, a packet that has the same PN and same payload as what have been observed earlier) is perfectly fine, because processing the payload of such packet does not cause any change in state. Processing of previously-observed frames are guaranteed to be idempotent.

-- 
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/1624#issuecomment-409483467

---

From: MikkelFJ <[email-masked]>
Date: Wed, 01 Aug 2018 00:54:58 -0700

I tend to agree with @kazuho but on the other hand, taking that position forces all future frames and extensions to be idempotent. I'm not sure that is a good tradeoff.

-- 
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/1624#issuecomment-409484842

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 01 Aug 2018 01:01:10 -0700

@mikkelfj I’d argue that any frame that cause a change in state needs to be retransmittable. We allow that by using different PNs.

In my view, what we are arguing about is if we need to have a special rule for the case of receiving a “retransmitted” frame using same PN.

-- 
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/1624#issuecomment-409486602

---

From: MikkelFJ <[email-masked]>
Date: Wed, 01 Aug 2018 01:11:01 -0700

> In my view, what we are arguing about is if we need to have a special rule for the case of receiving a “retransmitted” frame using same PN.

Which, as I see it, is the same as arguing if all frames must be idempotent. I do not disagree here.

-- 
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/1624#issuecomment-409489359

---

From: ekr <[email-masked]>
Date: Wed, 01 Aug 2018 05:29:58 -0700

I'm with @mt here. It's not that hard to implement the now-conventional
anti-replay techniques, and if we don't do this, then we have to analyze
every single QUIC frame to guarantee it can't cause problems if replayed
(and moreover implementors need to enforce it). To take a simple example
from earlier, consider the flow control frames: if we require duplicate
suppression, then even if you miss the rule about things being
monotonically increasing, things are fine. However, if duplicates are
allowed, and you make that mistake, then attackers can just replay a flow
control frame and force you into a bad posture.

On Wed, Aug 1, 2018 at 12:23 AM, Martin Thomson <[email-masked]>
wrote:

> I'm going to hold the line on MUST. The protocol doesn't make any sense if
> you get (valid) duplicates and don't discard them. Given that you have to
> track what you are acknowledging, then the only additional cost is the
> threshold, which is nearly zero. It can even *be* zero if you use the
> minimum value from what you are tracking.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/pull/1624#issuecomment-409476533>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABD1oWWTUfo-YA7nAp5RTM_MbNfsmPxAks5uMVd6gaJpZM4Vp4qq>
> .
>

-- 
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/1624#issuecomment-409557936

---

From: ekr <[email-masked]>
Date: Wed, 01 Aug 2018 05:38:15 -0700

ekr commented on this pull request.

> @@ -1065,6 +1066,12 @@ sending a packet with a number of 0x6b2d79 requires a packet number encoding
 with 14 bits or more; whereas the 30-bit packet number encoding is needed to
 send a packet with a number of 0x6bc107.
 
+An endpoint MUST discard packets with duplicate packet numbers.  That is, a
+packet is discarded if the reconstructed packet number is equal to the
+reconstructed packet number of a previously received and processed packet.  Note
+however that endpoints MUST attempt to remove packet protection before
+discarding packets (see Section 9.3 of {{QUIC-TLS}}).

I think i would put this in the TLS document S 5.4 instead. That way it's in logical order and this requirement seems like less of an aside.

Perhaps:

```
After unprotecting a packet, the receiver MUST check that it has not already processed
a packet with this PN and if so discard it. Duplicate suppression MUST happen after
the packet has been unprotected for the reasons described in Section 9.3. An efficient
algorithm for duplicate suppression can be found in RFC 2406 Section 3.4.3.```

-- 
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/1624#pullrequestreview-142354772

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 01 Aug 2018 15:52:02 -0700

@ekr 
> I'm with @mt here. It's not that hard to implement the now-conventional anti-replay techniques, and if we don't do this, then we have to analyze every single QUIC frame to guarantee it can't cause problems if replayed (and moreover implementors need to enforce it).

My understanding is that we need to analyze and guarantee that, regardless of if we require deduplication. This is because a peer can always replay a frame.

Taking the flow control frames as an example, the fact that endpoints need to handle out-of-order delivery means that they need to take care of receiving smaller maximum values that it has seen before. The requirement does not go away by requiring deduplication.

-- 
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/1624#issuecomment-409750813

---

From: Martin Thomson <[email-masked]>
Date: Wed, 01 Aug 2018 17:12:54 -0700

It's very different in my opinion to have rules about frame duplication (which we clearly need for any frame that might be retransmitted), and packet duplication.  A properly duplicate packet - one with identical content - is easy, but I don't want to have to think about what happens when a packet isn't genuinely identical.  And deduplication is easy to require because it's easy to implement.

This isn't a rulebook we can hit people with.  So if you process the same packet twice and no one hears it, then maybe it doesn't matter, but the "MUST"-language we put here is part of what determines how the protocol is analyzed.  If we don't say "no duplicates", we made that analysis harder and the results weaker.

-- 
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/1624#issuecomment-409764639

---

From: Martin Thomson <[email-masked]>
Date: Wed, 01 Aug 2018 17:14:44 -0700

@martinthomson pushed 1 commit.

8ef8fee  Reword and point to RFC 2406

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1624/files/57c1d360d49a186605329375d5a5fcb6312d04b8..8ef8fee8910afe6e18e938345b73839c7e55debf

---

From: ianswett <[email-masked]>
Date: Wed, 01 Aug 2018 17:22:04 -0700

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/1624#pullrequestreview-142608857

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 01 Aug 2018 17:34:13 -0700

@martinthomson
> This isn't a rulebook we can hit people with. 

Ah. Thank you for the suggestion. That’s true. Then, I will not object against requiring deduplication if it helps analyse the protocol.

-- 
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/1624#issuecomment-409767864

---

From: janaiyengar <[email-masked]>
Date: Mon, 06 Aug 2018 13:32:59 -0700

I am sympathetic to Kazuho's argument that this language seems to require state that a receiver may not have. I think this is easily resolvable though. If a packet received is either a duplicate or if it is to the left of the receiver's "window" (the lowest packet number that the receiver is willing to receive and process), the receiver drops it. I think this handles the cases where the receiver may not be able to dedup, but the outcome is the same -- the receiver drops the 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/1624#issuecomment-410843904

---

From: ianswett <[email-masked]>
Date: Tue, 07 Aug 2018 18:34:43 +0000 (UTC)

ianswett commented on this pull request.

> @@ -1065,6 +1066,13 @@ sending a packet with a number of 0x6b2d79 requires a packet number encoding
 
+An endpoint MUST discard packets with duplicate packet numbers.  After

How about combining the first two sentences, since they seem largely redundant? Maybe "An endpoint MUST discard packets with duplicate packet numbers after unprotecting the packet." or something similar.

-- 
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/1624#pullrequestreview-144129346

---

From: Mike Bishop <[email-masked]>
Date: Tue, 07 Aug 2018 20:58:14 +0000 (UTC)

I'm mostly in agreement with Kazuho from a theoretical perspective -- there is no harm in processing a true duplicate, and that's part of what the idempotency language was about.  My inclination is SHOULD discard, but MUST NOT send more than one packet with the same packet number.  If an attacker can spoof a packet into being valid, you've got bigger problems.

However, "MUST discard packets which can't be determined not to be a duplicate" is also workable.  There's no correctness loss either way.

-- 
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/1624#issuecomment-411200253

---

From: Mike Bishop <[email-masked]>
Date: Tue, 07 Aug 2018 20:59:35 +0000 (UTC)

MikeBishop commented on this pull request.

+An endpoint MUST discard packets with duplicate packet numbers.  After
+unprotecting a packet, the receiver MUST check that it has not already processed
+a packet with this PN and if so discard it. Duplicate suppression MUST happen

The receiver isn't necessarily able to confirm that for all packet numbers.  I think the language needs to be that if the receiver MUST discard if it can't verify that it hasn't processed this packet number before.

-- 
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/1624#pullrequestreview-144182379

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Aug 2018 19:16:25 -0700

5808eb8  Certainty is about not received

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1624/files/8ef8fee8910afe6e18e938345b73839c7e55debf..5808eb81a6e0a6a0cde7e2f4b00421d545b26687

---

From: Martin Thomson <[email-masked]>
Date: Thu, 09 Aug 2018 23:05:54 -0700

Merged #1624 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/1624#event-1781753750