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

[Antoine Delignat-Lavaud <[e-mail]>]

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