Subject: Re: [quicwg/base-drafts] unrecoverable loss pattern leads to deadlock (#2863)

From: Lars Eggert <[email-masked]>
Date: Wed, 16 Oct 2019 12:10:12 -0700

Discussed in Cupertino. With the change in #3066 and #3093, we believe this deadlock issue is addressed. (also, @kazuho has a pending edit).

-- 
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/2863#issuecomment-542848744

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 16 Oct 2019 12:11:42 -0700

@larseggert My point is https://github.com/quicwg/base-drafts/pull/3093#issuecomment-542848391.

-- 
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/2863#issuecomment-542849368

---

From: MikkelFJ <[email-masked]>
Date: Thu, 17 Oct 2019 22:39:07 -0700

Has anyone thought about revisiting implicit ACK's in handshake? It appears to me that many problems occur because of waiting for an ACK that doesn't happen?

-- 
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/2863#issuecomment-543518851

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 17 Oct 2019 23:31:24 -0700

IIUC, we agreed today in Cupertino that forbidding handshake key discarding (#3121) is the solution (at least for the time being), based on the assumption that it works.

I am starting to wonder if it works as expected.

Consider the case where the client migrates to a new path once the handshake is confirmed. That is in fact the recommended behavior when server-preferred address is used.

But IIUC, the existing transport logic is designed based on the assumption that path migration happens only after the exchange of Handshake packets have ceased.

I think I have found at least one issue: when the client intentionally migrates to a new path while still having unacknowledged handshake data, how does it choose the new client CID when sending the Handshake packet? Current design assumes that the *server* to pick the client CID from the pool of the CIDs it has received with the NEW_CONNECTION_ID frames. That works for 1-RTT packets, because 1-RTT packet does not have the Source CID field. But when using the long header packets, the sender choose the Source CID (the source being the client in this particular case). What is the solution here?

-- 
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/2863#issuecomment-543540677

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 18 Oct 2019 00:04:04 -0700

Regarding the way on discarding the Handshake keys, I think the failures the WG have made until now are that we have been looking for a clever method in trying to *infer* when a Handshake key can be dropped, and/or using 1-RTT packet to tell the peer that the peer can drop the handshake keys (the problem here is that the peer might continue retransmitting Handshake packets if the endpoint ceases to send 1-RTT packets). Based on the discussion in Cupertino, I think that if we are to drop the Handshake keys, we should adopt a safer approach.

I'd argue that there exists a class of approaches that can be considered safe. That is to require an endpoint to retain the Handshake keys until the peer sends an explicit signal instructing the endpoint to drop the Handshake keys. Assuming that my observation in [the comment right above](https://github.com/quicwg/base-drafts/issues/2863#issuecomment-543540677) is correct, we'd also need to adjust the transition to the handshake confirmed state to when an endpoint sends and receives that message.

That signal can be a new frame, a header bit, or a KeyUpdate.

-- 
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/2863#issuecomment-543555764

---

From: MikkelFJ <[email-masked]>
Date: Fri, 18 Oct 2019 05:24:28 -0700

What if each peer sends a required PING every 1/2 RTT until it sees a 1-RTT packet from peer or it times out using an aggressive version of PTO? Or something? And preferably replace handshake ACK's with implicit ACK's if possible.

-- 
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/2863#issuecomment-543713118

---

From: MikkelFJ <[email-masked]>
Date: Fri, 18 Oct 2019 05:27:55 -0700

Or better a each peer sends a handshake-PING  (HSPING) such the a received HSPING indicates that the peer is still not happy. This differs from ACK's in that it is unsolicited and therefore cannot hang.

-- 
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/2863#issuecomment-543714801

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 18 Oct 2019 06:54:34 -0700

@kazuho, it sounds like you are recommending something like the previous RETIRE_KEYS proposal?

-- 
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/2863#issuecomment-543758190

---

From: Nick Banks <[email-masked]>
Date: Fri, 18 Oct 2019 15:27:09 +0000 (UTC)

I would definitely prefer something like that over never getting rid of the handshake 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/issues/2863#issuecomment-543797023

---

From: martinduke <[email-masked]>
Date: Fri, 18 Oct 2019 08:58:12 -0700

I still prefer an implicit signal, and can think we can get it to work. But I would much prefer an explicit frame to keeping it forever.

-- 
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/2863#issuecomment-543809468

---

From: ekr <[email-masked]>
Date: Fri, 18 Oct 2019 15:34:31 -0700

> I think I have found at least one issue: when the client intentionally migrates to a new path while still having unacknowledged handshake data, how does it choose the new client CID when sending the Handshake packet? Current design assumes that the server to pick the client CID from the pool of the CIDs it has received with the NEW_CONNECTION_ID frames. That works for 1-RTT packets, because 1-RTT packet does not have the Source CID field. But when using the long header packets, the sender choose the Source CID (the source being the client in this particular case). What is the solution here?

Why not simply prohibit migration until all handshake packets have been ACKed. There is no ambiguity here, as the server is forbidden from initiating migration.

-- 
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/2863#issuecomment-543984852

---

From: ianswett <[email-masked]>
Date: Fri, 18 Oct 2019 16:03:41 -0700

When the handshake is confirmed, all the Handshake data is known to be delivered and Handshake recovery state can be discarded.  Previously, this was the moment Handshake keys were discarded, so the recovery doc relied on text about key discard to discard recovery state.

But now that the Handshake keys are staying around longer than the Handshake confirmed point, I'd suggest we add a paragraph to section 5.5(https://tools.ietf.org/html/draft-ietf-quic-recovery-23#section-5.5) like:
"When the handshake has been confirmed, all data sent in Handshake packet is known to be delivered and all recovery state should be discarded."

To be clear, this suggestion is not a hack.  We should do this if we're going to keep Handshake keys past Handshake Confirmed(which every solution I believe requires?), regardless of how long or why.  I'll note that the server can do this upon receipt of any 1-RTT packet, but waiting for an ACK isn't likely to delay dropping Handshake state significantly and is consistent between client and server.

-- 
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/2863#issuecomment-543995942

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 18 Oct 2019 16:26:18 -0700

@ekr
> Why not simply prohibit migration until all handshake packets have been ACKed. There is no ambiguity here, as the server is forbidden from initiating migration.

The problem here is that even in the case where an endpoint (e.g., server) has received all the handshake transcript and also the ACKs for all the ack-eliciting Handshake packets it has sent, the other side (e.g., client) might have still some data outstanding (due to ACKs being lost).

@ianswett
> I'd suggest we add a paragraph to section 5.5(https://tools.ietf.org/html/draft-ietf-quic-recovery-23#section-5.5) like:

It's not only the the receive-side state of the Handshake packets that you need to retain forever, but also the state related to that path (e.g., socket). I am not sure we'd prefer that.

-- 
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/2863#issuecomment-544003607

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 18 Oct 2019 16:37:51 -0700

@RyanAtGoogle
> it sounds like you are recommending something like the previous RETIRE_KEYS proposal?

That might be the case (honestly I do not remember the details of the RETIRES_KEYS proposal), though I think I'm inclined to suggesting using a continuous signal (i.e., a Key Update or burning an unused header bit). The reasons being i) using a continuous signal allows us to abandon the Handshake keys earlier ii) we do not need to define a new signal that needs to be retransmitted in the transport draft, or rely on a particular behavior defined in the recovery draft that is not generally considered as necessary for a recovery logic (i.e. PTO "MUST" trigger a PING when there is no data to send).

-- 
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/2863#issuecomment-544007910

---

From: martinduke <[email-masked]>
Date: Fri, 18 Oct 2019 17:16:56 -0700

I remember thinking RETIRE_KEYS was extraneous given all the implicit
signals. But it seems that screwy-but-legal behavior seems to make those
implicit signals risky. So let's stop being cute and just use the signal.

On Fri, Oct 18, 2019, 4:37 PM Kazuho Oku <[email-masked]> wrote:

> @RyanAtGoogle <https://github.com/RyanAtGoogle>
>
> That might be the case (honestly I do not remember the details of the
> RETIRES_KEYS proposal), though I think I'm inclined to suggesting using a
> continuous signal (i.e., a Key Update or burning an unused header bit). The
> reasons being i) using a continuous signal allows us to abandon the
> Handshake keys earlier ii) we do not need to define a new signal that needs
> to be retransmitted in the transport draft, or rely on a particular
> behavior defined in the recovery draft that is not generally considered as
> necessary for a recovery logic (i.e. PTO "MUST" trigger a PING when there
> is no data to send).
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/issues/2863?email_source=notifications&email_token=AF2EYEJOXBO3V7VFTTX5MJDQPJCFDA5CNFSM4H4LZFRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBWONZQ#issuecomment-544007910>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AF2EYEOL7XJAQAYKIY6MYLLQPJCFDANCNFSM4H4LZFRA>
> .
>

