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

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 17:37:44 -0800

@martinthomson pushed 3 commits.

a2a860f8651bb1283a721763bd0a4c9b3c580b44  Merge branch 'master' into simultaneous-update
e378a885781b04b50bbae624d9a8cd601162353b  KEYS_READY frame
1b63f75ca6cbe17e30397b77f55317cd7eb25278  Merge branch 'master' into simultaneous-update

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 17:55:38 -0800

@martinthomson pushed 1 commit.

fcc1d9e7d1a2239734a9f5722c9522723218fb66  Add to example flows

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

---

From: Nick Banks <[email-masked]>
Date: Tue, 12 Feb 2019 18:11:26 -0800

nibanks approved this pull request.

LGTM. I fully support this PR. Just some small nits for the most part.

-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.
+Once the 1-RTT keys are established and confirmed through the use of the
+KEYS_READY frame, it is possible to update the keys used to protect packets.

To me, the name KEYS_READY seems to indicate that the keys are about to be used, not that they are in use now. I think I'd prefer something like KEYS_ACTIVE or KEY_IN_USE. Just a personal preference though.

> @@ -1259,14 +1259,14 @@ Client                                                  Server
 Initial[0]: CRYPTO[CH] ->
 
                                  Initial[0]: CRYPTO[SH] ACK[0]
-                       Handshake[0]: CRYPTO[EE, CERT, CV, FIN]
+           Handshake[0]: KEYS_READY, CRYPTO[EE, CERT, CV, FIN]
                                  <- 1-RTT[0]: STREAM[1, "..."]
 
 Initial[1]: ACK[0]

Is this Initial necessary? Assuming it is coalesced with the Handshake that contains KEYS_READY, wouldn't that cause the Initial keys to be thrown away, implicitly ACK'ing outstanding Initial packets?

> @@ -5012,6 +5020,44 @@ Reason Phrase:
   This SHOULD be a UTF-8 encoded string {{!RFC3629}}.

+## KEYS_READY Frame {#frame-keys-ready}
+
+An endpoint sends a KEYS_READY frame (type=0x1e) to signal that it has installed
+keys for reading and writing packets.  Receipt of this frame in a packet

```suggestion
keys for both reading and writing packets.  Receipt of this frame in a packet
```
To be a bit more clear, it seems you are only supposed to send this when you are both keys are ready for use, not one or the other, correct?

-                           1-RTT[1]: STREAM[55, "..."], ACK[0]
+               1-RTT[1]: KEYS_READY, STREAM[55, "..."], ACK[0]
                                        <- Handshake[1]: ACK[0]

Similarly with this Handshake packet. Is it necessary, assuming it was coalesced with the 1-RTT packet? On a side note, if we don't get rid of it, I think it might be better to put the Handshake packet before the 1-RTT packet in the list, since that's the order they'd be coalesced (ditto for the 0-RTT example below).

-- 
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-202998585

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 18:56:44 -0800

martinthomson commented on this pull request.

Thanks Nick.

I don't care about the name, and KEYS_ACTIVE is technically more correct.

 
 Initial[1]: ACK[0]

You are right.  The client might send this because it processes the Initial before it processes the KEYS_ACTIVE frame, but it doesn't have to send it.

Yeah, I'll remove it on the basis that it's a nice optimization.

Yes, both.  And the clarification is good.

-- 
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-203009587

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 12 Feb 2019 19:02:17 -0800

kazuho commented on this pull request.

+## KEYS_READY Frame {#frame-keys-ready}
+
+An endpoint sends a KEYS_READY frame (type=0x1e) to signal that it has installed
+keys for reading and writing packets.  Receipt of this frame in a packet
+indicates that all earlier keys can be safely discarded.

At least for dropping the Initial and Handshake keys, I had assumed that we would be sending the frame after all the transmissions have been acknowledged rather than when the new keys become available.

The concern of _not_ waiting for ACKs is that packets would be deemed lost, having negative effect on the congestion control. For example, the proposed design suggests the server to drop Handshake keys when it receives ClientFinished, without sending an ACK for the packet that carried ClientFinished.

Are we fine with that?

The other issue regarding the text is that it does not state that the signal is unilateral. My understanding is that an endpoint is expected to drop the keys when _both_ of the following conditions are met: 1) the endpoint has received the _done_ frame, 2) the endpoint is fine with dropping the keys. The text seems to not capture the second condition.

-- 
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-203011306

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 19:05:12 -0800

