quic: improve corruption detection

Author: lws-team

lib/roles/quic/ops-quic.c

@@ -1176,6 +1176,30 @@ rops_handle_POLLIN_quic(struct lws_context_per_thread *pt, struct lws *wsi,
 			n--;
 			continue;
 		}
+
+		/*
+		 * Fixed bit (RFC 9000 §17.2 & §17.3).  This is the one bit in
+		 * the first byte that is NOT covered by header protection (the
+		 * HP mask only touches the low nibble of a long header and the
+		 * low 5 bits of a short header), so we can check it here, before
+		 * trusting any field derived from this byte and before AEAD.
+		 *
+		 * A compliant sender always sets it.  A cleared Fixed bit means
+		 * the byte is corrupt or not a QUIC packet; since every field we
+		 * would parse next (form, type, version, CID lengths) is derived
+		 * from this byte, we cannot trust the length field to find the
+		 * next coalesced packet boundary either, so drop the rest of the
+		 * datagram rather than advance blindly.
+		 *
+		 * The reserved bits (0x0c long / 0x18 short) are header-protected
+		 * and so cannot be checked until after lws_quic_unmask_header();
+		 * the existing post-AEAD check still handles those.
+		 */
+		if (!(p[0] & 0x40)) {
+			lwsl_wsi_notice(wsi, "QUIC RX: Fixed bit not set (0x%02x), "
+					      "dropping corrupt datagram", p[0]);
+			break;
+		}
 		if (p[0] & 0x80) {
 			                        uint8_t type = (uint8_t)((p[0] & 0x30) >> 4);
                         uint32_t parsed_pkt_version = (uint32_t)((p[1] << 24) | (p[2] << 16) | (p[3] << 8) | p[4]);
@@ -1318,8 +1342,18 @@ rops_handle_POLLIN_quic(struct lws_context_per_thread *pt, struct lws *wsi,
 
 		if (!(p[0] & 0x80)) { /* Short header */
 			uint8_t kp = (p[0] & 0x04) >> 2;
-			if (nwsi->quic.qn && nwsi->quic.qn->handshake_done && nwsi->quic.qn->rx_key_phase != kp) {
-				/* Provisional key update */
+			if (nwsi->quic.qn && nwsi->quic.qn->handshake_done &&
+			    nwsi->quic.qn->rx_key_phase != kp &&
+			    nwsi->quic.qn->kp_probe_fail < LWS_QUIC_KP_PROBE_LIMIT) {
+				/*
+				 * Provisional key update.  The key-phase bit is
+				 * only 1 bit and is trivially flipped by wire
+				 * corruption, so this probe can fire on junk.
+				 * If it then fails AEAD we count it below and,
+				 * once kp_probe_fail saturates, we stop trusting
+				 * the bit entirely and treat mismatches as
+				 * corruption rather than a peer rotation.
+				 */
 				scratch_keys = *k;
 				scratch_keys.aead_rx = NULL;
 				scratch_keys.aead_tx = NULL;
@@ -1336,9 +1370,45 @@ rops_handle_POLLIN_quic(struct lws_context_per_thread *pt, struct lws *wsi,
 		if (dec_len < 0) {
 			lwsl_wsi_notice(wsi, "QUIC RX: AEAD Decryption failed (bad tag or truncated)");
 			if (is_key_update) {
+				/*
+				 * The key-phase bit mismatched and the rotated
+				 * keys still failed AEAD: that mismatch was
+				 * corruption, not a peer key update.  Account
+				 * it so we stop chasing key rotations on junk
+				 * once it becomes a pattern.
+				 */
 				lws_quic_keys_release_aead_rx(&scratch_keys);
 				lws_quic_keys_release_aead_tx(&scratch_keys);
+				if (nwsi->quic.qn)
+					nwsi->quic.qn->kp_probe_fail++;
+			}
+
+			/*
+			 * Corruption circuit-breaker.  A single undecryptable
+			 * packet is normal (loss, a stale PN, a probe for a key
+			 * phase we discarded).  A sustained run with no progress
+			 * is wire/path corruption that the loss machinery cannot
+			 * recover from at this rate: rather than spin key
+			 * derivation and stall until PTO exhaustion, close the
+			 * connection cleanly with NO_VIABLE_PATH so the peer and
+			 * any test harness see a decisive transport error.
+			 */
+			if (nwsi && nwsi->quic.qn) {
+				if (++nwsi->quic.qn->consec_decrypt_fail[pn_space] >=
+				    LWS_QUIC_DECRYPT_FAIL_LIMIT) {
+					lwsl_wsi_warn(wsi,
+						"QUIC RX: %u consecutive decrypt "
+						"failures in pn_space %d with no "
+						"progress; path is corrupted, "
+						"closing",
+						nwsi->quic.qn->consec_decrypt_fail[pn_space],
+						pn_space);
+					lws_quic_enter_closing_state(nwsi,
+						LWS_QUIC_ERR_NO_VIABLE_PATH, 0, 0);
+					return LWS_HPI_RET_PLEASE_CLOSE_ME;
+				}
 			}
+
 			/* F-130: Stateless Reset Token detection */
 			if (nwsi && nwsi->quic.qn && (size_t)n >= 21) {
 				static const uint8_t zero_token[16] = {0};
@@ -1427,6 +1497,15 @@ rops_handle_POLLIN_quic(struct lws_context_per_thread *pt, struct lws *wsi,
 		/* Check for duplicate/replayed packet numbers (Security Fix) */
 		int is_out_of_order = 0;
 		if (nwsi->quic.qn) {
+			/*
+			 * Reaching here means the packet authenticated cleanly,
+			 * so any prior decrypt-failure run for this pn_space is
+			 * over: reset the corruption circuit-breaker and the
+			 * key-phase-probe accounting.
+			 */
+			nwsi->quic.qn->consec_decrypt_fail[pn_space] = 0;
+			nwsi->quic.qn->kp_probe_fail = 0;
+
 			uint64_t highest = nwsi->quic.qn->highest_rx_pn[pn_space];
 			if ((nwsi->quic.qn->rx_pn_bitmask[pn_space] != 0 || highest != 0) && full_pn <= highest) {
 				uint64_t diff = highest - full_pn;
@@ -1439,6 +1518,29 @@ rops_handle_POLLIN_quic(struct lws_context_per_thread *pt, struct lws *wsi,
 			} else {
 				if (nwsi->quic.qn->rx_pn_bitmask[pn_space] != 0 || highest != 0) {
 					uint64_t diff = full_pn - highest;
+					/*
+					 * Forward-PN plausibility guard.  A
+					 * corrupted truncated PN decodes to an
+					 * arbitrary value, and a huge forward
+					 * jump would rescale the receive
+					 * bitmask and bump highest_rx_pn past
+					 * thousands of imaginary gaps,
+					 * poisoning ACK and loss accounting.
+					 * QUIC senders never gap PNs by this
+					 * much in practice, so drop the packet
+					 * rather than trust the decode.
+					 */
+					if (diff > LWS_QUIC_PN_FORWARD_JUMP_LIMIT) {
+						lwsl_wsi_notice(wsi,
+							"QUIC RX: decoded PN %llu "
+							"jumps %llu ahead of "
+							"highest %llu; treating "
+							"as corrupted truncated PN",
+							(unsigned long long)full_pn,
+							(unsigned long long)diff,
+							(unsigned long long)highest);
+						goto next_packet;
+					}
 					if (diff >= 64)
 						nwsi->quic.qn->rx_pn_bitmask[pn_space] = 0;
 					else

lib/roles/quic/private-lib-roles-quic.h

@@ -144,6 +144,32 @@ enum lws_quic_frame_type {
 #define LWS_QUIC_ERR_AEAD_LIMIT_REACHED       0x0f
 #define LWS_QUIC_ERR_NO_VIABLE_PATH           0x10
 
+/*
+ * Corruption-defense thresholds.
+ *
+ * LWS_QUIC_DECRYPT_FAIL_LIMIT: how many consecutive undecryptable packets in
+ *	a pn_space (with no successful decrypt in between) we tolerate before
+ *	declaring the path corrupted and closing with NO_VIABLE_PATH.  This is
+ *	deliberately generous: a burst of losses or coalesced junk from a
+ *	pmtud probe can produce several, and the PTO/loss machinery already
+ *	recovers genuine loss.  We only act on a sustained run that real loss
+ *	could not produce at this rate.
+ *
+ * LWS_QUIC_KP_PROBE_LIMIT: how many times we let a demasked key-phase
+ *	mismatch drive a provisional key update that then fails AEAD, before
+ *	we stop trusting the key-phase bit as a rotation signal.  Past this,
+ *	a KP mismatch is treated as corruption, not a peer-initiated update.
+ *
+ * LWS_QUIC_PN_FORWARD_JUMP_LIMIT: largest acceptable jump of a freshly
+ *	decoded packet number ahead of highest_rx_pn.  QUIC senders do not gap
+ *	PNs by hundreds for no reason; a decode that lands far ahead is almost
+ *	always a corrupted truncated PN, and committing it would wreck the
+ *	receive bitmask and ACK accounting.  We drop the packet instead.
+ */
+#define LWS_QUIC_DECRYPT_FAIL_LIMIT	256u
+#define LWS_QUIC_KP_PROBE_LIMIT		8u
+#define LWS_QUIC_PN_FORWARD_JUMP_LIMIT	1024u
+
 /*
  * A logical frame queued for transmission or in-flight waiting for ACK.
  */
@@ -266,6 +292,27 @@ struct lws_quic_netconn {
 	uint64_t		rx_pn_bitmask[LWS_QUIC_LEVEL_COUNT];
 	uint8_t			needs_ack[LWS_QUIC_LEVEL_COUNT];
 
+	/*
+	 * Corruption defense (inbound sanity self-checks).
+	 *
+	 * If a packet passes header protection but fails AEAD, it is either a
+	 * legitimate loss (the packet belongs to a key phase we no longer have,
+	 * or it is genuinely lost in flight) or it is wire/path corruption that
+	 * happened to look plausible enough to reach decryption.  We cannot tell
+	 * the two apart from a single failure, but a sustained run of failures
+	 * with no progress is corruption until proven otherwise.
+	 *
+	 * consec_decrypt_fail[] counts undecryptable packets per pn_space since
+	 * the last packet that decrypted cleanly; it is reset on every success.
+	 *
+	 * kp_probe_fail counts provisional key-update attempts (a short-header
+	 * packet whose demasked key-phase bit differs from rx_key_phase) that
+	 * subsequently failed AEAD.  A single bit flip in junk flips this, so we
+	 * refuse to keep re-deriving keys once it's clear the bit is just noise.
+	 */
+	uint32_t		consec_decrypt_fail[LWS_QUIC_LEVEL_COUNT];
+	uint32_t		kp_probe_fail;
+
 	/* RX Crypto Reassembly Buffers (Streams are handled by child WSIs) */
 	uint64_t		rx_crypto_offset[LWS_QUIC_LEVEL_COUNT];
 	lws_dll2_owner_t	rx_crypto_chunks[LWS_QUIC_LEVEL_COUNT];