Fix AEAD validation of initial packets in port

The interoperability tests disable client ip address
validation done by RETRY packet. All tests done in CI
take code path which sends a retry packet.

The first initial packet sent by client uses a different
initial encryption level keys to protect packet integrity.
The keys are derived from DCID chosen by client.

When server accepts connection on behalf of initial packet,
the 'DCID' gets changed which means the initial level encryption keys
are changing too. So when server skips sending a retry packet,
it must forget the qrx which was used to validate initial
packet sent by client.

Forgetting qrx is not straightforward, we must salvage the
unencrypted packets left there after they were validated.
Those unencrypted packets must be injected to newly created channel.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26808)

diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h
index b782f0d66b74058acdecdf67c1814d7693ddcf8e..07b5c218caec3365d222413580ff3c6bd69431c4 100644 (file)
--- a/include/internal/quic_channel.h
+++ b/include/internal/quic_channel.h
@@ -300,6 +300,8 @@ void ossl_quic_channel_on_stateless_reset(QUIC_CHANNEL *ch);
 
 void ossl_quic_channel_inject(QUIC_CHANNEL *ch, QUIC_URXE *e);
 
+void ossl_quic_channel_inject_pkt(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpkt);
+
 /*
  * Queries and Accessors
  * =====================

diff --git a/include/internal/quic_record_rx.h b/include/internal/quic_record_rx.h
index 54f3a6321b1bed2f12b5b699fb029726d98b5182..8db7f3fc0dcc429dc0ca0009d5fd91cd0d1195d2 100644 (file)
--- a/include/internal/quic_record_rx.h
+++ b/include/internal/quic_record_rx.h
@@ -320,6 +320,7 @@ int ossl_qrx_set_late_validation_cb(OSSL_QRX *qrx,
  * establish a new connection.
  */
 void ossl_qrx_inject_urxe(OSSL_QRX *qrx, QUIC_URXE *e);
+void ossl_qrx_inject_pkt(OSSL_QRX *qrx, OSSL_QRX_PKT *pkt);
 int ossl_qrx_validate_initial_packet(OSSL_QRX *qrx, QUIC_URXE *urxe,
                                      const QUIC_CONN_ID *dcid);
 

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 262962973fe0e72bfe18229630ec03c315c99825..169d96a8ded0ad7fb8711e56ca3825d33f39d8c7 100644 (file)
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -3443,6 +3443,11 @@ void ossl_quic_channel_inject(QUIC_CHANNEL *ch, QUIC_URXE *e)
     ossl_qrx_inject_urxe(ch->qrx, e);
 }
 
+void ossl_quic_channel_inject_pkt(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpkt)
+{
+    ossl_qrx_inject_pkt(ch->qrx, qpkt);
+}
+
 void ossl_quic_channel_on_stateless_reset(QUIC_CHANNEL *ch)
 {
     QUIC_TERMINATE_CAUSE tcause = {0};

diff --git a/ssl/quic/quic_port.c b/ssl/quic/quic_port.c
index db1bd48822e281acb5456625d75e2c99821976ed..64ebf1c4d657f09fe8184900bc553c032cc9e2f7 100644 (file)
--- a/ssl/quic/quic_port.c
+++ b/ssl/quic/quic_port.c
@@ -1460,8 +1460,10 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
     QUIC_CONN_ID odcid, scid;
     uint8_t gen_new_token = 0;
     OSSL_QRX *qrx = NULL;
+    OSSL_QRX *qrx_src = NULL;
     OSSL_QRX_ARGS qrx_args = {0};
     uint64_t cause_flags = 0;
+    OSSL_QRX_PKT *qrx_pkt = NULL;
 
     /* Don't handle anything if we are no longer running. */
     if (!ossl_quic_port_is_running(port))
@@ -1570,6 +1572,23 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
     if (ossl_qrx_validate_initial_packet(qrx, e, (const QUIC_CONN_ID *)dcid) == 0)
         goto undesirable;
 
+    if (port->validate_addr == 0) {
+        /*
+         * Forget qrx, because it becomes (almost) useless here. We must let
+         * channel to create a new QRX for connection ID server chooses. The
+         * validation keys for new DCID will be derived by
+         * ossl_quic_channel_on_new_conn() when we will be creating channel.
+         * See RFC 9000 section 7.2 negotiating connection id to better
+         * understand what's going on here.
+         *
+         * Did we say qrx is almost useless? Why? Because qrx remembers packets
+         * we just validated. Those packets must be injected to channel we are
+         * going to create. We use qrx_src alias so we can read packets from
+         * qrx and inject them to channel.
+         */
+         qrx_src = qrx;
+         qrx = NULL;
+    }
     /*
      * TODO(QUIC FUTURE): there should be some logic similar to accounting half-open
      * states in TCP. If we reach certain threshold, then we want to
@@ -1577,13 +1596,6 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
      */
     if (port->validate_addr == 1 && hdr.token == NULL) {
         port_send_retry(port, &e->peer, &hdr);
-        /*
-         * This is a kind of bummer because we forget secrets for initial
-         * level encryption. The secrets costs us CPU to compute. What we can
-         * do here is to store them within retry token. Then we can retrieve them
-         * from initial packet which will carry our retry token to validate
-         * client's address.
-         */
         goto undesirable;
     }
 
@@ -1616,6 +1628,16 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
             port_send_retry(port, &e->peer, &hdr);
             goto undesirable;
         }