We've had some discussions about losing packets during the handshake due to reordering, and I think that we just have to make some allowances in the congestion controller/loss recovery for that possibility.  For packets that we abandon like this, we lose the positive signal (i.e., congestion window increase), but we don't necessarily create a negative signal, because abandonment means that you don't declare them lost.

I'll tweak the text about the dropping of keys requiring both sending and receiving of the frame.

-- 
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_r256232628

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Feb 2019 03:08:40 +0000 (UTC)

Note the receiving only is a hangup from before I added the bit about delaying transmission of the frame in order to limit the number of active keys.  If you don't have that, receipt is sufficient because receiving the frame always implies that you need to send a matching frame.

-- 
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_r256233126

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 19:12:43 -0800

940bac4f4e98893a6da6a671a12a49374fd3c681  Rename frame; optimize some more; clarity; tweak

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

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 12 Feb 2019 19:22:24 -0800

> I think that we just have to make some allowances in the congestion controller/loss recovery for that possibility. For packets that we abandon like this, we lose the positive signal (i.e., congestion window increase), but we don't necessarily create a negative signal, because abandonment means that you don't declare them lost.

I think you are correct in pointing out that there's no negative signal even though we are losing a positive signal. As implied in my previous comment, my preference goes to delaying dropping keys until all the transmissions for that epoch is complete because that allows us to decouple loss-recovery / congestion control and epochs (FWIW, we will not bee seeing ACKs for ServerHello and ClientFinished). But I would not argue strongly, because there are ways to compensate for the missing signal.

> I'll tweak the text about the dropping of keys requiring both sending and receiving of the frame.

:+1:

-- 
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_r256235145

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 12 Feb 2019 19:36:10 -0800

> @@ -3605,12 +3601,12 @@ subsequent to the first do not need to fit within a single UDP datagram.
 
 #### Abandoning Initial Packets {#discard-initial}
 
-Endpoints cease both sending and processing Initial packets when it receives a
-Handshake packet containing a KEYS_READY frame.  Though packets might still be
-in flight or awaiting acknowledgment, no further Initial packets need to be
-exchanged beyond this point.  Initial packet protection keys are discarded (see
-Section 4.10 of {{QUIC-TLS}}) along with any loss recovery and congestion
-control state (see Sections 5.3.1.2 and 6.9 of {{QUIC-RECOVERY}}).
+Endpoints cease both sending and processing Initial packets when it both sends
+and receives a Handshake packet containing a KEYS_ACTIVE frame.  Though packets

I think we should say "sends and receives a Handshake packet _or a 1-RTT packet_ containing a KEYS_ACTIVE frame", assuming that we wouldn't be waiting for ACKs when dropping the keys that are to carry the ACKs.

This is because a Handshake packet carrying just the KEYS_ACTIVE frame can be dropped, while all other packets reach the peers.

-- 
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-203016928

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 21:27:32 -0800

3fca7a585564c7dd939170d51c714d8b34d7f3bf  1-RTT KEYS_ACTIVE can be used to discard Initial

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 21:31:47 -0800

8fd93b85d830c8be4cf3f183a804345d4fff0180  I should not edit on my phone

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 Feb 2019 21:41:39 -0800

I don't want to delay discarding Initial keys, because we're would lose the DoS fix. Handshake keys are probably ok, but it is easier to draw the line between handshake and key update.

I don't expect that loss and reordering will be that significant, nor will the loss of positive signals. Any missing packets should be small (just an ACK probably), so the reduced increase to congestion allowance would be negligible.

We could add text to Section 6.2.2 of the recovery draft to this effect.

-- 
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_r256254315

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 12 Feb 2019 22:00:56 -0800

janaiyengar commented on this pull request.

We should consider losses of handshake packets as not triggering congestion window reductions. This is perhaps a bit aggressive, but I would argue that we shouldn't have a lot of these packets anyways, and there are reasons beyond congestion that could cause these packets to be marked as lost.

I think we should go with the cleanest mechanism to drop keys -- this has been tricky to get right -- and we'll fix recovery around that. Once the other mechanisms are set, we can figure out how to best do recovery around them.

-- 
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_r256257416

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 12 Feb 2019 22:09:53 -0800

IMO the most architecturally sane way would be to transmit all necessary ACKs for every epoch, and I am not sure how much I buy the argument that dropping Initial keys earlier is a win, considering that there would be 1-RTT injection window anyways. A middlebox that observes an Initial can send CONNECTION_CLOSE to both endpoints (as they do for TCP), regardless of when we drop the Initial key. However I also agree that losing the ability to see the ACK is not a big deal. So maybe it's nothing more than a matter of taste.

