From: Alexandr Nedvedicky Date: Tue, 18 Feb 2025 00:34:04 +0000 (+0100) Subject: Fix AEAD validation of initial packets in port X-Git-Tag: openssl-3.5.0-alpha1~85 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96075a6a4061eab8274fc27f5f10959ddae433e5;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/26808) --- diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index b782f0d66b7..07b5c218cae 100644 --- 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 54f3a6321b1..8db7f3fc0dc 100644 --- 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 262962973fe..169d96a8ded 100644 --- 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 db1bd48822e..64ebf1c4d65 100644 --- 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 63aa9aff8fa..29625118ae4 100644 --- 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