Subject: Re: [quicwg/base-drafts] Desirable behavior when it takes time to derive the traffic keys for the next PN space (#3821)

From: ianswett <[email-masked]>
Date: Sat, 01 Aug 2020 20:12:59 -0700

I'm trying to fully digest everything above, so I apologize if I missed something important.

The idea that you wouldn't acknowledge packets that can be processed while waiting for keys for queued packets is clearly unrealistic for a variety of reasons, so we should definitely remove any text that assumes otherwise.

I'm unclear about why moving from handshake complete to confirmed is an improvement, given the example you cite seems to imply the server is retransmitting unnecessary data due to client delays, and for the server complete and confirmed happen at almost identical times.  Prior to CFIN or 1-RTT being acknowledged, the client knows the server has only one of the two keys.  PTO will retransmit the CFIN on the first PTO and the 1-RTT data on a subsequent PTO(assuming the first PTO doesn't send both).  I agree we expect HANDSHAKE_DONE to make this irrelevant in practice, but I can't reason about why this change is an improvement based on the problem description.  Can someone lay that out for me?

I believe we should make normative changes to transport along the lines of S2.  Specifically, I think we should say receivers SHOULD include the delay from the time an undecryptable packet is buffered to the time it's acknowledged in the ack_delay, as well as processing all buffered packets for a given encryption level before sending an ACK.  Ideally, a receiver would process buffered packets after the full datagram being processed as well, since that allows senders to fully mitigate this issue, but we could leave that to a description, rather than normative text if people prefer.  This provides more information to the sender, which it can ignore if it wants, and is much better than sending an Ack Delay of 0us in that case.  It's a bit odd that we say a receiver SHOULD set the ack delay field to 0 for Initial and Handshake packets today.

-- 
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/issues/3821#issuecomment-667619027

---

From: ianswett <[email-masked]>
Date: Sun, 02 Aug 2020 14:07:55 -0700

> > I'm unclear about why moving from handshake complete to confirmed is an improvement, given the example you cite seems to imply the server is retransmitting unnecessary data due to client delays, and for the server complete and confirmed happen at almost identical times.
> 
> I think it is because the client cannot make an assumption on how long it would take for the server to verify the client's TLS handshake messages being sent using Handshake packets. Important thing to note here is that it could not just be Finished. It might include client certificates. Theoretically, client's handshake transcript might be too large so that it requires multiple round-trips to be sent.

Specifically, is the concern is that the client will have it's Handshake data acknowledged and will spuriously PTO 1-RTT packets a few times over while waiting for the server to make 1-RTT receive keys available?  Is this a problem that's been observed?  It seems like it would in practice require client certs, since verifying only a CFIN should be very fast.

Making the change to wait for handshake confirmed to PTO 1-RTT data doesn't seem harmful given the existence of HANDSHAKE_DONE, but I'm also not sure it's going to matter much in practice.  It also adds an exception to the current rule that PTO is armed when the client isn't sure the server has completed address validation or there are bytes in flight.  If the Handshake data has been acknowledged, but the 1-RTT data hasn't, you'd have bytes in flight but not arm the PTO if I understand correctly.

Given there's still a SHOULD about sending packets from multiple PN spaces on PTO, including Handshake + 1RTT, I think this change will perform well, but I do have apprehension about seemingly small but subtle changes like this late in the process.

> 
> Regarding ack_delay, I might not sure if I fully follow. But I think I agree that it would become reasonable for an endpoint to send a Handshake Ack with a non-zero ack-delay, if it buffered the handshake packet that was undecryptable at the moment that packet was received, and later processed it. It is a natural outcome of us changing the definition of ack-delay to the sum of undecryptable buffering delay and acknowledgement delay.
> 
> As Initial packets would always be decryptable, I think "SHOULD set ack delay to 0" rule can remain for Initial packets.

Agreed, the potential for delay is only for the first Handshake and 1-RTT ACKs.

**My summary:** The issue is that ACKs for Handshake and 1-RTT packets could be delayed because the peer doesn't have the keys(either because it takes time to derive them, or it's missing crypto data to derive the next key).

This creates two potential issues(neither of which effects correctness, but could be suboptimal):
1. Packets may be acknowledged later than expected, causing the peer to spuriously PTO.
2. The first RTT sample in Handshake and 1-RTT packet spaces could have substantial delay in excess of the 'no ack delay' in Handshake or max_ack_delay for 1-RTT, which inflates SRTT and  RTTVar.

Changing when to arm PTO for 1-RTT data mitigates one case of the 1st issue(the server being slow to make 1-RTT receive keys available), but I don't believe it helps otherwise with the issue, such as delays in generating Handshake keys.  Are there any other design changes(besides complete->confirmed) being considered or suggested for the first issue that I'm missing?

If there's no loss, the second issue fixes itself without harm, which is nice.  And it's not as bad for ApplicationData, since that will have a non-0 max_ack_delay, which may be enough to absorb the endpoint delay generating keys or doing cert verification, though it won't typically be enough to adjust for a packet being lost and the data being PTO'd and then that making keys available.  I think Jana's suggestion ack_delay to including the time packets were buffered is sensible, and ignoring max_ack_delay for the first sample in a PN space seems relatively safe given at least one prior sample.  But what if there are no Initial ACKs received and ACK with the large ack_delay is the first ACK of the connection?  I guess you're stuck with one bad sample and hope the next is better? 

The one change we've found mitigates this issue entirely is coalescing packets from the next PN space(Handshake or 1RTT) and then processing the full coalesced packet before processing all buffered packets and sending an ACK.  That ensures there's one new packet being acknowledged and you get a non-delayed RTT sample.  It doesn't seem onerous to add a "SHOULD process all packets in the current datagram and all buffered packets before sending an ACK to avoid generating multiple inflated RTT samples" to me?