-- 
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_r256258855

---

From: Christian Huitema <[email-masked]>
Date: Tue, 12 Feb 2019 23:28:40 -0800

huitema commented on this pull request.

Some nits. We need a bit more clarity on the sending of KEY ACTIVE during the handshake, but otherwise this is good.

-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 Key Phase bit is used to indicate which packet protection keys are used to

I am confused. Line 866 mentions a Key Update bit, and here we see a Key Phase bit. Is that the same bit, or do we now have 2 bits?

> @@ -863,7 +863,7 @@ This protection applies to the least-significant bits of the first byte, plus
 the Packet Number field.  The four least-significant bits of the first byte are
 protected for packets with long headers; the five least significant bits of the
 first byte are protected for packets with short headers.  For both header forms,
-this covers the reserved bits and the Packet Number Length field; the Key Phase
+this covers the reserved bits and the Packet Number Length field; the Key Update
 bit is also protected for packets with a short header.

See comments in Key Update section. I would vote for keeping the Key Phase name.

> +corresponding key and IV are created from that secret as defined in
+{{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 a
+KEYS_ACTIVE frame in a packet from the current key phase.  A subsequent key
+update can only be performed after the endpoint has successfully processed a
+KEYS_ACTIVE frame from a packet with a matching key phase.  This ensures that
+keys are available to both peers before another can be initiated.
+
+Once an endpoint has successfully processed a packet with the same key phase, it
+can send a KEYS_ACTIVE frame.  Endpoints MAY defer sending a KEYS_ACTIVE frame
+after a key update (see {{key-update-old-keys}}).
+

I assume that "processed a packet" means "received, successfully decrypted, and processed the content of the frames." But I have to guess. Encrypting  and sending is a form of processing too. The term first occurs in section 8.1, and it is a bit puzzling there too. I would suggest adding a formal definition in "1.2. Terms and Definitions ".

> +## Responding to a Key Update
+
+An endpoint that sends a KEYS_ACTIVE frame can accept further key updates.  A
+key update can happen even without seeing a KEYS_ACTIVE frame from the peer.  If
+a packet is received with a key phase that differs from the value the endpoint
+used to protect the packet containing its last KEYS_ACTIVE frame, the endpoint
+creates a new packet protection secret for reading and the corresponding key and
+IV.  An endpoint uses the same key derivation process as its peer uses to
+generate keys for receiving.
+
+If the packet protection is successfully removed using the updated key and IV,
+then the keys the endpoint initiates a key update in response, as described in
+{{key-update-initiate}}.  An endpoint that responds to a key update MUST send a
+KEYS_ACTIVE frame to indicate that it is both sending and receiving with updated
+keys, though it MAY defer sending the frame (see {{key-update-old-keys}}).
+

Isn't that a restatement of the lines 1167-1169? Why do we have the same text twice, with slightly different phrasing, "processed" vs. "packet protection is successfully removed" ?

> +corresponding key and IV are created from that secret as defined in
+{{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 a
+KEYS_ACTIVE frame in a packet from the current key phase.  A subsequent key
+update can only be performed after the endpoint has successfully processed a
+KEYS_ACTIVE frame from a packet with a matching key phase.  This ensures that
+keys are available to both peers before another can be initiated.
+
+Once an endpoint has successfully processed a packet with the same key phase, it
+can send a KEYS_ACTIVE frame.  Endpoints MAY defer sending a KEYS_ACTIVE frame
+after a key update (see {{key-update-old-keys}}).
+

A lot of that text is going to be specified again in the following paragraphs. Do we need a paraphrase here?

> +place of discarded keys when key updates are not permitted.
+
+
+## Using Old Keys {#key-update-old-keys}
+
+During a key update, packets protected with older keys might arrive if they were
+delayed by the network.  If those old keys are available, then they can be used
+to remove packet protection.
+
+After a key update, an endpoint MAY delay sending the KEYS_ACTIVE frame by up to
+three times the Probe Timeout (PTO, see {{QUIC-RECOVERY}}) to minimize the
+number of active keys it maintains.  During this time, an endpoint can use old
+keys to process delayed packets rather than enabling a new key update.  This
+only applies to key updates that use the Key Phase bit; endpoints MUST NOT defer
+sending of KEYS_ACTIVE during and immediately after the handshake.
+

"key updates that use the Key Phase bit". That's a weird way to put it. Maybe a reference to the ladder diagram in the transport spec?

> +corresponding key and IV are created from that secret as defined in
+{{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 a
+KEYS_ACTIVE frame in a packet from the current key phase.  A subsequent key
+update can only be performed after the endpoint has successfully processed a
+KEYS_ACTIVE frame from a packet with a matching key phase.  This ensures that
+keys are available to both peers before another can be initiated.
+
+Once an endpoint has successfully processed a packet with the same key phase, it
+can send a KEYS_ACTIVE frame.  Endpoints MAY defer sending a KEYS_ACTIVE frame
+after a key update (see {{key-update-old-keys}}).
+

Initial to Handshake, and Handshake to 1-RTT. Are these special cases of Key Update?

> @@ -1259,15 +1259,13 @@ Client                                                  Server
 
                                  Initial[0]: CRYPTO[SH] ACK[0]
-                       Handshake[0]: CRYPTO[EE, CERT, CV, FIN]
+           Handshake[0]: KEYS_ACTIVE, CRYPTO[EE, CERT, CV, FIN]
                                  <- 1-RTT[0]: STREAM[1, "..."]

Sending the KEY ACTIVE there seems wrong. The logic "I have processed a packet at this key level". That has not happened yet, will only happen at the sending of `<- Handshake[1]: ACK[0]`. That's the same reason why you correctly do not place a Key Active frame in the packet `<- 1-RTT[0]: STREAM[1, "..."]`

> @@ -1283,15 +1281,13 @@ Initial[0]: CRYPTO[CH]
 0-RTT[0]: STREAM[0, "..."] ->
 
                                  Initial[0]: CRYPTO[SH] ACK[0]
-                        Handshake[0] CRYPTO[EE, CERT, CV, FIN]
+           Handshake[0]: KEYS_ACTIVE, CRYPTO[EE, CERT, CV, FIN]
                           <- 1-RTT[0]: STREAM[1, "..."] ACK[0]

Same issue. You cannot send Key Active there because the server has not yet received an Handshake packet from the peer.

> @@ -3520,7 +3520,7 @@ carries ACKs in either direction.
 
 ~~~
 +-+-+-+-+-+-+-+-+
-|1|1| 0 |R R|P P|
+|1|1| 0 | R | P |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Unrelated change. Makes the PR harder to read.

>  #### Abandoning Initial Packets {#discard-initial}
 
-A client stops both sending and processing Initial packets when it sends its
-first Handshake packet.  A server stops sending and processing Initial packets
-when it receives its first Handshake packet.  Though packets might still be in
-flight or awaiting acknowledgment, no further Initial packets need to be
-exchanged beyond this point.  Initial packet protection keys are discarded (see
-Section 4.10 of {{QUIC-TLS}}) along with any loss recovery and congestion
-control state (see Sections 5.3.1.2 and 6.9 of {{QUIC-RECOVERY}}).
+Endpoints cease both sending and processing Initial packets when it both sends
+and receives a Handshake or 1-RTT packet containing a KEYS_ACTIVE frame.  Though
+packets might still be in flight or awaiting acknowledgment, no further Initial
+packets need to be exchanged beyond this point.  Initial packet protection keys
+are discarded (see Section 4.10 of {{QUIC-TLS}}) along with any loss recovery
+and congestion control state (see Sections 5.3.1.2 and 6.9 of {{QUIC-RECOVERY}}).

The server side specification feels wrong. The server does not know that the peer has received the server initial until it receives the KEY ACTIVE (Handshake) from the client. Yes, there is an "implicit" dependency that the Handshake cannot be processed without receiving first the Initial packet. There will be cases where Initial is lost, and both Initial and Handshake have to be retransmitted.

> @@ -3843,7 +3847,7 @@ short packet header.
  0                   1                   2                   3
  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 +-+-+-+-+-+-+-+-+
-|0|1|S|R|R|K|P P|
+|0|1|S| R |K| P |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Unrelated change.

-- 
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-203046954

---

From: Nick Banks <[email-masked]>
Date: Wed, 13 Feb 2019 15:18:09 +0000 (UTC)

nibanks commented on this pull request.

I personally prefer the immediate discarding of keys and implicit ACKs.

-- 
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_r256441433

---

From: Nick Banks <[email-masked]>
Date: Wed, 13 Feb 2019 07:25:56 -0800

My understanding is that the logic to send a KEYS_ACTIVE frame is that you have both read and write keys available for use. Not that you have used them. For that reason, the server doesn't send the KEYS_ACTIVE for 1-RTT, but it does send it for Handshake.

-- 
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_r256445322

---

From: Nick Banks <[email-masked]>
Date: Wed, 13 Feb 2019 07:28:57 -0800

The KEYS_ACTIVE is sent when both read and write keys are ready, but you don't throw away the previous keys until you have sent and received a KEYS_ACTIVE. I'm unsure of what the problem is here. If you receive a KEYS_ACTIVE then the peer must have received all the necessary packets of yours sent with the previous key.

-- 
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_r256446751

---

From: Mike Bishop <[email-masked]>
Date: Wed, 13 Feb 2019 17:32:42 +0000 (UTC)

MikeBishop commented on this pull request.

@kazuho, but implementations could quite reasonably be skeptical of packets with CONNECTION_CLOSE and wait to see if others arrive.  Injecting a fictitious Initial packet with a broken/different handshake is a much more effective attack.  It's true that 1 RTT is probably plenty of time for an attacker, but I'd prefer to minimize the window anyway.

-- 
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_r256506294

---

From: Mike Bishop <[email-masked]>
Date: Wed, 13 Feb 2019 10:02:53 -0800

>From a less-TLS-steeped reader; take the comments for what they're worth.  :-)

> +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

"can" feels very weak.  Even if they MAY delay sending, aren't they required to eventually send it?

+KEYS_ACTIVE frame to indicate that it is both sending and receiving with updated
+keys, though it MAY defer sending the frame (see {{key-update-old-keys}}).
+
+An endpoint does not always need to send packets when it detects that its peer
+has updated keys.  The next packet that it sends use the new keys and include
+the KEYS_ACTIVE frame.  If an endpoint detects a second update before it has
+sent any packets with updated keys or a KEYS_ACTIVE frame, it indicates that its
+peer has updated keys twice without awaiting a reciprocal update.  An endpoint
+MUST treat consecutive key updates as a connection error of type
+KEY_UPDATE_ERROR.
+
+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.

If you're going to mention this last, it's probably worth expanding on this a bit.  What behavior might generate such a side-channel?  How are dummy keys used to emulate normal behavior in those cases?

+
+

Maybe "after the handshake is complete," assuming that's now a defined term?

+
+During a key update, packets protected with older keys might arrive if they were
+delayed by the network.  If those old keys are available, then they can be used
+to remove packet protection.
+
+After a key update, an endpoint MAY delay sending the KEYS_ACTIVE frame by up to
+three times the Probe Timeout (PTO, see {{QUIC-RECOVERY}}) to minimize the
+number of active keys it maintains.  During this time, an endpoint can use old
+keys to process delayed packets rather than enabling a new key update.  This
+only applies to key updates that use the Key Phase bit; endpoints MUST NOT defer
+sending of KEYS_ACTIVE during and immediately after the handshake.
+
+Even if old keys are available, those keys MUST NOT be used to protect packets
+with packets that have higher packet numbers than packets that were protected
+with newer keys.  An endpoint that successfully removes protection with old keys

Would it make sense to mandate dropping write keys immediately, but permit retaining read keys?

-- 
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-203355408

---

From: Christian Huitema <[email-masked]>
Date: Wed, 13 Feb 2019 19:05:09 +0000 (UTC)

I left a comment on the mailing list. The more I look at it, the more I think that we should not invent a special mechanism, but simply reuse PATH_CHALLENGE/PATH_RESPONSE. What we are doing is effectively a continuity test for a set of addresses and a key. The challenge/response approach would bring clarity to the "acknowledge or not" issue.

Of course, there is downside to reusing path continuity for testing key continuity. If we want to decouple, then we could create KEY_CHALLENGE/KEY_RESPONSE frames that parallel for keys what the existing challenges to for paths.

-- 
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-463325186

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Feb 2019 17:45:31 -0800

efeaf0dadda0471d9446f436458a680aaf6375d9  reflow2
5fb15c8a413ee38ccf1a8ee86f327fbcac1f2c86  Fix some words
b3d8ff1a8c9a9bd280c4ed7ea8501af7ebfb79d5  Varied feedback

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

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 13 Feb 2019 20:15:36 -0800

This is some heavy lifting, thank you for doing it! A few comments.

```suggestion
The Key Phase bit indicates which packet protection keys are being used to
```

-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 Key Phase bit 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 bit is toggled to signal each key update.

```suggestion
of 1-RTT packets and toggled for each subsequent key.
```

+protect the packet.  The Key Phase bit is initially set to 0 for the first set

```suggestion
protect the packet.  The Key Phase bit is set to 0 for the first set
```

> -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.
+
+## 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
+corresponding key and IV are created from that secret as defined in
+{{protection-keys}}.  The header protection key is not updated.
+
+The endpoint toggles the value of the Key Phase bit, and uses the updated key

```suggestion
The endpoint toggles the value of the Key Phase bit and uses the updated key
```

>                        <--------
+... Update to @N
+@N [1] QUIC Packets

This is a bit confusing. The KEYS_ACTIVE sent above are for @M, right? If so, shouldn't the endpoint send a KEYS_ACTIVE for @N as soon as it updates to the new keys (on line 1133)?

+An endpoint MUST NOT initiate a key update prior to having received and
+successfully processed a KEYS_ACTIVE frame contained in a packet 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 do not need to
+  wait for a KEYS_ACTIVE frame, they are driven solely by changes in the TLS
+  handshake.  The KEYS_ACTIVE frame is used to allow Initial and Handshake keys
+  to be discarded when they are no longer needed and - in the case of the first
+  1-RTT key phase - to enable the first key update.
+
+Once an endpoint has successfully processed a packet with the same key phase, it

"with the same key phase" as what?

> +  handshake.  The KEYS_ACTIVE frame is used to allow Initial and Handshake keys
+  to be discarded when they are no longer needed and - in the case of the first
+  1-RTT key phase - to enable the first key update.
+
+Once an endpoint has successfully processed a packet with the same key phase, it
+MUST send a KEYS_ACTIVE frame, though endpoints MAY defer sending the frame (see
+{{key-update-old-keys}}).
+
+
+## Responding to a Key Update
+
+An endpoint that sends a KEYS_ACTIVE frame can accept further key updates.  A
+key update can happen even without seeing a KEYS_ACTIVE frame from the peer.  If
+a packet is received with a key phase that differs from the value the endpoint
+used to protect the packet containing its last KEYS_ACTIVE frame, the endpoint
+creates a new packet protection secret for reading and the corresponding key and

Is this always the case? If I was using 0, and now start using 1 and send KEYS_ACTIVE with 1, I will continue to receive packets with key phase 0. That doesn't mean I need to create a new secret, since I haven't discarded the previous secret yet, right?

+place of discarded keys when key updates are not permitted; using dummy keys

If this were to happen, wouldn't the connection be closed with a KEY_UPDATE_ERROR? Is there much value in the timing signal at this point?

+to remove packet protection.
+
+An endpoint always sends packets that are protected with the newest keys.  Keys
+used for protecting packets that an endpoint sends can be discarded immediately
+after newer keys are available.
+
+After a key update, an endpoint MAY delay sending the KEYS_ACTIVE frame by up to
+three times the Probe Timeout (PTO, see {{QUIC-RECOVERY}}) to minimize the
+number of active keys it maintains.  During this time, an endpoint can use old
+keys to process delayed packets rather than enabling a new key update.  This
+only applies to key updates; endpoints MUST NOT defer sending of KEYS_ACTIVE
+during and immediately after the handshake.
+
+Even if old keys are available, those keys MUST NOT be used to protect packets
+with packets that have higher packet numbers than packets that were protected

```suggestion
that have higher packet numbers than packets that were protected
```

+

Why? It seems appropriate to close the connection here.

+keys to process delayed packets rather than enabling a new key update.  This
+only applies to key updates; endpoints MUST NOT defer sending of KEYS_ACTIVE
+during and immediately after the handshake.
+
+Even if old keys are available, those keys MUST NOT be used to protect packets
+with packets that have higher packet numbers than packets that were protected
+with newer keys.  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.
+
+An endpoint SHOULD retain old read keys for a period of no more than three times
+the current PTO.  After this period, old read keys and their corresponding
+secrets SHOULD be discarded.
+
+An endpoint that receives a packet protected with old keys that includes an

Suggestion: "An endpoint that receives an acknowledgement for a packet protected with new keys in a packet protected with older keys MAY treat that as a ..."

> @@ -2934,6 +2931,9 @@ containing that information is acknowledged.
 * PING and PADDING frames contain no information, so lost PING or PADDING frames
   do not require repair.
 
+* KEYS_ACTIVE frames are sent until acknowledged or until newer keys are used
+  for sending packets.

I'm not sure how this works. If KEYS_ACTIVE is not yet acked, we don't know if the peer has seen this KEYS_UPDATE YET.  We therefore cannot assume that the peer can accept another key update, and so this endpoint cannot use newer keys until an ack is received for the previously sent KEYS_UPDATE.

If this is correct, you should point this out earlier in "Responding to a Key Update" above as well.

> @@ -3414,7 +3414,7 @@ packet type.  While type-specific semantics for this version are described in
 the following sections, several long-header packets in this version of QUIC
 contain these additional fields:
 
-Reserved Bits (R):
+Reserved Bit (R):

It's still two bits.

> @@ -5012,6 +5017,45 @@ Reason Phrase:

+## KEYS_ACTIVE Frame {#frame-keys-active}
+
+An endpoint sends a KEYS_ACTIVE frame (type=0x1e) to signal that it has
+installed keys for both sending and receiving packets.  Once this frame has been
+both sent and received, older keys can be safely discarded.
+
+The KEYS_ACTIVE frame contains no additional fields.
+
+The packet that carries a KEYS_ACTIVE frame determines which keys are active and
+usable.  The keys with the same key phase as those used in the packet that
+carries the KEYS_ACTIVE frame are active.
+
+An endpoint MUST send a KEYS_ACTIVE packet in the first packet it sends using

```suggestion
An endpoint MUST send a KEYS_ACTIVE frame in the first packet it sends using
```

> @@ -5012,6 +5017,45 @@ Reason Phrase:

+## KEYS_ACTIVE Frame {#frame-keys-active}
+
+An endpoint sends a KEYS_ACTIVE frame (type=0x1e) to signal that it has
+installed keys for both sending and receiving packets.  Once this frame has been
+both sent and received, older keys can be safely discarded.
+
+The KEYS_ACTIVE frame contains no additional fields.
+
+The packet that carries a KEYS_ACTIVE frame determines which keys are active and
+usable.  The keys with the same key phase as those used in the packet that
+carries the KEYS_ACTIVE frame are active.
+
+An endpoint MUST send a KEYS_ACTIVE packet in the first packet it sends using
+keys, but only if it is also able to receive packets that are protected using

```suggestion
those keys, but only if it is also able to receive packets that are protected using
```

> +both sent and received, older keys can be safely discarded.
+
+The KEYS_ACTIVE frame contains no additional fields.
+
+The packet that carries a KEYS_ACTIVE frame determines which keys are active and
+usable.  The keys with the same key phase as those used in the packet that
+carries the KEYS_ACTIVE frame are active.
+
+An endpoint MUST send a KEYS_ACTIVE packet in the first packet it sends using
+keys, but only if it is also able to receive packets that are protected using
+the corresponding keys.
+
+KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.

As I wrote above, I don't understand how this can happen.

+The packet that carries a KEYS_ACTIVE frame determines which keys are active and
+usable.  The keys with the same key phase as those used in the packet that
+carries the KEYS_ACTIVE frame are active.
+
+An endpoint MUST send a KEYS_ACTIVE packet in the first packet it sends using
+keys, but only if it is also able to receive packets that are protected using
+the corresponding keys.
+
+KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.
+
+Endpoints MUST NOT send KEYS_ACTIVE frames in Initial or 0-RTT packets.
+

I would move these last 3 paras to a new subsection called "Key Updates During the Handshake" in the "Key Update" section above.

> +keys, but only if it is also able to receive packets that are protected using
+the corresponding keys.
+
+KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.
+
+Endpoints MUST NOT send KEYS_ACTIVE frames in Initial or 0-RTT packets.
+
+A KEYS_ACTIVE frame used during the handshake can be used to indicate the
+availability of Handshake keys by including it in a Handshake packet.  An
+endpoint sends this frame in its first Handshake packet.  Once received, an
+endpoint can discard Initial keys.
+
+A KEYS_ACTIVE frame used after the completion of the handshake in 1-RTT packets

```suggestion
A KEYS_ACTIVE frame used in 1-RTT packets immediately after the completion of the handshake
```
(not any time after handshake but immediately after)

> +the corresponding keys.
+
+KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.
+
+Endpoints MUST NOT send KEYS_ACTIVE frames in Initial or 0-RTT packets.
+
+A KEYS_ACTIVE frame used during the handshake can be used to indicate the
+availability of Handshake keys by including it in a Handshake packet.  An
+endpoint sends this frame in its first Handshake packet.  Once received, an
+endpoint can discard Initial keys.
+
+A KEYS_ACTIVE frame used after the completion of the handshake in 1-RTT packets
+indicates that Handshake keys are no longer needed.  A client sends this frame

```suggestion
indicates that Handshake keys are no longer needed.  A client SHOULD send this frame
```

+KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.
+
+Endpoints MUST NOT send KEYS_ACTIVE frames in Initial or 0-RTT packets.
+
+A KEYS_ACTIVE frame used during the handshake can be used to indicate the
+availability of Handshake keys by including it in a Handshake packet.  An
+endpoint sends this frame in its first Handshake packet.  Once received, an
+endpoint can discard Initial keys.
+
+A KEYS_ACTIVE frame used after the completion of the handshake in 1-RTT packets
+indicates that Handshake keys are no longer needed.  A client sends this frame
+in its first 1-RTT packet, and a server sends this frame in the first packet it

```suggestion
in its first 1-RTT packet, and a server SHOULD send this frame in the first packet it
```

> +KEYS_ACTIVE frames are retransmitted when declared lost, however implementations
+need to take care not to retransmit lost KEYS_ACTIVE frames if they initiate a
+subsequent key update.  This can happen if an acknowledgment for a packet
+containing a KEYS_ACTIVE frame is lost.
+
+Endpoints MUST NOT send KEYS_ACTIVE frames in Initial or 0-RTT packets.
+
+A KEYS_ACTIVE frame used during the handshake can be used to indicate the
+availability of Handshake keys by including it in a Handshake packet.  An
+endpoint sends this frame in its first Handshake packet.  Once received, an
+endpoint can discard Initial keys.
+
+A KEYS_ACTIVE frame used after the completion of the handshake in 1-RTT packets
+indicates that Handshake keys are no longer needed.  A client sends this frame
+in its first 1-RTT packet, and a server sends this frame in the first packet it
+sends after completing the handshake.  A server might send 1-RTT keys prior to

A server never sends 1-RTT keys. I'm not sure what this sentence is trying to say.

-- 
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-203548487

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Feb 2019 21:40:18 -0800

I've added a note about the handshake to clear that up.  Logically, they are the same, but there is nothing to be gained by delaying the signal, or by requiring that the signal be used before a key change can be made.

+

No difference intended.  I've tried to even this out.

Text added.

+
+

I've tried very hard to separate key update as a concept from the key changes that happen during the handshake.  So I'll remove the 'that use the Key Phase bit' part.

+

I don't want to mandate dropping keys so directly, but I will mention that it is possible.  Note that there are cases during the handshake where write keys for different packet number spaces need to be kept.  So this would be limited to key update.

Yes, this is the logic.  I had to tweak the description of the frame to account for that though...

+... Update to @N
+@N [1] QUIC Packets

During a key update, the initiator doesn't update until it receives a packet in the new keys.  If we sent the frame in the first packet we'd get the simultaneous update problem.

That makes me a little more receptive to the idea that @kazuho suggests on the mailing list, but not much more.  Having special rules for Handshake isn't a huge burden.

p.s., I'm sure that github users `@M` and `@N` get pinged all the time.

No.  Because packets can be injected.

It is correct.

Interesting observation:  If your KEYS_ACTIVE got through and you don't see the ACK before your peer updates again, then you can just retransmit, but it's a different frame at that point.

That suggests that maybe you don't need to worry about stopping retransmission for KEYS_ACTIVE.  The sender can always update, and the sender of a packet with new keys containing KEYS_ACTIVE is always receptive to a key update.

Technically however, KEYS_ACTIVE sent in those circumstances has different semantics, so the text is correct as is.

+

Say I initiate an update.  I receive a KEYS_ACTIVE in an updated packet.

At this point, I am expected to start sending KEYS_ACTIVE.  But I can also initiate another key update.

If I initiate a key update, which I can do without sending any more packets, then I MUST NOT send a KEYS_ACTIVE in that packet, because that leaves me open to a double update from my peer.

I'm going with MUST.

Oops, I meant 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/2237#discussion_r256660837

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Feb 2019 21:40:46 -0800

@martinthomson pushed 2 commits.

b56d3d968526ed70767ce4a96aa367c4e65496b3  You need to update read keys too
5b075a83cfc33611d51fbb417c83e3d84cf047dd  Review feedback

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

---

From: Mike Bishop <[email-masked]>
Date: Thu, 14 Feb 2019 17:00:04 +0000 (UTC)

+... Update to @N
+@N [1] QUIC Packets

Poor Matt Mullenweg....  😁

-- 
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_r256922880

---

From: Martin Thomson <[email-masked]>
Date: Thu, 14 Feb 2019 16:46:38 -0800

331d3a7b9751c86be54655d3aa6c5ea6794c063c  Merge branch 'master' into simultaneous-update

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