Subject: [quicwg/base-drafts] Merge crypto timeout into PTO (#2655)

From: ianswett <[email-masked]>
Date: Fri, 26 Apr 2019 20:27:58 -0700

Makes the crypto timeout(including deadlock prevention) a special case of PTO.

Fixes #2650
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Merge crypto timeout into PTO

-- File Changes --

    M draft-ietf-quic-recovery.md (169)

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Fri, 26 Apr 2019 20:35:24 -0700

@ianswett pushed 1 commit.

5074beaf37aa1111784f6f28ac25806ea30ec1c3  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/2655/files/f36dced5e540fccf60a8a3c3e4d7e263a1bf133a..5074beaf37aa1111784f6f28ac25806ea30ec1c3

---

From: MikkelFJ <[email-masked]>
Date: Fri, 26 Apr 2019 22:50:24 -0700

mikkelfj commented on this pull request.

Overall, it's great to reduce the number of overlapping timers, but see comments.

> +The initial PTO SHOULD be set to twice the initial RTT or 1 second if no
+initial RTT is available as recommended in {{?RFC6298}}.
+
+Until the server has validated the client's address on the path, the amount of
+data it can send is limited, as specified in Section 8.1 of {{QUIC-TRANSPORT}}.
+If not all unacknowledged CRYPTO data can be sent, then all unacknowledged
+CRYPTO data sent in Initial packets should be retransmitted.  If no data can be
+sent, then no alarm should be armed until data has been received from the
+client.
+
+Because the server could be blocked until more packets are received, the client
+MUST ensure that the crypto retransmission timer is set if there is
+unacknowledged crypto data or if the client does not yet have 1-RTT keys.
+If the crypto retransmission timer expires before the client has 1-RTT keys,
+it is possible that the client may not have any crypto data to retransmit.
+However, the client MUST send a new packet, containing only PING or PADDING

```suggestion
However, the client MUST send a new packet, containing only PADDING
```
PING is not valid nor necessary here, no?

> +the count of bytes in flight.
+
+Endpoints stop sending and receiving Initial packets once they start exchanging
+Handshake packets (see Section 17.2.2.1 of {{QUIC-TRANSPORT}}). At this point,
+recovery state for all in-flight Initial packets is discarded.
+
+When 0-RTT is rejected, recovery state for all in-flight 0-RTT packets is
+discarded.
+
+If a server accepts 0-RTT, but does not buffer 0-RTT packets that arrive
+before Initial packets, early 0-RTT packets will be declared lost, but that
+is expected to be infrequent.
+
+It is expected that keys are discarded after packets encrypted with them would
+be acknowledged or declared lost.  Initial secrets however might be destroyed
+sooner, as soon as handshake keys are available (see Section 4.10 of

Discarding keys based on being declared lost seems odd to me. Either you abandon the handshake, or you discard keys when you are ready to advance to the next PN space, however that is defined. That readiness might involve overlapped PN spaces, waiting for older packets to ACK or to declare lost, but loss alone cannot be the criteria.

> @@ -689,6 +597,69 @@ is received that newly acknowledges packets, loss detection proceeds as
 dictated by packet and time threshold mechanisms, see {{ack-loss-detection}}.

+## PTO for Crypto Packets
+
+The initial PTO SHOULD be set to twice the initial RTT or 1 second if no
+initial RTT is available as recommended in {{?RFC6298}}.
+
+Until the server has validated the client's address on the path, the amount of
+data it can send is limited, as specified in Section 8.1 of {{QUIC-TRANSPORT}}.
+If not all unacknowledged CRYPTO data can be sent, then all unacknowledged
+CRYPTO data sent in Initial packets should be retransmitted.  If no data can be
+sent, then no alarm should be armed until data has been received from the
+client.
+

If no alarm is armed, the connection is at the mercy of the client. I supposed connection timeout is not armed yet, and also the wrong metric, but the server needs to be able to time out the connection attempt somehow.

On a related note, text below on Retry/VNEG says they do not ack an Initial, but I suppose they could be used to time out connection state server side?

-- 
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/2655#pullrequestreview-231410100

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 28 Apr 2019 00:04:47 -0700

janaiyengar approved this pull request.

Gah! I hate it when these diffs are so broken. I've gone over it, and I assume that this basically just moved text (and removed some unnecessary text), right? I want to do an editorial pass, but it might be best to do that once this is in. Go ahead and merge after you've fixed the merge 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/2655#pullrequestreview-231452091

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 28 Apr 2019 00:06:03 -0700

janaiyengar commented on this pull request.

Fair point. Yeah, I think only PADDING is fine.

-- 
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/2655#discussion_r279180509

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 28 Apr 2019 00:07:41 -0700

+

This is pre-existing text, not new to this PR. The point however here isn't that you discard keys based on declaring packets lost, but that it is generally expected to be the case that packets are already declared lost (or acked) by the time keys are discarded.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2655#discussion_r279180560

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 28 Apr 2019 00:14:46 -0700

Yes, I meant to come to this -- thanks for catching it. This is not specific to this PR, so I've filed #2656 for it.

-- 
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/2655#discussion_r279180878