QUIC RXDP: Strictly enforce ACK PNs with regard to TX key epochs

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21029)

3 files changed, 48 insertions, 7 deletions

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 017a1ab28f..951160a7f0 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -691,6 +691,7 @@ static void rxku_detected(QUIC_PN pn, void *arg)
     ch->rxku_pending_confirm        = 1;
     ch->rxku_trigger_pn             = pn;
     ch->rxku_update_end_deadline    = ossl_time_add(get_time(ch), pto);
+    ch->rxku_expected               = 0;
 
     if (decision == DECISION_SOLICITED_TXKU)
         /* NOT gated by usual txku_allowed() */
diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h
index c4be7d7922..607f109119 100644
--- a/ssl/quic/quic_channel_local.h
+++ b/ssl/quic/quic_channel_local.h
@@ -228,11 +228,11 @@ struct quic_channel_st {
     OSSL_TIME                       rxku_update_end_deadline;
 
     /*
-     * The first (application space) PN sent with a new key phase. Valid if
-     * txku_in_progress. Once a packet we sent with a PN p (p >= txku_pn) is
-     * ACKed, the TXKU is considered completed and txku_in_progress becomes 0.
-     * For sanity's sake, such a PN p should also be <= the highest PN we have
-     * ever sent, of course.
+     * The first (application space) PN sent with a new key phase. Valid if the
+     * QTX key epoch is greater than 0. Once a packet we sent with a PN p (p >=
+     * txku_pn) is ACKed, the TXKU is considered completed and txku_in_progress
+     * becomes 0. For sanity's sake, such a PN p should also be <= the highest
+     * PN we have ever sent, of course.
      */
     QUIC_PN                         txku_pn;
 
@@ -386,6 +386,11 @@ struct quic_channel_st {
 
     /* Temporary variable indicating rxku_pending_confirm is to become 0. */
     unsigned int                    rxku_pending_confirm_done           : 1;
+
+    /*
+     * If set, RXKU is expected (because we initiated a spontaneous TXKU).
+     */
+    unsigned int                    rxku_expected                       : 1;
 };
 
 # endif
diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
index f22e658e8a..12aae998d9 100644
--- a/ssl/quic/quic_rx_depack.c
+++ b/ssl/quic/quic_rx_depack.c
@@ -60,7 +60,8 @@ static int depack_do_frame_ping(PACKET *pkt, QUIC_CHANNEL *ch,
 
 static int depack_do_frame_ack(PACKET *pkt, QUIC_CHANNEL *ch,
                                int packet_space, OSSL_TIME received,
-                               uint64_t frame_type)
+                               uint64_t frame_type,
+                               OSSL_QRX_PKT *qpacket)
 {
     OSSL_QUIC_FRAME_ACK ack;
     OSSL_QUIC_ACK_RANGE *ack_ranges = NULL;
@@ -80,6 +81,40 @@ static int depack_do_frame_ack(PACKET *pkt, QUIC_CHANNEL *ch,
     if (!ossl_quic_wire_decode_frame_ack(pkt, ack_delay_exp, &ack, NULL))
         goto malformed;
 
+    if (qpacket->hdr->type == QUIC_PKT_TYPE_1RTT
+        && (qpacket->key_epoch < ossl_qrx_get_key_epoch(ch->qrx)
+            || ch->rxku_expected)
+        && ack.ack_ranges[0].end >= ch->txku_pn) {
+        /*
+         * RFC 9001 s. 6.2: An endpoint that receives an acknowledgment that is
+         * carried in a packet protected with old keys where any acknowledged
+         * packet was protected with newer keys MAY treat that as a connection
+         * error of type KEY_UPDATE_ERROR.
+         *
+         * Two cases to handle here:
+         *
+         *   - We did spontaneous TXKU, the peer has responded in kind and we
+         *     have detected RXKU; !ch->rxku_expected, but then it sent a packet
+         *     with old keys acknowledging a packet in the new key epoch.
+         *
+         *     This also covers the case where we got RXKU and triggered
+         *     solicited TXKU, and then for some reason the peer sent an ACK of
+         *     a PN in our new TX key epoch with old keys.
+         *
+         *   - We did spontaneous TXKU; ch->txku_pn is the starting PN of our
+         *     new TX key epoch; the peer has not initiated a solicited TXKU in
+         *     response (so we have not detected RXKU); in this case the RX key
+         *     epoch has not incremented and ch->rxku_expected is still 1.
+         */
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               QUIC_ERR_KEY_UPDATE_ERROR,
+                                               frame_type,
+                                               "acked packet which initiated a "
+                                               "key update without a "
+                                               "corresponding key update");
+        return 0;
+    }
+
     if (!ossl_ackm_on_rx_ack_frame(ch->ackm, &ack,
                                    packet_space, received))
         goto malformed;
@@ -837,7 +872,7 @@ static int depack_process_frames(QUIC_CHANNEL *ch, PACKET *pkt,
                 return 0;
             }
             if (!depack_do_frame_ack(pkt, ch, packet_space, received,
-                                     frame_type))
+                                     frame_type, parent_pkt))
                 return 0;
             break;