Subject: [quicwg/base-drafts] Server should not accept 1-RTT traffic before handshake completion (#3159)

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Mon, 28 Oct 2019 03:46:19 -0700

In TLS 1.3, 1-RTT secrets can be derived once the server finished is computed, when the handshake is half-complete. This allows the server to send application data without waiting for the client finished flight (a feature sometimes called 0.5-RTT in the TLS WG). This is safe because checking the peer's finished message is only required for receiving, and the finished must be received before any application traffic when using the TLS record layer.

In QUIC, the server may receive 1-RTT packets before the client finished. This can happen because of packet loss, or an active adversary deliberately dropping handshake packets from the client, or because the client is malicious and does not send its finished flight at all. As far as I understand, the current QUIC spec allows the server to process the 1-RTT packets and hand them to the application without waiting for the client finished. Doing this falls outside the scope of what is currently proved secure about the TLS handshake, and it leads to multiple problems:
- Deadlock when the network drops the client finished flight: see #2863
- Potential to bypass client authentication requirements: if a server is configured with required client certificate authentication during the handshake (e.g. for access control to a service), a client may send the request before the client finished flight. If QUIC passes the request to the application, it may incorrectly believe that the client has been authenticated.
- Potential for downgrade attacks: the client finished is used to explicitly authenticate the negotiation of algorithms in the handshake. Due to its defensive design, TLS 1.3 also implicitly authenticates the transcript in the transport secret derivation. However, the implicit authentication only holds for strong AEAD algorithms, whereas the explicit authentication relies on strong hash algorithms (and/or signature, depending on the mode). 

A reliable way to prevent all of these 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 packet (the same handshake packet may be sent in multiple datagrams). 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 short 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 a new transport-level signal for handshake confirmation (as proposed in #3145) is a bad 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). Instead, QUIC should simply ensure that the signal has been received. It's only an issue on the server side, because clients cannot decrypt 1-RTT packets without processing the server finished.

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

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 28 Oct 2019 05:25:39 -0700

> In QUIC, the server may receive 1-RTT packets before the client finished.

While this is true, I wonder if we have the issues you point out, because we prohibit the use of such packets until the server receives Client Finished.

[Section 5.7 of the TLS draft](https://quicwg.org/base-drafts/draft-ietf-quic-tls.html#rfc.section.5.7) states that _a server MUST NOT process data from incoming 1-RTT protected packets before the TLS handshake is complete,_ where "handshake complete" is defined as _when the TLS stack has both sent a Finished message and verified the peer’s Finished message_ ([section 4.1.1](https://quicwg.org/base-drafts/draft-ietf-quic-tls.html#rfc.section.4.1.1)).

-- 
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/3159#issuecomment-546923231

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Mon, 28 Oct 2019 14:21:36 +0000 (UTC)

Yes, I agree that this spec is correct in that respect. However, it is clear that several implementations are not implementing this restriction correctly, which is understandable given that it requires server to actively delay processing buffered packets (i.e. use memory resources). I think it is much better to enforce the correct behavior by requiring that the finished is repeated in every datagram until the handshake is confirmed by the server, because the server is guaranteed that it does not need a logic for received data that cannot be passed to the application. 

-- 
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/3159#issuecomment-546968628

---

From: Martin Thomson <[email-masked]>
Date: Mon, 28 Oct 2019 20:46:27 -0700

This recommendation has been made a few times, but we have thus far resisted it on the basis of it not working with client authentication.

The suggestion that the server might offer proof that it didn't do something bad is new.  If I were inclined to take that suggestion seriously, I would instead say that the server should use an exporter to generate something.  You could even see how this might make its way into the proposed HANDSHAKE_DONE frame.

But I don't think that this needs this level of mechanism.  This is something that servers just need to do on their own.  Just like having a good source of randomness for their key generation, the consequences of failure here are that they might be open to attack.

I have no problem with test suites that purposefully "drop" critical packets to reveal misbehaving servers.  That might be better than adding mechanism.

That said, the server can just withhold ACK frames until it sees the whole handshake, even if it processes the entire handshake.  A server could answer requests from 1-RTT until clients start looking for that happening.  Ultimately, we can't stop the server from performing processing if it offers no reliable proof of that happening.  And I don't think that it's worth trying.

-- 
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/3159#issuecomment-547243572

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Tue, 29 Oct 2019 09:05:52 -0700

To help decide whether this is likely to be a problem, I decided to run an experiment. I wrote a very simple patch to OpenSSL to disable sending a finished:

```diff
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index a11ba60..9681787 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -439,6 +439,7 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;

     case TLS_ST_CR_FINISHED:
         if (s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY
                 || s->early_data_state == SSL_EARLY_DATA_FINISHED_WRITING)
             st->hand_state = TLS_ST_PENDING_EARLY_DATA_END;
@@ -446,8 +447,8 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
                  && s->hello_retry_request == SSL_HRR_NONE)
             st->hand_state = TLS_ST_CW_CHANGE;
         else
-            st->hand_state = (s->s3.tmp.cert_req != 0) ? TLS_ST_CW_CERT
-                                                        : TLS_ST_CW_FINISHED;
+            st->hand_state = (s->s3.tmp.cert_req != 0) ? TLS_ST_CW_CERT : TLS_ST_OK;
         return WRITE_TRAN_CONTINUE;

     case TLS_ST_PENDING_EARLY_DATA_END:
@@ -459,8 +460,9 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)

     case TLS_ST_CW_END_OF_EARLY_DATA:
     case TLS_ST_CW_CHANGE:
-        st->hand_state = (s->s3.tmp.cert_req != 0) ? TLS_ST_CW_CERT
-                                                    : TLS_ST_CW_FINISHED;
+        st->hand_state = (s->s3.tmp.cert_req != 0) ? TLS_ST_CW_CERT : TLS_ST_OK;
         return WRITE_TRAN_CONTINUE;

     case TLS_ST_CW_CERT:
```

I checked with `openssl s_client -msg` to check that the patch works as expected. Then, I tried to connect to various public h3-23 endpoints listed on https://github.com/quicwg/base-drafts/wiki/Implementations using the ngtcp2 client built with my patched OpenSSL. The point of the experiment is to check which servers would acknowledge the short packets that I sent. Note that there can be false negatives in this experiment if the server is passing the 1-RTT data to the application but withholding the ACKS (this is insecure because the application may make unsafe side effects based on the received data, e.g. a POST/DELETE request).

Two servers on the list clearly implement a wrong behavior:

- quic.tech:4433 straight up acknowledged the 1-RTT packet in a short packet
```
I00000042 0xfd9df02263e4eeab1552a84d0e608bfa91 frm rx 0 Short(0x70) ACK(0x02) largest_ack=0 ack_delay=15(1919) ack_block_count=0
I00000042 0xfd9df02263e4eeab1552a84d0e608bfa91 frm rx 0 Short(0x70) ACK(0x02) block=[0..0] block_count=0
```

- http3-test.litespeedtech.com: acknowledges the client 1-RTT packet, but does that in an handshake packet (!)
```
I00000618 0x7d6af4994dbcab77dba708cb2ab0ef553f pkt rx pkn=5 dcid=0x7d6af4994dbcab77dba708cb2ab0ef553f scid=0x4b54982b8489544f type=Handshake(0x02) len=22 k=0
I00000618 0x7d6af4994dbcab77dba708cb2ab0ef553f frm rx 5 Handshake(0x02) ACK(0x02) largest_ack=0 ack_delay=0(0) ack_block_count=0
I00000618 0x7d6af4994dbcab77dba708cb2ab0ef553f frm rx 5 Handshake(0x02) ACK(0x02) block=[0..0] block_count=0
I00000618 0x7d6af4994dbcab77dba708cb2ab0ef553f pkt read packet 56 left 0
```

It seems very likely that some implementations will ignore this requirement in practice (and understandably so, as buffering the 1-RTT packets is expensive), to the point that I think it is worth making the correct behavior verifiable by the client (as it somewhat exposes the client in the downgrade case, although a complete exploit seems unlikely with a signature, and difficult with a PSK binder)

-- 
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/3159#issuecomment-547499323

---

From: Subodh Iyengar <[email-masked]>
Date: Tue, 29 Oct 2019 10:17:07 -0700

ya in hindsight, the keyschedule for TLS 1.3 might have derived client_application_traffic_secret_0 from the same handshake context as resumption_master_secret. Maybe in TLS 1.4? ;)

-- 
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/3159#issuecomment-547534283

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Tue, 29 Oct 2019 10:32:16 -0700

Yes... without changing the 1.3 key schedule QUIC could also use the exporter MS as the client 1-RTT traffic secret.

-- 
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/3159#issuecomment-547541118

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 29 Oct 2019 14:20:34 -0700

@ad-l Thank you for sharing the results. This is very interesting. FWIW, I do not think QUIC stacks using picotls would have the issue, as picotls hands out the 1-RTT read key only after processing ClientFinished... but I can see that the API might vary between the TLS stacks.

Though, if we are to have a protocol design that enforces the server to wait for CF, just bundling CF in every 1-RTT packet is not enough. I'd argue that CF should include some bits that is unpredictable by the 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/3159#issuecomment-547634533

---

From: martinduke <[email-masked]>
Date: Tue, 29 Oct 2019 14:55:00 -0700

Like @kazuho, our TLS doesn't even deliver the receive keys until it gets Finished. Rather than mess with the spec or rely on various QUIC implementations to do it right, I think it's best to fix the various TLS implementations (or, more specifically, their QUIC-facing secret export API) to do this correctly. There are relatively few of these and it's a problem specific to TLS 1.3.

The ability to coalesce sent packets is optional and the spec goes to great pains to not require sending them at any point, which has caused much suffering for @ianswett. Moreover, I am not at all convinced that coalescing Finished solves the 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/issues/3159#issuecomment-547646884

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Oct 2019 16:29:26 -0700

This might be receptive to an interop-level test.  Dropping Handshake packets from the client should lead to a situation where the client provides ample temptation for the server to break this rule.

-- 
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/3159#issuecomment-547674079

---

From: Marten Seemann <[email-masked]>
Date: Tue, 29 Oct 2019 18:27:06 -0700

> Without changing the 1.3 key schedule QUIC could use the exporter MS to derive the client 1-RTT traffic secret.

@ad-l How does this help? Can't the exporter MS derived at the same time as the traffic secrets?

-- 
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/3159#issuecomment-547699180

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Oct 2019 21:00:45 -0700

My bad for leading you astray there. I forgot that only the resumption secret is spun off after the client Finished.  Yes, an exporter doesn't help.

(As an aside, I'm fairly made exporters that way to allow them to be used with QUIC.  Bit of a fail there in retrospect.)

-- 
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/3159#issuecomment-547729682

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Tue, 29 Oct 2019 21:01:55 -0700

> @ad-l How does this help? Can't the exporter MS derived at the same time as the traffic secrets?

Sorry, for some reason I was convinced that exporter master includes client finished, but after double checking, this is only the case of resumption master (honestly I think this is wrong because exporter are used for binding authentication and they pretty much fail at binding client authentication. I would suggest this is fixed in future TLS). 

-- 
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/3159#issuecomment-547729843

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Tue, 29 Oct 2019 21:04:20 -0700

> The ability to coalesce sent packets is optional and the spec goes to great pains to not require sending them at any point, which has caused much suffering for @ianswett. Moreover, I am not at all…

I agree, coalescing Finished is just a way to avoid server being blocked. The proper fix to the problem is to ensure that the client traffic secret for 1-RTT is derived from a transcript that includes client finished, as Subodh suggests. Unfortunately, only the TLS resumption master secret includes 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/3159#issuecomment-547730366

---

From: Marten Seemann <[email-masked]>
Date: Tue, 29 Oct 2019 21:41:24 -0700

Would it be unreasonable to derive the client's 1-RTT send key from the resumption master secret then?
I guess one downside of this approach is that we'd be using different key derivation paths for server and client, since the server needs to be able to send 0.5-RTT data.

-- 
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/3159#issuecomment-547736383

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Oct 2019 22:07:21 -0700

I'd advise against doing anything like that.  A change to the key schedule is very disruptive.

-- 
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/3159#issuecomment-547740662

---

From: Marten Seemann <[email-masked]>
Date: Tue, 29 Oct 2019 22:12:57 -0700

Is it disruptive because we'd all have to update our implementations (I imagine it would just be one more HKDF call a QUIC stack would perform when the TLS stacks hands it the 1-RTT keys, wouldn't it?), or are there any cryptographic reasons not to do this?

I'm just trying to understand the issue here better, not advocating for one way or the other at this point.

-- 
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/3159#issuecomment-547741663

---

From: martinduke <[email-masked]>
Date: Tue, 29 Oct 2019 22:32:02 -0700

My feeble crypto brain says it would be fine, but it feels like enough of a deviation from TLS1.3 to need a formal security analysis.

-- 
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/3159#issuecomment-547745062

---

From: martinduke <[email-masked]>
Date: Tue, 29 Oct 2019 22:36:49 -0700

Figure 5 of quic-tls is pretty clear on what should happen, but I think that section could use some normative text.

-- 
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/3159#issuecomment-547745916

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Oct 2019 23:55:33 -0700

> are there any cryptographic reasons not to do this?

If we did something like this, it would likely trigger completely new analysis of the protocol.  The thing that concerns me is that it would reduce or eliminate key separation between resumption keys and traffic keys, which is a concern.  We'd be better off creating a new "secret" with the same transcript being input to its expansion.  But that smells a lot like defining TLS 1.4 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/3159#issuecomment-547763004

---

From: MikkelFJ <[email-masked]>
Date: Wed, 30 Oct 2019 00:52:38 -0700

@marten-seemann it's disruptive also because of possible ongoing hardware design. That said, if it is important enough to fix ...

-- 
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/3159#issuecomment-547778190

---

From: Alessandro Ghedini <[email-masked]>
Date: Thu, 31 Oct 2019 05:54:49 -0700

> quic.tech:4433 straight up acknowledged the 1-RTT packet in a short packet

This is on purpose btw. We process the 1-RTT packet (as in, we decrypt it), but do not process the frames it contains until the handshake is complete. The reasoning behind this was to avoid having to buffer potentially invalid 1-RTT packets.

-- 
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/3159#issuecomment-548360061

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Thu, 31 Oct 2019 06:19:16 -0700

> This is on purpose btw. We process the 1-RTT packet (as in, we decrypt it), but do not process the frames it contains until the handshake is complete. The reasoning behind this was to avoid having…
If you don't look at the frames but acknowledge the packets when do you stop buffering them? When the server is out of memory?

-- 
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/3159#issuecomment-548370434

---

From: Alessandro Ghedini <[email-masked]>
Date: Thu, 31 Oct 2019 06:22:07 -0700

There's a limit on the number of packets buffered, something like "initial congestion window"-amount of packets.

-- 
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/3159#issuecomment-548371683

---

From: ianswett <[email-masked]>
Date: Thu, 31 Oct 2019 06:24:30 -0700

@ghedo the transport spec says you MUST not acknowledge a packet until it's been processed.

Given you're limiting the number of buffered packets, I don't see any obvious gain from trying to decrypt them before buffering them.  The buffer will be cleared when the handshake completes no matter what.

-- 
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/3159#issuecomment-548372774

---

From: Alessandro Ghedini <[email-masked]>
Date: Thu, 31 Oct 2019 06:28:28 -0700

Yeah, but that doesn't mean I should still buffer junk packets if I can avoid 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/3159#issuecomment-548374531

---

From: Alessandro Ghedini <[email-masked]>
Date: Thu, 31 Oct 2019 06:29:48 -0700

As for the not ackng, sure, but that's a separate 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/issues/3159#issuecomment-548375119

---

From: Antoine Delignat-Lavaud <[email-masked]>
Date: Thu, 31 Oct 2019 06:37:00 -0700

> As for the not ackng, sure, but that's a separate problem.

Agree it's off topic, but it's still very interesting for security research. I would never have expected implementations to do this and the security implications for stream integrity can be very surprising in some scenarios (such as compromised PSK), so thanks for clarifying 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/3159#issuecomment-548378457

---

From: Alessandro Ghedini <[email-masked]>
Date: Thu, 31 Oct 2019 06:38:39 -0700

@ianswett can you point me where the draft says that? I can't seem to be able to find it in https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#rfc.section.13.2

-- 
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/3159#issuecomment-548379191

---

From: ianswett <[email-masked]>
Date: Thu, 31 Oct 2019 06:45:02 -0700

It's the first sentence in 13.1:
"A packet MUST NOT be acknowledged until packet protection has been successfully removed and all frames contained in the packet have been processed."

-- 
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/3159#issuecomment-548382040

---

From: MikkelFJ <[email-masked]>
Date: Thu, 31 Oct 2019 06:47:31 -0700

Sure, you can't ACK or process a packet before the key space is valid, but you can buffer them. There is not point in buffering garbage. Forcing keys to only be available after the key space is confirmed amounts to babysitting and there are plenty other things any endpoint can do wrong. Receiving an early ACK is ofcourse a protocol violation.

-- 
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/3159#issuecomment-548383177

---

From: MikkelFJ <[email-masked]>
Date: Thu, 31 Oct 2019 06:49:12 -0700

In interesting aside: how do you deal with max ack delay when you buffer packets that cannot be processed yet?

-- 
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/3159#issuecomment-548383920