Subject: Re: [quicwg/base-drafts] Rework Key Update (#2237)

From: Kazuho Oku <[email-masked]>
Date: Wed, 02 Jan 2019 17:08:22 -0800

FWIW, I *do* want to block key updates for a short while after establishing a connection, until the 0-RTT ingress key is retired.

Because then, we will have the guarantee that any incoming packet (unless severely outdated) can be decrypted just by retaining two decryption keys for the "application data space" (see https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#rfc.section.12.3) at any given time.

-- 
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/2237#issuecomment-451032746

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 17:52:52 -0800

@martinthomson pushed 2 commits.

4836c87e3b4317e847547101835ad96633f328a3  Review comments
8c72c8d98e9fc5d4538232089793bccf49a1beed  Use two bits for key update, lots of new text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2237/files/4d3f65aacb02e09cb9b4e0ca2f4d5b9c6adeb373..8c72c8d98e9fc5d4538232089793bccf49a1beed

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 17:53:27 -0800

I've made some fairly significant changes here along the lines of what @kazuho suggests.  I think that this is better, but I expect that this will need some discussion.

-- 
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/2237#issuecomment-452542738

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 08 Jan 2019 20:47:23 -0800

kazuho commented on this pull request.

Thank you for writing this down.

It was easy to read. I like the discussing immediate updates first then about delaying permission to restrict the number of keys. LGTM modulo the following comments.

-The KEY_PHASE bit allows a recipient to detect a change in keying material
-without necessarily needing to receive the first packet that triggered the
-change.  An endpoint that notices a changed KEY_PHASE bit can update keys and
-decrypt the packet that contains the changed bit.
+The the low bit of the Key Update field (0x04) is the Key Phase bit.  The Key

"The the"

-A packet that triggers a key update could arrive after successfully processing a
-packet with a higher packet number.  This is only possible if there is a key
-compromise and an attack, or if the peer is incorrectly reverting to use of old
-keys.  Because the latter cannot be differentiated from an attack, an endpoint
-MUST immediately terminate the connection if it detects this condition.
+An endpoint SHOULD retain old read keys for a period of no more than three times
+the Probe Timeout (PTO, see {{QUIC-RECOVERY}}).  After this period, old read
+keys and their corresponding secrets SHOULD be discarded.  An endpoint MAY keep
+the Key Update Permitted bit set to 0 until it discards old read keys to limit

How about explicitly stating "until it discards the 0-RTT key or the previous 1-RTT read key" here, to be clear that we can have the Key Update Permitted bit set to zero until the 0-RTT key is discarded when 0-RTT is used.

-- 
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/2237#pullrequestreview-190570401

---

From: Marten Seemann <[email-masked]>
Date: Tue, 08 Jan 2019 22:23:10 -0800

marten-seemann commented on this pull request.

> @@ -1101,75 +1101,153 @@ anticipation of receiving a ClientHello.
 # Key Update
 
 Once the 1-RTT keys are established and the short header is in use, it is
-possible to update the keys. The KEY_PHASE bit in the short header is used to
-indicate whether key updates have occurred. The KEY_PHASE bit is initially set
-to 0 and then inverted with each key update.
+possible to update the keys used to protect packets. The Key Update field in the
+short header are used to indicate when key updates are permitted and when they

s/are/is/

 This mechanism replaces the TLS KeyUpdate message.  Endpoints MUST NOT send a
 TLS KeyUpdate message.  Endpoints MUST treat the receipt of a TLS KeyUpdate
 message as a connection error of type 0x10a, equivalent to a fatal TLS alert of
 unexpected_message (see {{tls-errors}}).
 
-An endpoint MUST NOT initiate more than one key update at a time.  A new key
-cannot be used until the endpoint has received and successfully decrypted a
-packet with a matching KEY_PHASE.
+{{ex-key-update}} shows a key update process, with keys used identified with @M
+and @N.  Values in brackets [] indicate the value of Key Update bits.
+
+~~~
+   Initiating Peer                    Responding Peer
+
+@M [10] QUIC Packets
+. Update to @N
+@N [01] QUIC Packets
+                      -------->
+                                     QUIC Packets [10] @M

Why is this line here? The responding peer should update the keys immediately when receiving the first @N packet.

+Phase is used to indicate which packet protection keys are in use.  The Key

Maybe better: "which packet protection key was used to protect this packet."

-If the packet can be decrypted and authenticated using the updated key and IV,
-then the keys the endpoint uses for packet protection are also updated.  The
-next packet sent by the endpoint will then use the new keys.
+The endpoint uses the key and IV to protect all subsequent packets, clears the
+Key Update Permitted bit, and toggles the value of the low Key Phase bit.

This sentence reads better if you turn it around and insert an "updated": "The endpoint clears the Key Update Permitted bit, and toggles the value of the low Key Phase bit, and uses the updated key and IV to protect all subsequent packets."

-If the packet can be decrypted and authenticated using the updated key and IV,
-then the keys the endpoint uses for packet protection are also updated.  The
-next packet sent by the endpoint will then use the new keys.
+The endpoint uses the key and IV to protect all subsequent packets, clears the
+Key Update Permitted bit, and toggles the value of the low Key Phase bit.
+
+An endpoint MUST NOT initiate more than one key update at a time.  A new key
+cannot be used until the endpoint has successfully processed a packet with a

Better: A subsequent key update can only be performed once the endpoint...

-If the packet can be decrypted and authenticated using the updated key and IV,
-then the keys the endpoint uses for packet protection are also updated.  The
-next packet sent by the endpoint will then use the new keys.
+The endpoint uses the key and IV to protect all subsequent packets, clears the
+Key Update Permitted bit, and toggles the value of the low Key Phase bit.
+
+An endpoint MUST NOT initiate more than one key update at a time.  A new key
+cannot be used until the endpoint has successfully processed a packet with a
+matching Key Phase and the Key Update Permitted bit set.  Together, these
+indicate that the key update was received and acted on.
+
+Once an endpoint has received and successfully processed packets with the same
+Key Phase value, this indicates that the peer has also updated keys.  The
+endpoint can then set the Key Update Permitted bit to 1 on packets it

I'd prefer stronger language here. Ideally, a MUST (although we'd have to rearrange the text a bit to allow deferring this until the old keys are discarded).

-- 
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/2237#pullrequestreview-190578232

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 21:46:34 +0000 (UTC)

a2ef7224f97218680dc5b5cdf607834c3772370b  Changes from review comments
d62829d76d18807be535838d0b986c265d61f0a5  Fix sudden missing reference problem

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2237/files/8c72c8d98e9fc5d4538232089793bccf49a1beed..d62829d76d18807be535838d0b986c265d61f0a5

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 21:46:35 +0000 (UTC)

martinthomson commented on this pull request.

Yeah, I struggled with this too.  I want people to enable key updates, but the fact is that we can't really enforce it.  If we didn't want to allow peers to minimize the number of active keys, you could allow endpoints to punish them if they acknowledged a packet with a matching key phase in a packet that didn't include the Key Update Permitted bit.  I will try to work in a SHOULD somehow.

-- 
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/2237#discussion_r246557796

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 22:12:04 +0000 (UTC)

@martinthomson pushed 1 commit.

e9bb544b9b84cea909ab0c98cea28f6839cc6bf4  Disallow reneging on Key Update Permitted

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

---

From: hardie <[email-masked]>
Date: Wed, 09 Jan 2019 22:25:11 +0000 (UTC)

hardie commented on this pull request.

-without necessarily needing to receive the first packet that triggered the
-change.  An endpoint that notices a changed KEY_PHASE bit can update keys and
-decrypt the packet that contains the changed bit.
+The low bit of the Key Update field (0x04) is the Key Phase bit.  The Key Phase
+is used to indicate which packet protection keys are used to protect the packet.
+The Key Phase bit is initially set to 0 for the first set of 1-RTT packets.  The
+Key Phase is toggled to signal each key update.
+
+The Key Phase bit allows a recipient to detect a change in keying material
+without needing to receive the first packet that triggered the change.  An
+endpoint that notices a changed Key Phase updates keys and decrypts the packet
+that contains the changed value.
+
+The high bit of the Key Update field (0x08) is the Key Update Permitted bit.
+Endpoints set this value to 0 until they successfully process a packet with keys

As written, this seems to imply that Endpoints must set the value to 1 when they successfully process a packet.  I agree with Ian that there will be cases where this is set to 0 and never changed (e.g. when the expected flow size is short).  Perhaps this would work instead:

"The high bit of the Key Update field (0x08) is the Key Update Permitted bit.  Endpoints may set this value to 0 for any reason, forbidding Key Updates.  Endpoint MUST NOT set this value to 1 until they successfully process a packet the largest received packet number with keys from the same key phase as they are using.  They MAY set it to 1 to indicate key update is permitted at any time after that."

-- 
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/2237#pullrequestreview-190968792

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 22:49:40 +0000 (UTC)

acff3b6afb460f447ca4232f0218f9c83d4adc97  Make key update optional

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