-- 
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/2863#issuecomment-544019361

---

From: ianswett <[email-masked]>
Date: Fri, 18 Oct 2019 20:08:24 -0700

In response to @kazuho "when the client intentionally migrates to a new path while still having unacknowledged handshake data, how does it choose the new client CID when sending the Handshake packet?"

I'm arguing the client should never have unacknowledged Handshake data when it migrates, because it waits for handshake confirmation to migrate and at that point, all Handshake data should be discarded.

There is a possibility the server does not believe the handshake is confirmed but the client does.  In that case, if the client didn't keep listening on the old path when it migrated AND the server didn't send anything in 1-RTT, there would be a problem.  But don't we require both client and server to validate the path in this case, which would mean both would achieve handshake confirmation on the new path, if they had not done so previously?

-- 
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/2863#issuecomment-544070052

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 19 Oct 2019 05:35:00 -0700

@ianswett 
> There is a possibility the server does not believe the handshake is confirmed but the client does.  In that case, if the client didn't keep listening on the old path when it migrated AND the server didn't send anything in 1-RTT, there would be a problem.

Yes. This is the case I am pointing out.

>  But don't we require both client and server to validate the path in this case, which would mean both would achieve handshake confirmation on the new path, if they had not done so previously?

I am not sure if there is a guarantee that handshake confirmation would be obtained before the path is validated, because the path might become validated while all the packets carrying 1-RTT ACKs get dropped. And at that point, the endpoints might cease sending 1-RTT packets to each other.

