Commit b758149cea for openssl.org
commit b758149ceacb622300430afa1e8cd5fa9a0f3a90
Author: Neil Horman <nhorman@openssl.org>
Date: Thu Jul 9 15:34:30 2026 -0400
Don't store ACK-only frames in TX history for QUIC.
When QUIC sends an ACK-only frame, there is no expectation that the
peer will ack that ack (i.e. it is itself not ack-eliciting). However,
our implementation stores these frames in the TX history regardless. In and
of itself thats ok, but if a malicious client establishes a connection,
and then drives the connection such that ack-only frames are forced from
the peer (i.e. by sending numerous ping frames), and then withholding
any subseqent acks for ack-eliciting data, like legitimate data, said
malicious client can force inappropriate memory growth on the server,
leading to potential DOS attacks.
Don't store any ACK-only frames in the TX history to address this. Record it in
our TX history so that the send window moves forward appropriately, but for
ack-only frames, immediately remove it, since we don't expect to get an ack for
them anyway.
Initially authored by Opal Wright <opal.wright@trailofbits.com>
The initial proposal had some shortcommings in which the highest pn
acked value was not accounted for which I have fixed with the assistance
of Claude
Assisted-by: Anthopic Sonnet 5
Fixes CVE-2026-63075
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Bob Beck <beck@openssl.org>
Merge-date: Mon Aug 24 12:07:51 2026
diff --git a/include/internal/quic_ackm.h b/include/internal/quic_ackm.h
index c0617da485..aa402d294e 100644
--- a/include/internal/quic_ackm.h
+++ b/include/internal/quic_ackm.h
@@ -129,6 +129,11 @@ struct ossl_ackm_tx_pkt_st {
};
int ossl_ackm_on_tx_packet(OSSL_ACKM *ackm, OSSL_ACKM_TX_PKT *pkt);
+
+/*
+ * Records transmission of a packet containing only ACK frames.
+ */
+int ossl_ackm_on_tx_ack_only_packet(OSSL_ACKM *ackm, OSSL_ACKM_TX_PKT *pkt);
int ossl_ackm_on_rx_datagram(OSSL_ACKM *ackm, size_t num_bytes);
#define OSSL_ACKM_ECN_NONE 0
diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
index 24acb8635c..b2e5c1f497 100644
--- a/ssl/quic/quic_ackm.c
+++ b/ssl/quic/quic_ackm.c
@@ -1135,6 +1135,38 @@ int ossl_ackm_on_tx_packet(OSSL_ACKM *ackm, OSSL_ACKM_TX_PKT *pkt)
return 1;
}
+int ossl_ackm_on_tx_ack_only_packet(OSSL_ACKM *ackm, OSSL_ACKM_TX_PKT *pkt)
+{
+ struct tx_pkt_history_st *h;
+ unsigned int pkt_space;
+
+ if (pkt == NULL || pkt->pkt_space >= QUIC_PN_SPACE_NUM)
+ return 0;
+
+ /*
+ * A packet containing only an ACK frame must not be treated as
+ * in-flight or ack-eliciting; if it were, ossl_ackm_on_tx_packet()
+ * below would (correctly) perform bytes-in-flight/timer/CC bookkeeping
+ * for a packet we are about to discard from history, which would be
+ * incorrect.
+ */
+ if (pkt->is_inflight || pkt->is_ack_eliciting)
+ return 0;
+
+ pkt_space = pkt->pkt_space;
+
+ /*
+ * No one can expect ACK for packet which carries ACK frames only
+ * (ack_only packet). The ACKM does not need to keep record for ack_only
+ * packet. For ack_only packet the ACKM manager must be updated by the
+ * highest packet number which got sent.
+ */
+ h = get_tx_history(ackm, pkt_space);
+ h->highest_sent = pkt->pkt_num;
+
+ return 1;
+}
+
int ossl_ackm_on_rx_datagram(OSSL_ACKM *ackm, size_t num_bytes)
{
/* No-op on the client. */
diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index bd026af3a4..9058c92a4f 100644
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -2947,6 +2947,20 @@ fatal_err:
return TXP_ERR_INTERNAL;
}
+static int txp_pkt_is_ack_only(const QUIC_TXPIM_PKT *tpkt)
+{
+ return tpkt->had_ack_frame
+ && !tpkt->ackm_pkt.is_inflight
+ && !tpkt->ackm_pkt.is_ack_eliciting
+ && !tpkt->had_handshake_done_frame
+ && !tpkt->had_max_data_frame
+ && !tpkt->had_max_streams_bidi_frame
+ && !tpkt->had_max_streams_uni_frame
+ && !tpkt->had_conn_close
+ && tpkt->retx_head == NULL
+ && ossl_quic_txpim_pkt_get_num_chunks(tpkt) == 0;
+}
+
/*
* Commits and queues a packet for transmission. There is no backing out after
* this.
@@ -2955,8 +2969,9 @@ fatal_err:
*
* - Sends the packet to the QTX for encryption and transmission;
*
- * - Records the packet as having been transmitted in FIFM. ACKM is informed,
- * etc. and the TXPIM record is filed.
+ * - Records non-ACK-only packets as having been transmitted in FIFM. ACKM is
+ * informed, etc. and the TXPIM record is filed only when later callbacks
+ * need it.
*
* - Informs various subsystems of frames that were sent and clears frame
* wanted flags so that we do not generate the same frames again.
@@ -2983,7 +2998,7 @@ static int txp_pkt_commit(OSSL_QUIC_TX_PACKETISER *txp,
uint32_t archetype,
int *txpim_pkt_reffed)
{
- int rc = 1;
+ int ack_only, rc = 1;
uint32_t enc_level = pkt->h.enc_level;
uint32_t pn_space = ossl_quic_enc_level_to_pn_space(enc_level);
QUIC_TXPIM_PKT *tpkt = pkt->tpkt;
@@ -3027,28 +3042,35 @@ static int txp_pkt_commit(OSSL_QUIC_TX_PACKETISER *txp,
return 0; /* alloc error */
}
- /* Dispatch to FIFD. */
- if (!ossl_quic_fifd_pkt_commit(&txp->fifd, tpkt))
+ ack_only = txp_pkt_is_ack_only(tpkt);
+
+ /* Dispatch packets that need loss/retransmit callbacks to FIFD. */
+ if (!ack_only && !ossl_quic_fifd_pkt_commit(&txp->fifd, tpkt))
return 0;
/*
* Transmission and Post-Packet Generation Bookkeeping
* ===================================================
*
- * No backing out anymore - at this point the ACKM has recorded the packet
- * as having been sent, so we need to increment our next PN counter, or
- * the ACKM will complain when we try to record a duplicate packet with
- * the same PN later. At this point actually sending the packet may still
- * fail. In this unlikely event it will simply be handled as though it
- * were a lost packet.
+ * No backing out anymore - at this point we need to increment our next PN
+ * counter, or the ACKM will complain when we try to record a duplicate
+ * packet with the same PN later. Non-ACK-only packets have also been
+ * recorded in ACKM, so if QTX write fails they are handled as though they
+ * were lost. ACK-only packets are not recorded and will be cleaned up by
+ * the caller.
*/
++txp->next_pn[pn_space];
- *txpim_pkt_reffed = 1;
+ if (!ack_only)
+ *txpim_pkt_reffed = 1;
/* Send the packet. */
if (!ossl_qtx_write_pkt(txp->args.qtx, &txpkt))
return 0;
+ if (ack_only
+ && !ossl_ackm_on_tx_ack_only_packet(txp->args.ackm, &tpkt->ackm_pkt))
+ rc = 0;
+
/*
* Record FC and stream abort frames as sent; deactivate streams which no
* longer have anything to do.