litespeedtech / litespeedtech/lsquic
Several RFC Compliance Issues in lsquic
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 1.9k
- Forks
- 397
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 8
Description
Hi, while reviewing lsquic's QUIC implementation against RFC 9000, RFC 9001, and RFC 9002, we noticed several places where the current behavior appears to differ from the specification. Each item below cites the relevant RFC text alongside the corresponding source code for your reference. We hope this is helpful for improving RFC conformance.
1. STOP_SENDING for Stream in Ready State Aborts Connection Rather Than Sending RESET_STREAM
RFC Reference: RFC 9000 Section 3.5
"An endpoint that receives a STOP_SENDING frame MUST send a RESET_STREAM frame if the stream is in the 'Ready' or 'Send' state."
Analysis:
In process_stop_sending_frame(), when a locally-initiated stream is in the SSS_READY sending state (corresponding to RFC 9000's "Ready" state), the code calls ABORT_QUIETLY with TEC_PROTOCOL_VIOLATION and returns immediately (lines 5394-5401). This terminates the entire connection without sending a RESET_STREAM frame for the stream. For streams in the SSS_SEND state, the code falls through to lsquic_stream_stop_sending_in() which does send RESET_STREAM. The current implementation treats STOP_SENDING for a Ready-state stream as a protocol violation, whereas RFC 9000 Section 3.5 lists Ready as one of the two states in which RESET_STREAM MUST be sent.
Source Code Evidence (lsquic_full_conn_ietf.c):
// lsquic_full_conn_ietf.c:5390-5403
our_stream = !is_peer_initiated(conn, stream_id);
stream = find_stream_by_id(conn, stream_id);
if (stream)
{
if (our_stream &&
SSS_READY == (sss = lsquic_stream_sending_state(stream)))
{
ABORT_QUIETLY(0, TEC_PROTOCOL_VIOLATION, "stream %"PRIu64" is in "
"%s state: receipt of STOP_SENDING frame is a violation",
stream_id, lsquic_sss2str[sss]);
return 0;
}
lsquic_stream_stop_sending_in(stream, error_code);
}
2. Invalid Retry Token Does Not Trigger CONNECTION_CLOSE with INVALID_TOKEN
RFC Reference: RFC 9000 Section 8.1.2
"If a server receives a client Initial that contains an invalid Retry token but is otherwise valid, the server SHOULD immediately close the connection with an INVALID_TOKEN error."
Analysis:
When the server receives an Initial packet with an invalid Retry token, lsquic_tg_validate_token() returns non-zero and the code enters the TOKEN_RETRY branch at line 1550-1554, which logs a debug message and returns NULL — silently dropping the packet without sending a CONNECTION_CLOSE frame. The code explicitly acknowledges this deviation in a comment at lines 1556-1561. The error code TEC_INVALID_TOKEN (0xB) is defined in lsquic_ietf.h:21 but is not referenced in any .c file. This is a SHOULD-level requirement rather than MUST.
Source Code Evidence (lsquic_engine.c):
// lsquic_engine.c:1550-1562
if (TOKEN_RETRY == packet_in->pi_data[packet_in->pi_token])
{
LSQ_DEBUGC("CID %"CID_FMT" has invalid Retry token",
CID_BITS(&packet_in->pi_conn_id));
return NULL;
}
/* According to the spec, we SHOULD send CONNECTION_CLOSE
* when receiving an invalid Retry token. We don't do it
* because it's a lot of code change for an event that is
* not likely to happen: a major browser copying the token
* incorrectly.
*/
break;
3. PTO Timer for Application Data Space Armed Before Handshake Confirmation
RFC Reference: RFC 9002 Section 6.2.1
"An endpoint MUST NOT set its PTO timer for the Application Data packet number space until the handshake is confirmed."
Analysis:
In lsquic_send_ctl_sent_packet() (lines 883-886), whenever a retransmittable packet is sent in any packet number space, set_retx_alarm() is called unconditionally if the alarm is not already set, without checking handshake confirmation status (LSCONN_HANDSHAKE_DONE or SC_1RTT_ACKED). set_retx_alarm() (line 637) sets the alarm identically for all three PNS values via AL_RETX_INIT + pns. Because lsquic_enclev2pns[ENC_LEV_0RTT] = PNS_APP (lsquic_packet_common.c:109), 0-RTT data packets sent by a client before handshake completion map to PNS_APP and arm the AL_RETX_APP alarm. The SC_1RTT_ACKED guard is only applied during re-arming after ACK processing (line 1564), not during the initial arming path.
Source Code Evidence (lsquic_send_ctl.c):
// lsquic_send_ctl.c:883-886
if (packet_out->po_frame_types & ctl->sc_retx_frames)
{
if (!lsquic_alarmset_is_set(ctl->sc_alset, AL_RETX_INIT + pns))
set_retx_alarm(ctl, pns, packet_out->po_sent);
...
}
// lsquic_send_ctl.c:637 (inside set_retx_alarm)
lsquic_alarmset_set(ctl->sc_alset, AL_RETX_INIT + pns, now + delay);
4. BBR Congestion Controller Does Not Reduce cwnd on Persistent Congestion
RFC Reference: RFC 9002 Section 7.6.2
"When persistent congestion is declared, the sender's congestion window MUST be reduced to the minimum congestion window (kMinimumWindow)."
Analysis:
The BBR congestion controller's timeout handler lsquic_bbr_timeout() (lines 1068-1069) is an empty no-op function. When a PTO timer expires, retx_alarm_rings() (line 406) calls cci_timeout(), which dispatches to this no-op for BBR, meaning the congestion window is never reduced on persistent congestion. A grep for persistent_congestion across the entire codebase returns zero results — there is no persistent congestion detection logic. The consecutive RTO counter sc_n_consec_rtos is tracked (line 404) but is only used for RTO backoff, not to trigger a kMinimumWindow reduction. The Cubic controller does reduce cu_cwnd to 2 * TCP_MSS on timeout (lsquic_cubic.c:230-231), but this is standard RTO behavior rather than a persistent-congestion-specific response.
Source Code Evidence (lsquic_bbr.c):
// lsquic_bbr.c:1068-1069
static void
lsquic_bbr_timeout (void *cong_ctl) { /* Noop */ }
Source Code Evidence (lsquic_send_ctl.c):
// lsquic_send_ctl.c:399-412
if (ctl->sc_n_consec_rtos < MAX_CONSEC_RTOs)
{
++ctl->sc_n_consec_rtos;
ctl->sc_ci->cci_timeout(CGP(ctl)); // calls lsquic_bbr_timeout (no-op)
}
5. RTT Sample Generated from ACK Without Verifying Ack-Eliciting Flag
RFC Reference: RFC 9002 Section 5.1
"An RTT sample MUST NOT be generated on receiving an ACK frame that does not newly acknowledge at least one ack-eliciting packet."
Analysis:
In lsquic_send_ctl_got_ack(), the do_rtt flag is set based solely on whether the newly acknowledged packet's number matches largest_acked(acki) (line 1539), without checking whether that packet contains ack-eliciting frames (i.e., po_frame_types & ctl->sc_retx_frames). The same pattern exists in send_ctl_acked_loss_chain() at line 1090-1091. The IQUIC_FRAME_ACKABLE_MASK defined in lsquic_packet_common.h:210-213 identifies ack-eliciting frame types but is not referenced in the RTT sampling path. Consequently, an ACK frame that newly acknowledges only non-ack-eliciting packets (such as pure ACK or PADDING-only packets) whose packet number happens to be the largest acked would still trigger an RTT sample via take_rtt_sample() (line 1553).
Source Code Evidence (lsquic_send_ctl.c):
// lsquic_send_ctl.c:1539
do_rtt |= packet_out->po_packno == largest_acked(acki);
// lsquic_send_ctl.c:1090-1091 (send_ctl_acked_loss_chain)
if (chain_cur->po_packno == largest_acked)
*do_rtt = 1;
// lsquic_send_ctl.c:1551-1553
if (do_rtt)
{
take_rtt_sample(ctl, ack_recv_time, acki->lack_delta);
6. Transport Parameter Validation Failure in Full Connection Path Sends TEC_INTERNAL_ERROR
RFC Reference: RFC 9000 Section 7.4
"An endpoint MUST treat receipt of a transport parameter with an invalid value as a connection error of type TRANSPORT_PARAMETER_ERROR."
Analysis:
When handshake_ok() fails to retrieve peer transport parameters (returning NULL), it calls ABORT_WARN("could not get transport parameters") at line 3802. The ABORT_WARN macro (lines 251-252) sets only the IFC_ERROR flag and does not set conn->ifc_error.u.err. When the CONNECTION_CLOSE frame is generated (lines 4081-4088), the code checks conn->ifc_error.u.err != 0 (false in this case), then falls through to else if (conn->ifc_flags & IFC_ERROR) and defaults to CONN_ERR(0, TEC_INTERNAL_ERROR) (0x1) instead of TEC_TRANSPORT_PARAMETER_ERROR (0x8). The mini connection path correctly uses TEC_TRANSPORT_PARAMETER_ERROR at lsquic_mini_conn_ietf.c:2174. The connection is still terminated, but the error code sent on the wire differs from the value specified by RFC 9000 Section 7.4.
Source Code Evidence (lsquic_full_conn_ietf.c):
// lsquic_full_conn_ietf.c:3802
ABORT_WARN("could not get transport parameters");
// lsquic_full_conn_ietf.c:4081-4088
if (conn->ifc_error.u.err != 0)
{
conn_err = conn->ifc_error;
error_reason = conn->ifc_errmsg;
}
else if (conn->ifc_flags & IFC_ERROR)
{
conn_err = CONN_ERR(0, TEC_INTERNAL_ERROR); // defaults to 0x1
error_reason = "connection error";
}
7. CRYPTO Frame in 0-RTT Packet Uses TEC_FRAME_ENCODING_ERROR in Full Connection Path
RFC Reference: RFC 9001 Section 8.3
"A server MUST treat receipt of a CRYPTO frame in a 0-RTT packet as a connection error of type PROTOCOL_VIOLATION."
Analysis:
In the full connection's process_packet_frame() (lines 6697-6708), any frame type not in lsquic_legal_frames_by_level for the current encryption level is aborted with TEC_FRAME_ENCODING_ERROR (line 6708). Since QUIC_FTBIT_CRYPTO is not in the legal set for ENC_LEV_0RTT, a CRYPTO frame in a 0-RTT packet triggers TEC_FRAME_ENCODING_ERROR (0x7) rather than TEC_PROTOCOL_VIOLATION (0xA) required by RFC 9001 Section 8.3. The mini connection path (lsquic_mini_conn_ietf.c:1429-1436) distinguishes between QUIC_FRAME_INVALID (→ FRAME_ENCODING_ERROR) and other illegal frames (→ PROTOCOL_VIOLATION), producing the RFC-specified error code. The two code paths use inconsistent error codes for the same protocol violation.
Source Code Evidence (lsquic_full_conn_ietf.c):
// lsquic_full_conn_ietf.c:6697-6708
if (lsquic_legal_frames_by_level[conn->ifc_conn.cn_version][enc_level]
& (1 << type))
{
...
return process_frames[type](conn, packet_in, p, len);
}
else
{
...
ABORT_QUIETLY(0, TEC_FRAME_ENCODING_ERROR, "invalid frame");
return 0;
}
Source Code Evidence (lsquic_mini_conn_ietf.c):
// lsquic_mini_conn_ietf.c:1429-1436
if (type == QUIC_FRAME_INVALID)
ietf_mini_conn_ci_abort_error(&conn->imc_conn, 0,
TEC_FRAME_ENCODING_ERROR, ...);
else
ietf_mini_conn_ci_abort_error(&conn->imc_conn, 0,
TEC_PROTOCOL_VIOLATION, ...);
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by separating the seven findings and reading the cited RFC sections alongside the named entry points: process_stop_sending_frame(), lsquic_tg_validate_token(), lsquic_send_ctl_sent_packet(), lsquic_bbr_timeout(), lsquic_send_ctl_got_ack(), handshake_ok(), and process_packet_frame(). Compare each full-connection path with the referenced mini-connection behavior where provided. Done means each reported case follows its cited RFC requirement and the inconsistent error or timer behavior is covered by verification.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100