openssl / project

Tracking of project related issues
2 stars 1 forks source link

Re(generate) initial packet secrets based on odcid when available during retry packet response/address validation #915

Closed nhorman closed 3 days ago

nhorman commented 2 weeks ago

During the investigation of the QUIC server address validation feature, we discovered that, while we could send retry packets to an initiating client, for which the client would correctly send back a new Initial packet, the TLS handshake would never complete between a client and server. In testing with the following reproducer:

server:
quic-client-block 8888 ../../test/certs/servercert.pem ../../test/certs/serverkey.pem

client:
SSLKEYLOGFILE=./keylog.txt ./quic-client-block 127.0.0.1 8888

resulted in the following tcpdump: image

A few extra printfs indicated that the call to qrx_decrypt_pkt_body made in qrx_process_pkt was failing, indicating that the cipher used to decode the CRYPTO data in the inital packet sent in response to the retry was failing to get decoded.

This is occuring because the input key/iv to the AES-128-GCM cipher used to do the encryption/decryption here is based on the destination connection id used, which changes between the original and second initial packets. The client follows RFC 9000, which in section 17.2.5.2:

A client sets the Destination Connection ID field of this Initial
   packet to the value from the Source Connection ID field in the Retry
   packet.  Changing the Destination Connection ID field also results in
   a change to the keys used to protect the Initial packet.  It also
   sets the Token field to the token provided in the Retry packet.  The
   client MUST NOT change the Source Connection ID because the server
   could include the connection ID as part of its token validation
   logic; see Section 8.1.4.

So the client properly creates a new set of keys to encrypt the new initial packet CRYPTO section, but the server is still using the original destination cid derived key/iv to decode, leading to the failure.

The task here is to adjust the server side channel creation code such that, when handling a inital packet sent in response to a retry packet, keys are regenerated using the new connection id, rather than the old.

This code is functional in that regard, but needs cleaning:

commit 0556ab493c1c75e3fe5f9b65d33cc9b56da3c147
Author: Neil Horman <nhorman@openssl.org>
Date:   Tue Nov 5 06:43:28 2024 -0500

    Generate initial secrets based on new dcid rather than odcid

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index b8b0b0743c..0e7b2db65f 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -3364,7 +3364,8 @@ static void ch_on_idle_timeout(QUIC_CHANNEL *ch)

 static int ch_on_new_conn_common(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
                                  const QUIC_CONN_ID *peer_scid,
-                                 const QUIC_CONN_ID *peer_dcid)
+                                 const QUIC_CONN_ID *peer_dcid,
+                                 const QUIC_CONN_ID *peer_odcid)
 {
     /* Note our newly learnt peer address and CIDs. */
     ch->cur_peer_addr   = *peer;
@@ -3395,7 +3396,7 @@ static int ch_on_new_conn_common(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
         return 0;

     /* Register the peer ODCID in the LCIDM. */
-    if (!ossl_quic_lcidm_enrol_odcid(ch->lcidm, ch, &ch->init_dcid))
+    if (!ossl_quic_lcidm_enrol_odcid(ch->lcidm, ch, peer_odcid == NULL ? peer_dcid : peer_odcid))
         return 0;

     /* Change state. */
@@ -3416,7 +3417,7 @@ int ossl_quic_channel_on_new_conn(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
     if (!ossl_quic_lcidm_generate_initial(ch->lcidm, ch, &ch->cur_local_cid))
         return 0;

-    return ch_on_new_conn_common(ch, peer, peer_scid, peer_dcid);
+    return ch_on_new_conn_common(ch, peer, peer_scid, peer_dcid, NULL);
 }

 int ossl_quic_bind_channel(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
@@ -3438,7 +3439,7 @@ int ossl_quic_bind_channel(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
      * peer_odcid <=> is initial dst conn id chosen by peer in its
      * first initial packet we received without token.
      */
-    return ch_on_new_conn_common(ch, peer, peer_scid, peer_odcid);
+    return ch_on_new_conn_common(ch, peer, peer_scid, peer_dcid, peer_odcid);
 }

 SSL *ossl_quic_channel_get0_ssl(QUIC_CHANNEL *ch)