-- 
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/2863#issuecomment-544139101

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Oct 2019 01:14:35 -0700

What @kazuho points out is a problem. Keeping HS keys forever prevents the deadlock, but it does so by admitting that HS packets can be sent forever. This means after migration as well. That is something we don't consider in the spec. 

> I'm arguing the client should never have unacknowledged Handshake data when it migrates, because it waits for handshake confirmation to migrate and at that point, all Handshake data should be disca…
Handshake data is discarded when the handshake keys are dropped, not when confirmation is reached (see [section 5.3 in recovery draft](https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#handshakes-and-new-paths). Now with handshake keys forever, the client can keep retransmitting forever. And as @kazuho notes, the server can surely send keep retransmitting as well since if it hasn't reached confirmation yet.

There are a few ways to curb this... but I'm strongly inclined towards an explicit signal from both sides indicating handshake confirmation, after which migration can be initiated.

-- 
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/2863#issuecomment-544852798

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Fri, 25 Oct 2019 06:43:24 -0700

There is a much larger issue at stake here. The currently published proofs for the TLS 1.3 handshake do not allow the server to process 1-RTT packets until the client finished has been verified. It does not mean there is an attack, because of the defensive design of TLS 1.3 which implicitly authenticates the transcript in the transport secret derivation. However, this is still a weaker argument than checking the finish, particularly with respect to downgrade attacks. If a server supports a transport ciphersuite with broken integrity (e.g. someone discovers a way to forge CCM8 tags), then clients of this server that support strong ciphersuites may still be attacked.

There is also a clear risk with client authentication. If a server is configured to require a client certificate, but QUIC gives to the application 1-RTT packets before the client finished flight has been received, the application may incorrectly believe that this data has passed the client authentication check. 

A reliable way to prevent such issues is to mandate that every 1-RTT packet sent by the client before the client finished frame is acknowledged by the server must include in the same datagram a copy of the finished frame (in the form of a separate handshake packet, before the short 1-RTT packet). The overhead is limited because finished messages are short, so even with the overhead of the long packet headers and AEAD tag, less than 100 bytes are used in the datagram (leaving the rest for the 1-RTT packet). If this is done, it is possible to enforce that a server must have checked the client finished before processing 1-RTT packets from the client. Adding some separate signal for handshake confirmation is a terrible idea - in TLS, receiving the peer's finished message IS the the explicit handshake confirmation signal (which is why CCS is removed in 1.3). Restoring a transport-level signal is a clear step backwards and must be avoided. 

-- 
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/2863#issuecomment-546359389

---

From: David Schinazi <[email-masked]>
Date: Fri, 25 Oct 2019 11:35:31 -0700

Hi @ad-l, since you're describing a "much larger issue" that I agree should be discussed, could you please open a separate issue? This issue is about a specific deadlock that can happen during the handshake, not about the security properties of the 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/issues/2863#issuecomment-546464716