openssl / project

Tracking of project related issues
2 stars 1 forks source link

Send odcid as orig_odcid is sent as QUIC ORIG_DCID parameter during retry retransmission of initial packet #916

Open nhorman opened 2 weeks ago

nhorman commented 2 weeks ago

In investigating the server address validation feature for QUIC, an error was encountered when attempting to complete the TLS handshake, after hacking up the code to do CRYPTO decoding properly (as described in https://github.com/openssl/project/issues/915)

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

with the hacked changes from #915 in place, running this test results in the client sending a connection close packet with the reason TRANSPORT_PARAM_ERROR. Sepecifically the client noted that the transport paramter orig_destination_connection_id was set to the new dcid, rather than the original dcid as provided in the first inital packet, leading to the error.

the task here is to modify the quic transport param generation code in ch_generate_transport_params such that, for servers, when available the odcid is encoded to this parameter, rather than the new dcid.

This code is a hack, but illustrates roughly what needs to be done:

commit ecfc56cfa1541380826ec4014db43c8dcfd22254
Author: Neil Horman <nhorman@openssl.org>
Date:   Tue Nov 5 10:52:52 2024 -0500

    ensure that ocid is sent after a retry frame causes new inital packet

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 0e7b2db65f..4224996d72 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -1716,6 +1716,7 @@ static int ch_generate_transport_params(QUIC_CHANNEL *ch)
     WPACKET wpkt;
     int wpkt_valid = 0;
     size_t buf_len = 0;
+    int rc;

     if (ch->local_transport_params != NULL || ch->got_local_transport_params)
         goto err;
@@ -1733,8 +1734,14 @@ static int ch_generate_transport_params(QUIC_CHANNEL *ch)
         goto err;

     if (ch->is_server) {
-        if (!ossl_quic_wire_encode_transport_param_cid(&wpkt, QUIC_TPARAM_ORIG_DCID,
-                                                       &ch->init_dcid))
+        if (ch->odcid.id_len == 0)
+            rc = ossl_quic_wire_encode_transport_param_cid(&wpkt, QUIC_TPARAM_ORIG_DCID,
+                                                           &ch->init_dcid);
+        else
+            rc = ossl_quic_wire_encode_transport_param_cid(&wpkt, QUIC_TPARAM_ORIG_DCID,
+                                                           &ch->odcid);
+
+        if (rc == 0)
             goto err;

         if (!ossl_quic_wire_encode_transport_param_cid(&wpkt, QUIC_TPARAM_INITIAL_SCID,
@@ -3371,6 +3378,9 @@ static int ch_on_new_conn_common(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
     ch->cur_peer_addr   = *peer;
     ch->init_dcid       = *peer_dcid;
     ch->cur_remote_dcid = *peer_scid;
+    ch->odcid.id_len = 0;
+    if (peer_odcid != NULL)
+        ch->odcid = *peer_odcid;

     /* Inform QTX of peer address. */
     if (!ossl_quic_tx_packetiser_set_peer(ch->txp, &ch->cur_peer_addr))
diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h
index 7cafa109c4..232201c23e 100644
--- a/ssl/quic/quic_channel_local.h
+++ b/ssl/quic/quic_channel_local.h
@@ -106,7 +106,7 @@ struct quic_channel_st {
      * Randomly generated and required by RFC to be at least 8 bytes.
      */
     QUIC_CONN_ID                    init_dcid;
-
+    QUIC_CONN_ID                    odcid;
     /*
      * Client: The SCID found in the first Initial packet from the server.
      * Not valid for servers.