Subject: [quicwg/base-drafts] Rewrite key update section (#3050)

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Sep 2019 20:15:06 -0700

This makes some significant editorial changes to the key update section,
hopefully making the various activities clearer and more explicit.

In the process, I am also trying to address #2792, which is the timing
sidechannel created by having the generation of updated keys inline with
packet processing.  In doing so, I&#39;m suggesting that endpoints create
the next keys at some time after the key update happens.  Right now, I&#39;m
thinking 1-2 PTOs is probably close enough to workable.  I&#39;ve limited
this at 3PTO.  This is, however, just a (firm) suggestion at this stage.

For endpoints that only want to keep 2 sets of keys, this is probably
the right time frame, especially if we keep the current advice for 3PTO
before a subsequent update.

The effect of this is that attempts to update at certain times could
cause all packets after the update to be discarded.  That would only
happen if implementations consistently ignored advice on update
frequency, so I think that&#39;s tolerable, but I&#39;d like input on this.

(This also attempts to take up the advice from the other, older PRs on
this subject.)

Closes #2792, #2791, #2237.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Rewrite key update section

-- File Changes --

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

-- Patch Links --

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

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 18 Sep 2019 22:51:06 -0700

kazuho commented on this pull request.

Thank you very much for writing this.

This is a big change in text and I assume it has been tough to write down. It's also hard to read and make comments at once, so below are my tentative comments.

> +{{protection-keys}}.  The header protection key is not updated.
+
+
+The endpoint toggles the value of the Key Phase bit and uses the updated key and
+IV to protect all subsequent packets.
+
+An endpoint MUST NOT initiate a key update prior to having received an
+acknowledgment for a packet that it sent protected with keys from the current
+key phase.  This ensures that keys are available to both peers before another
+can be initiated.
+
+Note:
+
+: Changes in keys from Initial to Handshake and from Handshake to 1-RTT don't
+  use this key update process. Key changes during the handshake are driven
+  solely by changes in the TLS handshake state.

It seems a bit odd that we do not talk about 0-RTT packet here, even though Initial and Handshake are mentioned.

Maybe something like: _Keys of packets other than the 1-RTT packets are never updated; their keys are derived solely from the TLS handshake state._

> +
+
+## Responding to a Key Update
+
+Once an endpoint has acknowledged a packet in the current key phase, a peer is
+permitted to initiate a key update.  If a packet is received with a key phase
+that differs from the value the endpoint used to protect the last packet it
+sent, the endpoint uses the next packet protection keys for reading and the
+corresponding key and IV; see {{receive-key-generation}} for considerations
+about generating these keys.
+
+An endpoint uses the same key derivation process as its peer uses to generate
+keys for receiving.
+
+If a packet is successfully processed using the next key and IV, then the peer
+has initiated a key update.  The endpoint MUST updates its send keys to the

s/updates/update/

> +Once an endpoint has acknowledged a packet in the current key phase, a peer is
+permitted to initiate a key update.  If a packet is received with a key phase
+that differs from the value the endpoint used to protect the last packet it
+sent, the endpoint uses the next packet protection keys for reading and the
+corresponding key and IV; see {{receive-key-generation}} for considerations
+about generating these keys.
+
+An endpoint uses the same key derivation process as its peer uses to generate
+keys for receiving.
+
+If a packet is successfully processed using the next key and IV, then the peer
+has initiated a key update.  The endpoint MUST updates its send keys to the
+corresponding key phase in response, as described in {{key-update-initiate}}.
+By acknowledging the packet that triggered the key update in a packet protected
+with the updated keys, the endpoint will ultimately signal that the key update
+is complete.

Would it be possible to clarify the ordering of the operation that the endpoint needs to follow?

IIUC, there is a MUST requirement to update the send keys, before sending an ACK for a packet that triggered that update.

We have the following paragraph starting from line 1280 stating that the peer can close the connection if the endpoint does not follow this ordering requirement.
>  An endpoint that receives an acknowledgement that is carried in a packet
> 	protected with old keys where any acknowledged packet was protected with newer
> 	keys MAY treat that as a connection error of type KEY_UPDATE_ERROR.  This
> 	indicates that a peer has received and acknowledged a packet that initiates a
> 	key update, but has not updated keys in response.

> +apparent key updates are easy to forge and - while the process of key update
+does not require significant effort - triggering this process could be used by
+an attacker for DoS.
+
+For this reason, endpoints MUST be able to retain two sets of packet protection
+keys for receiving packets: the current and the next.  Retaining the previous
+keys in addition to these might improve performance, but this is not essential.
+
+The time taken to generate new keys could reveal through timing side channels
+that a key update has occurred, or where an attacker injects packets, be used to
+reveal the value of the Key Phase on injected packets.  After receiving a key
+update, an endpoint SHOULD generate and save the next set of packet protection
+keys.  After new keys are available, receipt of packets will not create timing
+signals about the value of the Key Phase.
+
+This depends on not doing this generating during packet processing and it can

Maybe s/this generating/this key generation/?

> +keys for receiving packets: the current and the next.  Retaining the previous
+keys in addition to these might improve performance, but this is not essential.
+
+The time taken to generate new keys could reveal through timing side channels
+that a key update has occurred, or where an attacker injects packets, be used to
+reveal the value of the Key Phase on injected packets.  After receiving a key
+update, an endpoint SHOULD generate and save the next set of packet protection
+keys.  After new keys are available, receipt of packets will not create timing
+signals about the value of the Key Phase.
+
+This depends on not doing this generating during packet processing and it can
+require that endpoints maintain three sets of packet protection keys for
+receiving: for the previous key phase, for the current key phase, and for the
+next key phase.  Endpoints MAY choose to defer generation of the next packet
+protection keys until they discard old keys so that only two sets of receive
+keys need to be retained at any point in time.

IIUC, this sentence is suggesting an alternative approach to the previous sentence. Assuming that is the case, I think it might be slightly better to start this sentence with "Endpoints MAY _instead_".

> +update, an endpoint SHOULD generate and save the next set of packet protection
+keys.  After new keys are available, receipt of packets will not create timing
+signals about the value of the Key Phase.
+
+This depends on not doing this generating during packet processing and it can
+require that endpoints maintain three sets of packet protection keys for
+receiving: for the previous key phase, for the current key phase, and for the
+next key phase.  Endpoints MAY choose to defer generation of the next packet
+protection keys until they discard old keys so that only two sets of receive
+keys need to be retained at any point in time.
+
+
+## Sending with Updated Keys {#old-keys-send}
+
+An endpoint always sends packets that are protected with the newest keys.  Keys
+used for adding packet protection can be discarded immediately after switching

Maybe "adding" is superfluous?

-- 
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/3050#pullrequestreview-290325868

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Sep 2019 23:11:45 -0700

@martinthomson pushed 1 commit.

678c0486f91012173e827b1d708429b153bc3037  Kazuho is a great help here

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3050/files/85db1f71811872bc01e4ac3692e545620258b82e..678c0486f91012173e827b1d708429b153bc3037

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Sep 2019 23:12:43 -0700

Thanks @kazuho.  Do you have any opinion on the advice about how many keys to hold, when to flip from keeping previous vs. next (especially if you are doing just two keys) and the technical changes?

-- 
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/3050#issuecomment-532982818

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 18 Sep 2019 23:49:39 -0700

@martinthomson It seemed to me that the advice on how to use 2 keys seemed correct and appropriate. It's a bit unfortunate that the advice is scattered, but I assume that's because we have to talk about the features rather than talking about implementation strategies. I also like the introduction of KEY_UPDATE_ERROR.

-- 
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/3050#issuecomment-532992898

---

From: David Schinazi <[email-masked]>
Date: Thu, 19 Sep 2019 01:42:44 -0700

DavidSchinazi commented on this pull request.

Thanks for rewriting this, I think in general the new text is clearer than the previous one. Comments inline.

> @@ -1168,89 +1168,232 @@ anticipation of receiving a ClientHello.
 
 # Key Update
 
-Once the handshake is confirmed, 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.
+Once the 1-RTT keys are established and confirmed, it is possible to update the

The `handshake is confirmed` has a meaning [formally defined in section 4.1.2](https://quicwg.org/base-drafts/draft-ietf-quic-tls.html#handshake-confirmed). Do we have a formal definition for confirming 1-RTT keys?

> -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 period of no more than three times the
-PTO.  After this period, old keys and their corresponding secrets SHOULD be
-discarded.  Retaining keys allow endpoints to process packets that were sent
-with old keys and delayed in the network.  Packets with higher packet numbers
-always use the updated keys and MUST NOT be decrypted with old keys.
-
-This ensures that once the handshake is complete, packets with the same
-KEY_PHASE will have the same packet protection keys, unless there are multiple
-key updates in a short time frame succession and significant packet reordering.
+{{ex-key-update}} shows a key update process, with keys used identified with @M
+cannot be used until the endpoint has received and successfully decrypted a and
+@N.  The value of Key Phase bit is indicated in brackets \[]..

typo: value of *the* Key Phase bit

> -If an endpoint detects a second update before it has sent any packets with
-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 period of no more than three times the
-PTO.  After this period, old keys and their corresponding secrets SHOULD be
-discarded.  Retaining keys allow endpoints to process packets that were sent
-with old keys and delayed in the network.  Packets with higher packet numbers
-always use the updated keys and MUST NOT be decrypted with old keys.
-
-This ensures that once the handshake is complete, packets with the same
-KEY_PHASE will have the same packet protection keys, unless there are multiple
-key updates in a short time frame succession and significant packet reordering.
+{{ex-key-update}} shows a key update process, with keys used identified with @M
+cannot be used until the endpoint has received and successfully decrypted a and

I'm failing to parse this sentence.

-In deciding when to update keys, endpoints MUST NOT exceed the limits for use of
-specific keys, as described in Section 5.5 of {{!TLS13}}.
+## Initiating a Key Update {#key-update-initiate}
+
+Endpoints maintain separate read and write secrets for packet protection.  An
+endpoint initiates a key update by updating its packet protection write secret
+and using that to protect new packets.  The endpoint creates a new write secret
+from the existing write secret as performed in Section 7.2 of {{!TLS13}}.  This
+uses the KDF function provided by TLS with a label of "quic ku".  The

Is this PR changing the label for key updates?

 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 the first key update until the handshake is
-confirmed ({{handshake-confirmed}}). An endpoint MUST NOT initiate a subsequent
-key update until it has received an acknowledgment for a packet sent at the
-current KEY_PHASE.  This can be implemented by tracking the lowest packet

The implementation guidance sentence (`This can be implemented by...`) has been removed, did you not consider it useful?

+## Timing of Receive Key Generation {#receive-key-generation}
+
+Endpoints responding to an apparent key update MUST NOT generate a timing
+side-channel signal that might indicate that the Key Phase bit was invalid (see
+{{header-protect-analysis}}).  Endpoints can use dummy packet protection keys in
+place of discarded keys when key updates are not permitted; using dummy keys
+will generate no variation in the timing signal produced by attempting to remove
+packet protection, but all packets with an invalid Key Phase bit will be
+rejected.
+
+The process of creating new packet protection keys for receiving packets could
+reveal that a key update has occurred.  An endpoint MAY perform this process as
+part of packet processing, but this creates a timing signal that can be used by
+an attacker to learn when key updates happen and thus the value of the Key Phase
+bit in certain packets.  Endpoints SHOULD instead defer the creation of the next

If the endpoint is deferring creation of the next set of keys, does that mean that it's not allowed to send ACKs for up to 3 * PTO? That can really impact performance.

I'm wondering if alternatively we should recommend creating the next set of keys as soon as the old keys have been discarded, it would remove the timing side-channel without causing delays.

-- 
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/3050#pullrequestreview-290396663

---

From: Martin Thomson <[email-masked]>
Date: Thu, 19 Sep 2019 23:19:53 -0700

martinthomson commented on this pull request.

It might be.  What is it now?  (That's a serious question, because I can't tell.)  I should open an issue.

-- 
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/3050#discussion_r326484804

---

From: Martin Thomson <[email-masked]>
Date: Thu, 19 Sep 2019 23:32:26 -0700

Thanks David.

 # Key Update

I think that this is fine.  The reason we needed that definition is that we were hanging a lot from it.  This should however be referencing that definition, because it's handshake confirmation that I intended to use (but failed to).

This is READ keys, you update write keys immediately.

See #3054.

-- 
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/3050#pullrequestreview-290974470

---

From: Martin Thomson <[email-masked]>
Date: Thu, 19 Sep 2019 23:34:43 -0700

1b4d136a4ac9593b8211d378789ed2d08e16ea6e  Attempt to improve readability in a few places.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3050/files/678c0486f91012173e827b1d708429b153bc3037..1b4d136a4ac9593b8211d378789ed2d08e16ea6e

---

From: David Schinazi <[email-masked]>
Date: Fri, 20 Sep 2019 02:30:11 -0700

Ah, that makes a lot more sense. Thanks.

-- 
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/3050#discussion_r326546464

---

From: David Schinazi <[email-masked]>
Date: Fri, 20 Sep 2019 02:30:50 -0700

I don't know what it is either, we haven't implemented key updates yet.

-- 
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/3050#discussion_r326546824

---

From: ianswett <[email-masked]>
Date: Fri, 27 Sep 2019 10:26:05 -0700

ianswett commented on this pull request.

This generally looks like an improvement, some suggestions.

> +: Keys of packets other than the 1-RTT packets are never updated; their keys are
+  derived solely from the TLS handshake state.
+
+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.
+
+
+## Responding to a Key Update
+
+Once an endpoint has acknowledged a packet in the current key phase, a peer is

I misread this a few times at first, so how about "A peer is permitted to initiate a key update after receiving an acknowledgement of a packet in the current key phase."  Or you could remove this sentence entirely given there's a MUST NOT above.

+
+If a packet is successfully processed using the next key and IV, then the peer
+has initiated a key update.  The endpoint MUST update its send keys to the
+corresponding key phase in response, as described in {{key-update-initiate}}.
+Sending keys MUST be updated before sending an acknowledgement for the packet
+that was received with updated keys.  By acknowledging the packet that triggered
+the key update in a packet protected with the updated keys, the endpoint signals
+that the key update is complete.
+
+An endpoint can defer sending the packet or acknowledgement according to its
+normal packet sending behaviour; it is not necessary to immediately generate a
+packet in response to a key update.  The next packet sent by the endpoint will
+use the updated keys.  The next packet that contains an acknowledgement will
+cause the key update to be completed.  If an endpoint detects a second update
+before it has sent any packets with updated keys or an acknowledgement for the

It has to send an acknowledgement, so I think this can be more restrictive?
```suggestion
before it has sent any packets with updated keys containing an acknowledgement for the
```

> +keys twice without awaiting confirmation.  An endpoint MAY treat consecutive key
+updates as a connection error of type KEY_UPDATE_ERROR.
+
+An endpoint that receives an acknowledgement that is carried in a packet
+protected with old keys where any acknowledged packet was protected with newer
+keys MAY treat that as a connection error of type KEY_UPDATE_ERROR.  This
+indicates that a peer has received and acknowledged a packet that initiates a
+key update, but has not updated keys in response.
+
+
+## Timing of Receive Key Generation {#receive-key-generation}
+
+Endpoints responding to an apparent key update MUST NOT generate a timing
+side-channel signal that might indicate that the Key Phase bit was invalid (see
+{{header-protect-analysis}}).  Endpoints can use dummy packet protection keys in
+place of discarded keys when key updates are not permitted; using dummy keys

There's no way to indicate key updates are not permitted at all, correct?  Maybe say "not yet permitted" to be slightly clearer that it's too early/fast, and key updates are not an optional feature.

> +has initiated a key update.  The endpoint MUST update its send keys to the
+corresponding key phase in response, as described in {{key-update-initiate}}.
+Sending keys MUST be updated before sending an acknowledgement for the packet
+that was received with updated keys.  By acknowledging the packet that triggered
+the key update in a packet protected with the updated keys, the endpoint signals
+that the key update is complete.
+
+An endpoint can defer sending the packet or acknowledgement according to its
+normal packet sending behaviour; it is not necessary to immediately generate a
+packet in response to a key update.  The next packet sent by the endpoint will
+use the updated keys.  The next packet that contains an acknowledgement will
+cause the key update to be completed.  If an endpoint detects a second update
+before it has sent any packets with updated keys or an acknowledgement for the
+packet that initiated the key update, it indicates that its peer has updated
+keys twice without awaiting confirmation.  An endpoint MAY treat consecutive key
+updates as a connection error of type KEY_UPDATE_ERROR.

I'll note that this means every time there is a key update, there is a small window of time where a peer can reset the connection unless the receiver successfully decrypts the incoming packet before sending the connection close.  Maybe what you're intending is that the receiver detects a second update and is processed, but I wasn't sure.

> +keys.
+
+Packets with higher packet numbers MUST be protected with either the same or
+newer packet protection keys than packets with lower packet numbers.  An
+endpoint that successfully removes protection with old keys when newer keys were
+used for packets with lower packet numbers MUST treat this as a connection error
+of type KEY_UPDATE_ERROR.
+
+
+## Receiving with Different Keys {#old-keys-recv}
+
+For receiving packets during a key update, packets protected with older keys
+might arrive if they were delayed by the network.  Retaining old packet
+protection keys allows these packets to be successfully processed.
+
+As packet protected with keys from the next key phase use the same Key Phase

```suggestion
As packets protected with keys from the next key phase use the same Key Phase
```

> +channel that might reveal which keys were used to remove packet protection.
+
+Alternatively, endpoints can retain only two sets of packet protection keys,
+swapping previous for next after enough time has passed to allow for reordering
+in the network.  In this case, the Key Phase bit alone can be used to select
+keys.
+
+An endpoint SHOULD allow a period between one and two times the Probe Timeout
+(PTO; see {{QUIC-RECOVERY}}) after a key update before it creates the next set
+of packet protection keys.  These MAY replace the previous keys at that time.
+With the caveat that PTO is a subjective measure - that is, a peer could have a
+different view of the RTT - this time is expected to be long enough that any
+reordered packets would be declared lost by a peer even if they were
+acknowledged and short enough to allow for subsequent key updates.
+
+Endpoints SHOULD allow for the possibility that a peer is not able to handle a

I'm confused by the one to two times PTO vs three times PTO here.  I prefer this second paragraph that states you need to way 3*PTO after key update is complete, minus the fact I'm unclear what "a peer is not able to handle a key update during this period" means?

-- 
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/3050#pullrequestreview-294411517