Subject: [quicwg/base-drafts] Allow endpoints to generate traffic keys asynchronously (#3874)

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 01:38:55 -0700

As pointed out in #3821, the drafts assume that the traffic keys are generated synchronously. But that&#39;s not what we see in the wild. In fact, clients often need to spend time while verifying the certificate, and some if not most of the clients do receive and send QUIC packets while they do so.

This PR acknowledges the existence of such implementation, recommending them to buffer the undecryptable packets while they calculate the next key, to prevent loss events that would kill the slow start. It also allows endpoints to postpone arming the PTO timer when it knows that the peer is going to send a handshake message.

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

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

-- Commit Summary --

  * allow endpoints to generate traffic keys asynchronously, write down the expectations in relation

-- File Changes --

    M draft-ietf-quic-recovery.md (22)
    M draft-ietf-quic-tls.md (8)

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 01:57:32 -0700

@martinthomson commented on this pull request.

I have reservations about this.

Firstly, this should not be about what an endpoint "thinks" about the state of a peer.

Second, #3873 summarizes my view of the root problem, which is not about key generation taking time, but extra validation.

-- 
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/3874#pullrequestreview-444545591

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 03:53:32 -0700

@kazuho pushed 2 commits.

4e4957d93f14541d9f548ebc215e836cc67afaa6  clarify the precise condition
872e2f4c105e22d309228ee06aae1b593d5144da  QUIC does not generate keys, TLS provides them

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/2d30e24c46ddac06a6b57ff1b703ec085c507b0a..872e2f4c105e22d309228ee06aae1b593d5144da

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 03:54:04 -0700

@martinthomson Thank you for your comments. I think the two new commits address your concerns. PTAL.

-- 
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/3874#issuecomment-655444132

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 05:13:33 -0700

@kazuho pushed 1 commit.

43230756b3511dbff3fe740bbf63e73e27a284f6  Rearrange the text, based on @marten-seeman's point that the intent of the existing text has already been to forbid PTO timer to be armed for 1-RTT packets until the handshake completes, and that the pseudo-code is written as such

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/872e2f4c105e22d309228ee06aae1b593d5144da..43230756b3511dbff3fe740bbf63e73e27a284f6

---

From: Marten Seemann <[email-masked]>
Date: Wed, 08 Jul 2020 05:47:33 -0700

@marten-seemann approved this pull request.

If I understand correctly, the changes to -recovery are purely editorial now.
Only the changes to -tls could be considered a design change.

> @@ -518,14 +518,17 @@ or when Initial or Handshake keys are discarded. This ensures the PTO is always
 set based on the latest RTT information and for the last sent packet in the
 correct packet number space.
 
+When setting the PTO timer, the ApplicationData packet number space (Section
+4.1.1 of {{QUIC-TLS}}) MUST be ignored until the handshake completes. Not arming
+the PTO for ApplicationData prevents a client from retransmitting a 0-RTT packet
+on a PTO expiration before confirming that the server is able to decrypt 0-RTT

The client knows that the server is able to decrypt 0-RTT packets once it receives the ServerHello. The reason why it shouldn't retransmit these packets is that the client doesn't have the 1-RTT keys it would need to unprotected the ACKs sent for those packets.

>  When ack-eliciting packets in multiple packet number spaces are in flight,
-the timer MUST be set for the packet number space with the earliest timeout,
-with one exception. The ApplicationData packet number space (Section 4.1.1
-of {{QUIC-TLS}}) MUST be ignored until the handshake completes. Not arming
-the PTO for ApplicationData prevents a client from retransmitting a 0-RTT
-packet on a PTO expiration before confirming that the server is able to
-decrypt 0-RTT packets, and prevents a server from sending a 1-RTT packet on
-a PTO expiration before it has the keys to process an acknowledgement.
+the timer MUST be set for the packet number space with the earliest timeout.

This now seems to contradict the previous paragraph.

-- 
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/3874#pullrequestreview-444726192

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 06:17:43 -0700

@kazuho commented on this pull request.

Yeah technically we can say that "the timer MUST be set for the (non-ignored) packet number space with the earliest timeout." But does that make a difference?

-- 
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/3874#discussion_r451536201

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 06:44:22 -0700

@kazuho pushed 1 commit.

b5e0c16f7fc83e7bcc03122170c0a8a3b446d8e8  both endpoints refrain from sending probes before obtaining the read keys

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/43230756b3511dbff3fe740bbf63e73e27a284f6..b5e0c16f7fc83e7bcc03122170c0a8a3b446d8e8

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 06:47:42 -0700

I think you are correct in pointing out the ACK case, though
* the argument to not send 0-RTT probes before learning if the peer is accepting 0-RTT still holds; that's the only signal available to the client when the server rejects 0-RTT
* technically the acceptance (or rejection) of 0-RTT data is transmitting in EncryptedExtensions, not ServerHello.

In b5e0c16 I've changed the text to point out that both endpoints are covered by the ACK case.

-- 
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/3874#discussion_r451557053

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 18:00:15 -0700

The recovery piece is much clearer, but I think we need a different solution for the TLS stuff.

It probably pays to be precise in this case.  You could instead say that you set the Initial or Handshake PTO timer based on which space has the earliest timeout.

> @@ -501,9 +501,11 @@ handshake, new data is requested from TLS after providing received data.
 As keys for new encryption levels become available, TLS provides QUIC with those
 keys.  Separately, as keys at a given encryption level become available to TLS,
 TLS indicates to QUIC that reading or writing keys at that encryption level are
-available.  These events are not asynchronous; they always occur immediately
-after TLS is provided with new handshake bytes, or after TLS produces handshake
-bytes.
+available.  While waiting for TLS to provide these keys, an endpoint SHOULD
+buffer the received packets marked as protected by the keys yet to be provided,
+and process them once those keys become available.  If the keys are generated
+asynchronously, an endpoint MAY continue responding to the received packets that
+are processable while waiting for TLS to provide these keys.

I think that this fixates on the wrong aspect of the problem.  This text can instead just say "if keys are not yet available, and endpoint MAY continue to process those packets for which it has keys".  Or similar.

-- 
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/3874#pullrequestreview-445222013

---

From: Marten Seemann <[email-masked]>
Date: Wed, 08 Jul 2020 18:27:44 -0700

@marten-seemann commented on this pull request.

> @@ -518,14 +518,22 @@ or when Initial or Handshake keys are discarded. This ensures the PTO is always
 
+When setting the PTO timer, the ApplicationData packet number space (Section
+4.1.1 of {{QUIC-TLS}}) MUST be ignored until the handshake completes. Not arming
+the PTO for ApplicationData prevents unnecessary transmissions on a PTO
+expiration, such as:
+
+* an endpoint sending probe packets before obtaining the keys to process an
+  acknowledgement,
+
+* a client sending 0-RTT probe packets before confirming that the server is able

I believe we should remove this bullet point. See https://github.com/quicwg/base-drafts/pull/3874#discussion_r451515406 for the reasoning.

-- 
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/3874#pullrequestreview-445230560

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 18:48:29 -0700

+  to decrypt 0-RTT packets, or

Maybe this can be made more generic: 

A receiver might lack the keys necessary to process packets, or the sender might lack the keys necessary to process acknowledgments.  This occurs when the client sends 0-RTT packets as the server might not accept 0-RTT and the client will not have 1-RTT keys for processing acknowledgments until the handshake completes.  This occurs for 1-RTT packets as either endpoint might not have the ability to read until the handshake is complete.

Though this made me realize: we say "complete", but if you need to know that you have keys AND you peer has keys, you need to wait for "confirmed".

-- 
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/3874#pullrequestreview-445236854

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:33:43 -0700

@kazuho pushed 1 commit.

3e3421b809f9662af3a714b4d4c0705817361a7c  explicitly state from which PN spaces to choose

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/b5e0c16f7fc83e7bcc03122170c0a8a3b446d8e8..3e3421b809f9662af3a714b4d4c0705817361a7c

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:34:05 -0700

Thank you for the suggestion. Applied in 3e3421b.

-- 
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/3874#discussion_r451944382

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:36:51 -0700

@kazuho pushed 1 commit.

16038c12449105191790a8cb47f7922f6a5b7565  MT's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/3e3421b809f9662af3a714b4d4c0705817361a7c..16038c12449105191790a8cb47f7922f6a5b7565

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:37:12 -0700

Thanks, I agree that that's better. Applied in 16038c1.

-- 
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/3874#discussion_r451945094

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:44:28 -0700

@kazuho pushed 1 commit.

8af4de06d32b693097177daa0870f13c0d718414  MT's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/16038c12449105191790a8cb47f7922f6a5b7565..8af4de06d32b693097177daa0870f13c0d718414

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:45:09 -0700

Makes sense. Applied in 8af4de0.

-- 
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/3874#discussion_r451946756

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 20:47:07 -0700

Yeah my point in https://github.com/quicwg/base-drafts/pull/3874#discussion_r451557053 was that if the server rejects the 0-RTT, then that rejection acts as an alternative to ACK, and that therefore it might be worth mentioning.

But anyways I've applied MT's suggestion in daf4de0, I think it resolves your concern.

-- 
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/3874#discussion_r451947179

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 21:04:55 -0700

> @@ -518,14 +518,20 @@ or when Initial or Handshake keys are discarded. This ensures the PTO is always
 
+When setting the PTO timer, the ApplicationData packet number space (Section
+4.1.1 of {{QUIC-TLS}}) MUST be ignored until the handshake completes. PTO is not
+armed because issues other than packet loss might prevent an endpoint from
+receiving an acknowledgement. A receiver might lack the keys necessary to
+process packets, or the sender might lack the keys necessary to process
+acknowledgments. This occurs when the client sends 0-RTT packets as the server
+might not accept 0-RTT and the client will not have 1-RTT keys for processing
+acknowledgments until the handshake completes. This occurs for 1-RTT packets as
+either endpoint might not have the ability to read until the handshake is
+complete or confirmed.

```suggestion
complete.
```

Confirmation tells us that our peer has a completed handshake.  This is not a useful state here, because we are talking about a specific endpoint and their ability to read doesn't depend on confirmation.

I was questioning whether we should block probing more.  That is, until the handshake is confirmed.  That has performance implications, because you don't probe for longer, but it does mean that you never probe without being sure that the probe can be answered.

I suspect that there is a trade-off here.  Waiting for handshake complete is not perfect, but it is close enough that it is good enough.  Ideally we could run experiments to tell us when spurious losses due to keys not being available and set a policy based on that.  Because that is ultimately what this is: a policy that we use to protect the core recovery algorithm.

Maybe that suggests a different answer: we are going to pick a point at which we recommend that probing starts.  That will be imperfect and that's OK.  To that end, this MUST seems like a problem.

-- 
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/3874#pullrequestreview-445275426

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 21:19:41 -0700

@kazuho pushed 1 commit.

a87baafd19142a8042dc7acf8e82eb2e28012015  Update draft-ietf-quic-recovery.md

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

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 21:32:02 -0700

Thank you for the suggestion. I agree that the reference to handshake confirmation is unnecessary if not incorrect. Removed.

Regarding what should be written down in spec, I tend to agree that this should better be a recommendation rather than a requirement. We need to be clear to state when probes *have* to be sent in order to avoid deadlock, but anything above that can just be recommended.

That said, I would not mind leaving the normative text as they are now, so that we could call the changes to the recovery draft editorial. MUSTs in the recovery drafts are often recommendations, after all.

-- 
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/3874#discussion_r451957460

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 23:33:28 -0700

@martinthomson approved this pull request.

I think that this is good now.

And - as a bonus - it's now purely editorial.

-- 
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/3874#pullrequestreview-445323710

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 09 Jul 2020 00:55:16 -0700

@martinthomson Thank you for your help in moving this PR forward.

I'm a bit hesitant to call this PR editorial though, previously endpoints were expected to process TLS handshake messages synchronously, now we have a MAY that allows them to process them in an asynchronous manner (while waiting for TLS to provide new 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/3874#issuecomment-655969026

---

From: Marten Seemann <[email-masked]>
Date: Thu, 09 Jul 2020 04:40:35 -0700

-- 
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/3874#pullrequestreview-445535132

---

From: ianswett <[email-masked]>
Date: Thu, 09 Jul 2020 15:44:05 -0700

@ianswett commented on this pull request.

I'd like to better understand the recovery change you're making and it's motivation.

+the timer MUST be set to the earlier value of the Initial and Handshake packet
+number spaces.

I think this is a behavior change and I don't think it's always correct, since it causes the timer to never be armed for ApplicationData anytime anything in Handshake is in flight.  There are cases the Handshake keys have been dropped by the server and the Handshake data will never be acknowledged.

Can you clarify what the goal of changing this section is?  I feel it was clearer before, but I suspect there's something you're trying to improve.

-- 
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/3874#pullrequestreview-446015448

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 09 Jul 2020 17:30:56 -0700

Regarding if the new approach works, it does, because the server would make sure that all the handshake messages up to ServerFinished would be delivered to the client, and then the client would make sure that ClientFinished would be delivered to the server. At that moment, the server would be allowed to send 1-RTT probes.

Regarding if this is a design change rather than editorial, the answer depends on how we interpret the existing text. [Section 6.2.1](https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#section-6.2.1-6) currently states:
> When ack-eliciting packets in multiple packet number spaces are in flight, the timer MUST be set for the packet number space with the earliest timeout, with one exception. The ApplicationData packet number space (Section 4.1.1 of QUIC-TLS) MUST be ignored until the handshake completes.

There are two ways to interpret these two sentences, depending on if we consider the MUST in the second sentence to be a sub-condition of the first sentence, or consider the two sentences as listing two distinct requirements. @marten-seemann points out that the latter is what the pseudo-code does, and assuming that is the case, there is no design 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/3874#discussion_r452560791

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 09 Jul 2020 17:33:06 -0700

As pointed out in https://github.com/quicwg/base-drafts/pull/3874#discussion_r452560791, the existing text can be read in two ways, depending on how you interpret "with one exception." This PR rearranges two requirements to different paragraphs to be clear that there is no dependency between the two requirements.

-- 
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/3874#discussion_r452561324

---

From: Marten Seemann <[email-masked]>
Date: Thu, 09 Jul 2020 18:23:36 -0700

@ianswett When you drop keys, you're also supposed to drop the corresponding packets in that packet number space, so the Handshake packets in your example would not be considered outstanding.

That being said, I have to admit I don't understand why the existing text is ambiguous. @kazuho's change here (the one to the -recovery document), if I understand it correctly, is purely editorial.

-- 
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/3874#discussion_r452573469

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 13 Jul 2020 23:02:24 -0700

@janaiyengar commented on this pull request.

@kazuho: I've tried to clarify the text a bit more, please see if this fits. It's more general than the existing text (which is what I think you were going for), but it also gives specific examples, which helps understand the context.

+complete.

```suggestion
4.1.1 of {{QUIC-TLS}}) MUST be ignored until the handshake completes.
Not arming the PTO timer on ApplicationData prevents an endpoint from
retransmitting data in packets when the peer does not yet have the keys
necessary to process them or when  the endpoint does not yet have the
keys necessary to process their acknowledgements. For instance, this can
happen when a client retransmits data sent in 0-RTT packets to the server
before confirming that the server is able to decrypt 0-RTT packets. Or it
can happen when a server retransmits data sent in 1-RTT packets while the
client has not yet finished validating the server's certificate and therefore
can not read 1-RTT packets yet.
```

@ianswett This isn't behavior change. I've suggested a rephrase above that ought to make this clearer.

-- 
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/3874#pullrequestreview-447807266

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jul 2020 01:02:19 -0700

@kazuho pushed 1 commit.

b90653469d5aad86f728026f9d2c27feb435ec23  Jana's suggestions

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

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jul 2020 01:02:44 -0700

Thank you for the suggestion. Applied in b906534 with slight tweaks. PTAL.

-- 
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/3874#discussion_r454176672

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jul 2020 21:48:52 -0700

@kazuho pushed 1 commit.

2e36c1e926183e37f3dca7271a05348fe07b09e9  based on the established rationale, PTO should be disabled until the handshake is confirmed rather than until when completed

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874/files/b90653469d5aad86f728026f9d2c27feb435ec23..2e36c1e926183e37f3dca7271a05348fe07b09e9

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jul 2020 21:14:25 -0700

To make the change of status clear, the change to probing keeps probes for application data disabled until handshake confirmation rather than handshake completion.  This is based on the conclusions in [this comment](https://github.com/quicwg/base-drafts/issues/3821#issuecomment-658507628).  I think that PR is an incremental change in that direction, but more is needed to complete the set of changes proposed there.

My suggestion would be to get agreement on the overall strategy, as proposed.  Then we can take this PR and follow up with more comprehensive/systemic fixes.

-- 
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/3874#issuecomment-659147169