Subject: [quicwg/base-drafts] Let server abort on post-Retry packet number reset (#3990)

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 10 Aug 2020 05:40:29 -0700

Fixes issue #3989.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Let server abort on post-Retry packet number reset

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Mon, 10 Aug 2020 15:57:42 -0700

@ianswett commented on this pull request.

> @@ -4807,6 +4807,8 @@ responding to a Retry packet. However, the data sent in these packets could be
 different than what was sent earlier. Sending these new packets with the same
 packet number is likely to compromise the packet protection for those packets
 because the same key and nonce could be used to protect different content.
+A server MAY abort the connection if it detects that the client reset the

If we're making a normative change, I'd argue for SHOULD

-- 
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/3990#pullrequestreview-464641969

---

From: Mike Bishop <[email-masked]>
Date: Tue, 11 Aug 2020 13:25:16 -0700

@MikeBishop approved this pull request.

MAY or SHOULD abort if detects, but no requirement that it attempt to detect seems like a reasonable compromise.

-- 
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/3990#pullrequestreview-465401215

---

From: ianswett <[email-masked]>
Date: Tue, 11 Aug 2020 15:51:13 -0700

@ianswett approved this pull request.

MAY SGTM

-- 
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/3990#pullrequestreview-465480545

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 15:52:12 -0700

@janaiyengar 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/3990#pullrequestreview-465480933

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Aug 2020 20:26:09 -0700

@kazuho 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/3990#pullrequestreview-472121486

---

From: Christian Huitema <[email-masked]>
Date: Fri, 21 Aug 2020 08:25:26 -0700

@huitema 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/3990#pullrequestreview-472584751

---

From: Nick Banks <[email-masked]>
Date: Fri, 21 Aug 2020 08:26:43 -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/3990#pullrequestreview-472585740

---

From: Christian Huitema <[email-masked]>
Date: Sun, 23 Aug 2020 10:37:50 -0700

I know we have consensus and are ready to merge this PR, but... have we considered what error code the server shall use? Implementing this, the easiest for me is INVALID_TOKEN. Is that the obvious intent 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/3990#issuecomment-678802626

---

From: Marten Seemann <[email-masked]>
Date: Sun, 23 Aug 2020 19:05:05 -0700

INVALID_TOKEN seems wrong to me. Why not PROTOCOL_VIOLATION?

-- 
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/3990#issuecomment-678866181

---

From: ekr <[email-masked]>
Date: Fri, 28 Aug 2020 10:20:32 -0700

@ekr commented on this pull request.

I don't object to this particular decision, but I would have assumed that you could *always* abort a connection whenever the peer violates a MUST. No?

-- 
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/3990#discussion_r479437371

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 28 Aug 2020 12:20:23 -0700

@janaiyengar commented on this pull request.

Yes, that is generally true, and this makes it more explicit without adding any requirement on the server.

-- 
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/3990#discussion_r479491780

---

From: ekr <[email-masked]>
Date: Fri, 28 Aug 2020 12:31:25 -0700

Hmm... So I read this the opposite way, which is that an explicit mention like this implies that there is not a general rule

-- 
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/3990#discussion_r479496412

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 28 Aug 2020 13:19:23 -0700

We certainly don't mention a general rule anywhere, but we can't stop an endpoint from enforcing MUSTs on its own. I think it's useful to explicitly call it out out here because this is the sort of place where we want to call explicit attention. 

I would have liked a SHOULD here, but calling it out with a MAY is still useful I think.

-- 
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/3990#discussion_r479516271

---

From: ekr <[email-masked]>
Date: Fri, 28 Aug 2020 13:27:37 -0700

> We certainly don't mention a general rule anywhere, but we can't stop an endpoint from enforcing MUSTs on its own.

Well, we can't stop them in the "no protocol police" sense, but we certainly can have a MUST-level rule that forbids it.

>  I think it's useful to explicitly call it out out here because this is the sort of place where we want to call explicit attention.

Why?

My general point is that text should do work. And if there is already a general rule permitting to you to error on MUST violations then why does this text need to be there? And working backward from that one might infer that no such rule exists.

-- 
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/3990#discussion_r479519667

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 28 Aug 2020 18:19:59 -0700

My argument for why here is because this PN reuse can be a security issue.

That said, you're right that it's not as principled as I would like it to be; there also isn't such a general rule, and you make a good point that it might be useful to have a general rule.

Given that we don't have such a rule at the moment, I would be happy to take this text in and evaluate this with other MAYs in the general sense if there's enough interest to do it. Would you mind filing a separate issue for this?

-- 
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/3990#discussion_r479589737