Packets with No Frames Are Accepted Instead of Rejected · Issue #5186 · microsoft/msquic

Description
RFC 9000 §12.4 mandates treating a packet that contains no frames as a connection error of type PROTOCOL_VIOLATION.

However, in QuicConnRecvFrames, if the payload is empty, the while loop (line 4444) is not entered, and the packet is processed without error. We also didn't find any relevant checking in the caller of QuicConnRecvFrames.

msquic/src/core/connection.c

Lines 4409 to 4500 in 31d2b73

    
           QuicConnRecvFrames( 
        
               _In_ QUIC_CONNECTION* Connection, 
        
               _In_ QUIC_PATH* Path, 
        
               _In_ QUIC_RX_PACKET* Packet, 
        
               _In_ CXPLAT_ECN_TYPE ECN 
        
               ) 
        
           { 
        
               BOOLEAN AckEliciting = FALSE; 
        
               BOOLEAN AckImmediately = FALSE; 
        
               BOOLEAN UpdatedFlowControl = FALSE; 
        
               QUIC_ENCRYPT_LEVEL EncryptLevel = QuicKeyTypeToEncryptLevel(Packet->KeyType); 
        
               BOOLEAN Closed = Connection->State.ClosedLocally || Connection->State.ClosedRemotely; 
        
               const uint8_t* Payload = Packet->AvailBuffer + Packet->HeaderLength; 
        
               uint16_t PayloadLength = Packet->PayloadLength; 
        
               uint64_t RecvTime = CxPlatTimeUs64(); 
        
               // 
        
               // In closing state, respond to any packet with a new close frame (rate-limited). 
        
               // 
        
               if (Closed && !Connection->State.ShutdownComplete) { 
        
                   if (RecvTime - Connection->LastCloseResponseTimeUs >= QUIC_CLOSING_RESPONSE_MIN_INTERVAL) { 
        
                       QuicSendSetSendFlag( 
        
                           &Connection->Send, 
        
                           Connection->State.AppClosed ? 
        
                               QUIC_CONN_SEND_FLAG_APPLICATION_CLOSE : 
        
                               QUIC_CONN_SEND_FLAG_CONNECTION_CLOSE); 
        
                   } 
        
               } 
        
               if (QuicConnIsClient(Connection) && 
        
                   !Connection->State.GotFirstServerResponse) { 
        
                   Connection->State.GotFirstServerResponse = TRUE; 
        
               } 
        
               uint16_t Offset = 0; 
        
               while (Offset < PayloadLength) { 
        
                   // 
        
                   // Read the frame type. 
        
                   // 
        
                   QUIC_VAR_INT FrameType INIT_NO_SAL(0); 
        
                   if (!QuicVarIntDecode(PayloadLength, Payload, &Offset, &FrameType)) { 
        
                       QuicTraceEvent( 
        
                           ConnError, 
        
                           "[conn][%p] ERROR, %s.", 
        
                           Connection, 
        
                           "Frame type decode failure"); 
        
                       QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR); 
        
                       return FALSE; 
        
                   } 
        
                   if (!QUIC_FRAME_IS_KNOWN(FrameType)) { 
        
                       QuicTraceEvent( 
        
                           ConnError, 
        
                           "[conn][%p] ERROR, %s.", 
        
                           Connection, 
        
                           "Unknown frame type"); 
        
                       QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR); 
        
                       return FALSE; 
        
                   } 
        
                   // 
        
                   // Validate allowable frames based on the packet type. 
        
                   // 
        
                   if (EncryptLevel != QUIC_ENCRYPT_LEVEL_1_RTT) { 
        
                       switch (FrameType) { 
        
                       // 
        
                       // The following frames are allowed pre-1-RTT encryption level: 
        
                       // 
        
                       case QUIC_FRAME_PADDING: 
        
                       case QUIC_FRAME_PING: 
        
                       case QUIC_FRAME_ACK: 
        
                       case QUIC_FRAME_ACK_1: 
        
                       case QUIC_FRAME_CRYPTO: 
        
                       case QUIC_FRAME_CONNECTION_CLOSE: 
        
                           break; 
        
                       // 
        
                       // All other frame types are disallowed. 
        
                       // 
        
                       default: 
        
                           QuicTraceEvent( 
        
                               ConnErrorStatus, 
        
                               "[conn][%p] ERROR, %u, %s.", 
        
                               Connection, 
        
                               (uint32_t)FrameType, 
        
                               "Disallowed frame type"); 
        
                           QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR); 
        
                           return FALSE; 
        
                       } 
        
                   } else if (Packet->KeyType == QUIC_PACKET_KEY_0_RTT) { 
        
                       switch (FrameType) {

Suggested Fix

At the very start of QuicConnRecvFrames, after PayloadLength is known, add:

if (PayloadLength == 0) {
    QuicTraceEvent(
        ConnError,
        "[conn][%p] ERROR, %s.",
        Connection,
        "Packet contained no frames");
    QuicConnTransportError(Connection, QUIC_ERROR_PROTOCOL_VIOLATION);
    ... 
    return FALSE;
}

This ensures packets with no frames are rejected with the required PROTOCOL_VIOLATION error, as required by RFC 9000.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Packets with No Frames Are Accepted Instead of Rejected #5186

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	QuicConnRecvFrames(
	_In_ QUIC_CONNECTION* Connection,
	_In_ QUIC_PATH* Path,
	_In_ QUIC_RX_PACKET* Packet,
	_In_ CXPLAT_ECN_TYPE ECN
	)
	{
	BOOLEAN AckEliciting = FALSE;
	BOOLEAN AckImmediately = FALSE;
	BOOLEAN UpdatedFlowControl = FALSE;
	QUIC_ENCRYPT_LEVEL EncryptLevel = QuicKeyTypeToEncryptLevel(Packet->KeyType);
	BOOLEAN Closed = Connection->State.ClosedLocally \|\| Connection->State.ClosedRemotely;
	const uint8_t* Payload = Packet->AvailBuffer + Packet->HeaderLength;
	uint16_t PayloadLength = Packet->PayloadLength;
	uint64_t RecvTime = CxPlatTimeUs64();

	//
	// In closing state, respond to any packet with a new close frame (rate-limited).
	//
	if (Closed && !Connection->State.ShutdownComplete) {
	if (RecvTime - Connection->LastCloseResponseTimeUs >= QUIC_CLOSING_RESPONSE_MIN_INTERVAL) {
	QuicSendSetSendFlag(
	&Connection->Send,
	Connection->State.AppClosed ?
	QUIC_CONN_SEND_FLAG_APPLICATION_CLOSE :
	QUIC_CONN_SEND_FLAG_CONNECTION_CLOSE);
	}
	}

	if (QuicConnIsClient(Connection) &&
	!Connection->State.GotFirstServerResponse) {
	Connection->State.GotFirstServerResponse = TRUE;
	}

	uint16_t Offset = 0;
	while (Offset < PayloadLength) {

	//
	// Read the frame type.
	//
	QUIC_VAR_INT FrameType INIT_NO_SAL(0);
	if (!QuicVarIntDecode(PayloadLength, Payload, &Offset, &FrameType)) {
	QuicTraceEvent(
	ConnError,
	"[conn][%p] ERROR, %s.",
	Connection,
	"Frame type decode failure");
	QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR);
	return FALSE;
	}

	if (!QUIC_FRAME_IS_KNOWN(FrameType)) {
	QuicTraceEvent(
	ConnError,
	"[conn][%p] ERROR, %s.",
	Connection,
	"Unknown frame type");
	QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR);
	return FALSE;
	}

	//
	// Validate allowable frames based on the packet type.
	//
	if (EncryptLevel != QUIC_ENCRYPT_LEVEL_1_RTT) {
	switch (FrameType) {
	//
	// The following frames are allowed pre-1-RTT encryption level:
	//
	case QUIC_FRAME_PADDING:
	case QUIC_FRAME_PING:
	case QUIC_FRAME_ACK:
	case QUIC_FRAME_ACK_1:
	case QUIC_FRAME_CRYPTO:
	case QUIC_FRAME_CONNECTION_CLOSE:
	break;
	//
	// All other frame types are disallowed.
	//
	default:
	QuicTraceEvent(
	ConnErrorStatus,
	"[conn][%p] ERROR, %u, %s.",
	Connection,
	(uint32_t)FrameType,
	"Disallowed frame type");
	QuicConnTransportError(Connection, QUIC_ERROR_FRAME_ENCODING_ERROR);
	return FALSE;
	}

	} else if (Packet->KeyType == QUIC_PACKET_KEY_0_RTT) {
	switch (FrameType) {

Packets with No Frames Are Accepted Instead of Rejected #5186

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions