Subject: [quicwg/base-drafts] clarify that old keys should always be dropped (#2191)

From: Marten Seemann <[email-masked]>
Date: Fri, 14 Dec 2018 22:35:26 -0800

The old text said
> Keys and their corresponding secrets SHOULD be discarded when an endpoint has
received all packets with packet numbers lower than the lowest packet number
used for the new key. An endpoint might discard keys if it determines that the
length of the delay to affected packets is excessive.

A peer typically can't know when it received all packets with packet numbers lower than the lowest packet number used for the new key, since packets might have been lost or packet number might have been skipped. Dropping old keys shouldn't be a "might", it should be a SHOULD.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * clarify that old keys should always be dropped

-- File Changes --

    M draft-ietf-quic-tls.md (15)

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Dec 2018 18:55:52 -0800

martinthomson commented on this pull request.

I was going to suggest a few things, then I made https://github.com/marten-seemann/base-drafts/pull/1

> @@ -1108,16 +1108,11 @@ updated keys, it indicates that its peer has updated keys twice without awaiting
 a reciprocal update.  An endpoint MUST treat consecutive key updates as a fatal
 error and abort the connection.
 
-An endpoint SHOULD retain old keys for a short period to allow it to decrypt
-packets with smaller packet numbers than the packet that triggered the key
-update.  This allows an endpoint to consume packets that are reordered around
-the transition between keys.  Packets with higher packet numbers always use the
-updated keys and MUST NOT be decrypted with old keys.
-
-Keys and their corresponding secrets SHOULD be discarded when an endpoint has
-received all packets with packet numbers lower than the lowest packet number
-used for the new key.  An endpoint might discard keys if it determines that the
-length of the delay to affected packets is excessive.
+An endpoint SHOULD retain old keys for a short period, and discard them and
+the corresponding secrets afterwards.  This allows it to decrypt packets with
+smaller packet numbers than the packet that triggered the key update, which are
+reordered around the transition between keys.  Packets with higher packet

```suggestion
Packets with higher packet
```

> @@ -1108,16 +1108,11 @@ updated keys, it indicates that its peer has updated keys twice without awaiting
 a reciprocal update.  An endpoint MUST treat consecutive key updates as a fatal
 error and abort the connection.
 
-An endpoint SHOULD retain old keys for a short period to allow it to decrypt
-packets with smaller packet numbers than the packet that triggered the key
-update.  This allows an endpoint to consume packets that are reordered around
-the transition between keys.  Packets with higher packet numbers always use the
-updated keys and MUST NOT be decrypted with old keys.
-
-Keys and their corresponding secrets SHOULD be discarded when an endpoint has
-received all packets with packet numbers lower than the lowest packet number
-used for the new key.  An endpoint might discard keys if it determines that the
-length of the delay to affected packets is excessive.
+An endpoint SHOULD retain old keys for a short period, and discard them and
+the corresponding secrets afterwards.  This allows it to decrypt packets with
+smaller packet numbers than the packet that triggered the key update, which are

```suggestion
that were sent with the old keys, but were delayed in the network.
```

> @@ -1108,16 +1108,11 @@ updated keys, it indicates that its peer has updated keys twice without awaiting
 a reciprocal update.  An endpoint MUST treat consecutive key updates as a fatal
 error and abort the connection.
 
-An endpoint SHOULD retain old keys for a short period to allow it to decrypt
-packets with smaller packet numbers than the packet that triggered the key
-update.  This allows an endpoint to consume packets that are reordered around
-the transition between keys.  Packets with higher packet numbers always use the
-updated keys and MUST NOT be decrypted with old keys.
-
-Keys and their corresponding secrets SHOULD be discarded when an endpoint has
-received all packets with packet numbers lower than the lowest packet number
-used for the new key.  An endpoint might discard keys if it determines that the
-length of the delay to affected packets is excessive.
+An endpoint SHOULD retain old keys for a short period, and discard them and
+the corresponding secrets afterwards.  This allows it to decrypt packets with

```suggestion
the corresponding secrets afterwards.  This allows it to decrypt 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/2191#pullrequestreview-185424401

---

From: Marten Seemann <[email-masked]>
Date: Sun, 16 Dec 2018 21:05:50 -0800

@marten-seemann pushed 2 commits.

74ea30bb75b3edf5511cdf9ad3fe1eaf7eb1bad8  Key discard requirements
ab1cdf44338acefa34583d7f0d9159681602002d  Merge pull request #1 from quicwg/drop-old

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2191/files/99e488afefcca585bb4eff639430c273457605e7..ab1cdf44338acefa34583d7f0d9159681602002d

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 16 Dec 2018 21:12:03 -0800

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/2191#pullrequestreview-185441201

---

From: ianswett <[email-masked]>
Date: Mon, 17 Dec 2018 12:34:03 -0800

ianswett commented on this pull request.

> @@ -1108,16 +1108,12 @@ updated keys, it indicates that its peer has updated keys twice without awaiting
 
+An endpoint SHOULD retain old keys for a period of no more than three times the
+Retransmitions Timeout (RTO, see {{QUIC-RECOVERY}}).  After this period, old

It's now Probe Timeouts, aka PTO.

-- 
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/2191#pullrequestreview-185786091

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 02:48:38 -0800

Missed a few then.

```
1789:Retransmittion Timeout (RTO) as defined in {{QUIC-RECOVERY}} is RECOMMENDED.
2182:(RTO) interval as defined in {{QUIC-RECOVERY}}.
2253:draining state before the packets arrive.  If a peer could timeout within an RTO
```

-- 
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/2191#discussion_r242488389

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 02:49:33 -0800

@martinthomson pushed 1 commit.

bf27237f946cae8e0f5dfa81c60b12caa0c70ba4  Probe Timeout/PTO

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

---

From: ianswett <[email-masked]>
Date: Tue, 18 Dec 2018 07:17:20 -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/2191#pullrequestreview-186120115

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Dec 2018 15:42:25 -0800

Merged #2191 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/2191#event-2034393486