-- 
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/issues/3821#issuecomment-667724809

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 02 Aug 2020 14:40:15 -0700

@ianswett : I'm yet to process the messages above -- but this issue is the one that was covered in [this email thread on the list](https://mailarchive.ietf.org/arch/msg/quic/KBxSt7rv2PIs6RyumXwRJ0KUK6Q/).

-- 
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/issues/3821#issuecomment-667728044

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 02 Aug 2020 17:42:53 -0700

@kazuho 
> Specifically, is the concern is that the client will have it's Handshake data acknowledged and will spuriously PTO 1-RTT packets a few times over while waiting for the server to make 1-RTT receive…

I think the issue is about principles.

I tend to agree that it would be fine for an endpoint to arm PTO timer for 1-RTT packets, when it knows that the peer is going to be able to handle them. However, if we are going to talk about such cases, then I might point out there are other similar cases that are missing in the draft, that might be as important as the case you point out:

    * A server can arm 1-RTT PTO timer when an abbreviated TLS handshake (a.k.a. resumption) is being used. This is because a server to assume that the client would obtain 1-RTT read keys almost immediately.
    * A client can arm PTO timer for 0-RTT packets, assuming that the server would be resuming the connection.

My view is that all these are "optimizations" that an endpoint might attempt, based on the knowledge of the form of the TLS handshake being used. But they are just optimizations.

I think it would be fine to talk about such optimizations, but at the same time think that we should provide safe and principled rules as baseline, so that endpoints following the recommendations do not end up sending bunch of unnecessary packets.

> Given there's still a SHOULD about sending packets from multiple PN spaces on PTO, including Handshake + 1RTT, I think this change will perform well, but I do have apprehension about seemingly smal…

Yeah, I tend to think this as a bug fix. Existing PTO rules seem to assume that the client would spend time in processing the Handshake packets, but the server would not. But the reality is more nuanced, as pointed out by the client-certificate case as well as the examples shown above. And in case where the assumption is too aggressive (the client-cert case), we might see a number of unnecessary packets being sent.

All that said, I do think that we should try to minimize the impact of the changes that we introduce to the drafts. That's why I think that something along the lines of "SHOULD NOT arm PTO timer for ApplicationData packets until the handshake is confirmed, unless the sender can assume that the peer would be capable of processing ApplicationData packets when they arrive." Clients arming PTO for 1-RTT packets when client-certificate is not used would fall into this exception, and therefore the design change would not affect the existing QUIC stacks, assuming that they do not use client certificates. Stacks that do use client certificates have to have this fix applied.

-- 
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/issues/3821#issuecomment-667748606

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 02 Aug 2020 22:07:33 -0700

@ianswett 
>  But what if there are no Initial ACKs received and ACK with the large ack_delay is the first ACK of the connection? I guess you're stuck with one bad sample and hope the next is better?

That's correct. The resolution proposed by @janaiyengar does not mitigate that issue. By including queuing delay of unencryptable packets into ack_delay, the proposed resolution improves the quality of RTT samples. But as you say, it does not mitigate the case where that would be the first sample.

> It doesn't seem onerous to add a "SHOULD process all packets in the current datagram and all buffered packets before sending an ACK to avoid generating multiple inflated RTT samples" to me?

I'm not sure if doing that mitigates the problem. The proposed resolution recommends endpoints to send ACK for a handshake message before spending time to process it. As an example, a client would ack the Handshake packets before starting to verify the certificate chain. And after that, it would send a Handshake packet (that contains ClientFinished only, no ack will be included), and a 1-RTT packet acking the 0.5-RTT data sent by the server.

If the Handshake ack reached the server, then the server would have a good min_rtt estimate. If it did not, then, the next sample that the server would receive is the ack for the 0.5-RTT packet(s). As the servers do not arm the PTO timer for 0.5-RTT data, it is hard to have a protocol mechanism that prevents the first 1-RTT ACKs sent by the client from having a ack delay field that reflects the amount of time spent for verifying the certificate chain.

-- 
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/issues/3821#issuecomment-667805307

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 08:38:07 -0700

I created a separate issue(#3980) and PR(#3981) for the inflated RTT problem, since the solution seems fairly disjoint.

-- 
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/issues/3821#issuecomment-668091614

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 08:44:26 -0700

> I'm not sure if doing that mitigates the problem. The proposed resolution recommends endpoints to send ACK for a handshake message before spending time to process it. As an example, a client would…

I agree that if you have both sets of keys, the transport should send an ACK and not wait for all async crypto processing to complete.  This is similar to receiving a GET request at a proxy.  The proxy acknowledges the GET immediately even if it has to go to a backend to respond.  But that doesn't prevent greatly inflated RTT samples when the keys aren't immediately available.

-- 
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/issues/3821#issuecomment-668095002

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 09:27:41 -0700

> @ianswett : I'm yet to process the messages above -- but this issue was covered in [this email thread on the list](https://mailarchive.ietf.org/arch/msg/quic/KBxSt7rv2PIs6RyumXwRJ0KUK6Q/).

It's also above, but the inflight PR only attempts to address part of the problem, not the inflated RTT issue that I mentioned halfway through the issue.

Also, I'm finding it confusing to track what problems are trying to be solved and what ones are already largely solved, so I was hoping restating my understanding would allow you to correct me if I'm misunderstanding something.

-- 
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/issues/3821#issuecomment-668116841

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 Aug 2020 15:03:00 -0700

I think we should flag this as design and run the process.  It's hard to pin down exactly what changes here substantively, but it's worth getting the eyes.

-- 
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/issues/3821#issuecomment-672302637