Re: [quicwg/base-drafts] forbid key discarding before receiving a packet protected with new keys (#4079)

[Kazuho Oku <[e-mail]>]

@kazuho commented on this pull request.

Thank you for working on the PR. I like the changes to section 6.5. Regarding changes to section 6.1, please see my comments below.

> @@ -1496,10 +1496,11 @@ The endpoint that initiates a key update also updates the keys that it uses for
 receiving packets.  These keys will be needed to process packets the peer sends
 after updating.
 
-An endpoint SHOULD retain old keys so that packets sent by its peer prior to
-receiving the key update can be processed.  Discarding old keys too early can
-cause delayed packets to be discarded.  Discarding packets will be interpreted
-as packet loss by the peer and could adversely affect performance.
+An endpoint MUST retain old keys until it has successfully unprotected a packet
+sent using the new keys.  An endpoint SHOULD NOT discard old keys immediately
+after unprotecting a packet sent using the new keys.  Discarding old keys too
+early can cause delayed packets to be discarded.  Discarding packets will be
+interpreted as packet loss by the peer and could adversely affect performance.

Regarding the first sentence, I think SHOULD works too, but I agree that MUST is better here.

I do not think we need the second sentence, as we state that an endpoint "SHOULD retain no more than 3 PTO" down below.

I think the crux of the problem is that the last two sentences that are left unmodified. As correctly pointing out by the issue, the outcome of discarding the keys too early is more than just a performance degradation. Doing so might kill the connection. We should state that clearly, and then the choice of the keyword in the first sentence becomes more of an editorial choice.

-- 
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/4079#pullrequestreview-484061184