+
+        /*
+         * client is under amplification limit, until it completes
+         * handshake.
+         *
+         * forget qrx so channel can create a new one
+         * with valid initial encryption level keys.
+         */
+        qrx_src = qrx;
+        qrx = NULL;
     }
 
     port_bind_channel(port, &e->peer, &scid, &hdr.dst_conn_id,
@@ -1634,10 +1656,19 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
     if (gen_new_token == 1)
         generate_new_token(new_ch, &e->peer);
 
-    /*
-     * The qrx belongs to channel now, so don't free it.
-     */
-    qrx = NULL;
+    if (qrx != NULL) {
+        /*
+         * The qrx belongs to channel now, so don't free it.
+         */
+        qrx = NULL;
+    } else {
+        /*
+         * We still need to salvage packets from almost forgotten qrx
+         * and pass them to channel.
+         */
+        while (ossl_qrx_read_pkt(qrx_src, &qrx_pkt) == 1)
+            ossl_quic_channel_inject_pkt(new_ch, qrx_pkt);
+    }
 
     /*
      * If function reaches this place, then packet got validated in
@@ -1654,6 +1685,7 @@ static void port_default_packet_handler(QUIC_URXE *e, void *arg,
 
 undesirable:
     ossl_qrx_free(qrx);
+    ossl_qrx_free(qrx_src);
     ossl_quic_demux_release_urxe(port->demux, e);
 }
 

diff --git a/ssl/quic/quic_record_rx.c b/ssl/quic/quic_record_rx.c
index 63aa9aff8fa1545cdf520aedd65de1d71f40d8e0..29625118ae474475480472bec2ee36576381aa42 100644 (file)
--- a/ssl/quic/quic_record_rx.c
+++ b/ssl/quic/quic_record_rx.c
@@ -271,6 +271,19 @@ void ossl_qrx_inject_urxe(OSSL_QRX *qrx, QUIC_URXE *urxe)
                           qrx->msg_callback_arg);
 }
 
+void ossl_qrx_inject_pkt(OSSL_QRX *qrx, OSSL_QRX_PKT *pkt)
+{
+    RXE *rxe = (RXE *)pkt;
+
+    /*
+     * port_default_packet_handler() uses ossl_qrx_read_pkt()
+     * to get pkt. Such packet has refcount 1.
+     */
+    ossl_qrx_pkt_release(pkt);
+    if (ossl_assert(rxe->refcount == 0))
+        ossl_list_rxe_insert_tail(&qrx->rx_pending, rxe);
+}
+
 /*
  * qrx_validate_initial_pkt() is derived from qrx_process_pkt(). Unlike
  * qrx_process_pkt() the qrx_validate_initial_pkt() function can process