Subject: [quicwg/base-drafts] PTO probes are sent too frequently (#3546)

From: Tatsuhiro Tsujikawa <[email-masked]>
Date: Wed, 25 Mar 2020 18:46:05 -0700

It looks like under the current PTO algorithm PTO probes are sent too frequently under certain condition.
To create such a condition, suppose that client never send Handshake packets.
According to the -27 spec, client sends ack-eliciting anti-deadlock packet on pto_count=1.
Then it gets ACK from server for it, which resets pto_count to 0.  After next pto timeout, it sends anti-deadlock packet again.  This process repeats indefinitely until either endpoint gives up handshake.  The problem is that depending RTT, PTO probes are sent too frequently.  With RTT=6ms, it sends PTO probe once per ~33ms.  And both sides do this.  ACK+PADDING packets are also sent without delay in this period if remote endpoint does PTO.   I'm not sure TCP does the similar things, but it sounds like PTO probe is too aggressive.
If both endpoints are implemented properly, this probably does not happen, but in practice, we don't know the remote endpoint is broken in this case, and have no means to check that out.

-- 
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/3546

---

From: ianswett <[email-masked]>
Date: Thu, 26 Mar 2020 07:46:33 -0700

If the server keeps quickly ACKing packets but not sending handshake data, then yes, the client will keep sending full-sized packets, believing the server is blocked by the anti-amplification limit.

At first I thought this couldn't happen in a real-world scenario, but possibly if one had async TLS handshaking so the ServerHello could not be sent immediately upon receipt of the ClientHello and the server and client were very close, a packet could be exchanged as quickly as 1 packet each direction per kGranularity time until the async process returned?

TCP doesn't have an anti-amplification limit and it doesn't require Handshake packets to provide address validation.

I would characterize this as an issue with the client's anti-deadlock response, rather than a problem with PTO.

-- 
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/3546#issuecomment-604472286

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 03:56:28 -0700

I think the most intellectually clean thing to do here would be to not reset PTO count when an ACK is received and the PTO is only armed for anti-amplification purposes.  This is because even though at a transport level, everything is working(ACKs are quickly received), the client is arming the PTO for a very different reason(anti-deadlock).  And the PTO packets aren't allowing the server to make any more progress, because it's not blocked.

But I don't think that suggestion is very easy to implement, so I'm not sure it's what we want.

I'd suggest restricting the client to one anti-deadlock packet per second or some arbitrary period, which would be easier, but means someone has to pick a magic number.  I mention 1 second because it's a magic number already in QUIC recovery(default handshake timeout) and in TCP.  This basically amounts to anti-anti-amplification attack prevention I think :(

-- 
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/3546#issuecomment-604937626

---

From: Tatsuhiro Tsujikawa <[email-masked]>
Date: Fri, 27 Mar 2020 06:09:04 -0700

Does it make simpler if we reset pto_count only when ack-eliciting not pure probe packet (which includes lost data or new data, not just PING) is received without special casing for anti-deadlock packet?   This ensures that pto_count is reset only when endpoint has made a progress and delaying consecutive PTO if it only receives ACK for pure probe packet.  Server might be in the same situation if client somehow only sends ACK frame in Initial and server keeps sending Initial packet in PTO

-- 
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/3546#issuecomment-604991033

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 06:32:33 -0700

This is an interesting issue and I think @ianswett is correct in pointing out that this could be a real problem for slow devices doing public key operations in an asynchronous manner.

I like @tatsuhiro-t's suggestion to not reset the PTO timer. Maybe we can *always* increment it, regardless of the packet received from peer being ACK-eliciting or not.

-- 
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/3546#issuecomment-605002612

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 06:38:38 -0700

There are other cases when we send ack-eliciting packets including only PINGs or other non-application data and we do want to set pto_count back to 0 when those are acknowledged.

We could specifically recommend sending non-ack-eliciting packets for the anti-deadlock packet, but then you lose an opportunity for an RTT measurement early in the connection, which is unfortunate, though not a dealbreaker.  For full-sized Initial packets, it's likely they'd never be acknowledged if they weren't ack-eliciting, because no ack-eliciting Initial packets would be sent after them.

@kazuho I believe always incrementing pto_count would be detrimental to performance, unless it's under some very narrow circumstances.

-- 
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/3546#issuecomment-605005519

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 07:39:11 -0700

Sorry if I was unclear, I meant not resetting PTO timer when the client receives an Initial packet. Assuming that the server sends only a few (most likely one), I do not think not resetting is fine.

Note that in case the server is going to send a fair number of Initial packets (e.g., post quantum) it should be sending a retry regardless of what we decide here, as relying on CC wold be better than relying on 3x limit.

-- 
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/3546#issuecomment-605036118

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 07:45:29 -0700

We only reset pto_count upon receiving an Initial ACK, not any Initial packet, but I think your suggestion could work well.  It amounts to continuing with exponential backoff until the client has Handshake keys.

It's fairly simple to implement, and I wouldn't expect it to cause a noticeable regression, so unless someone comes up with something better, let's do 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/issues/3546#issuecomment-605039296

---

From: Martin Thomson <[email-masked]>
Date: Sun, 29 Mar 2020 19:51:32 -0700

So the operating principle here is that we are expecting progress to Handshake to be made, which means that we need to align with our existing logic that enables PTO in that case.

For reference, the check we use to enable PTO during the anti-amplification phase is the inverse of the following:

```
  if (no ack-eliciting packets in flight &&
      peer not awaiting address validation):
```

Which follows the "server at anti-amplification limit" check.  So maybe `(client && no ack-eliciting packets in flight && peer not awaiting address validation)` is the check you want rather than `is initial`.

-- 
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/3546#issuecomment-605754832

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 07:20:49 -0700

I updated the PR to only do this for the client, since I think that's the right thing to do.

I did the Initial check because I thought it was slightly simpler than the `peer not awaiting address validation` check, which I believe is equivalent, since receiving an Initial ACK means you still have Initial keys, so the server is awaiting address validation.  As soon as a Handshake ACK is received by the client, it knows the server has completed address validation.

But I could also use the PeerNotAwaitingAddressValidation() method that's already defined(and mistakenly not used, see #3555 

Then I think the check would be:
```
If (PeerNotAwaitingAddressValidation())
  pto_count = 0
```

-- 
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/3546#issuecomment-606028967

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 15:30:05 -0700

Looks good 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/3546#issuecomment-606285033