Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate RETIRE_CONNECTION_ID frame leads to connection close with PROTOCOL VIOLATION #1833

Open
jmuecke opened this issue Aug 20, 2024 · 0 comments

Comments

@jmuecke
Copy link

jmuecke commented Aug 20, 2024

I encountered an issue while using quiche as QUIC client to contact a quic-go server.
If the server retires the same connection ID twice and no more than one connection ID is available, quiche closes the connection with a protocol violation error.

Attached is a pcap file demonstrating the described behavior.
The duplicate RETIRE_CONNECTION_ID frames are in Wireshark No. 17 and 18.
CONNECTION_CLOSE is in the next packet (No. 19).

Expected behavior:
The client should not close the connection.
Instead, it should first verify if the RETIRE_CONNECTION_ID frame references an active connection ID. If it does, the client should then check whether the connection must be closed due to no other connection IDs being available after retirement.

Receiving the same RETIRE_CONNECTION_ID frame multiple times is expected behavior according to RFC9000:

New connection IDs are sent in NEW_CONNECTION_ID frames and retransmitted if the packet containing them is lost. Retransmissions of this frame carry the same sequence number value. Likewise, retired connection IDs are sent in RETIRE_CONNECTION_ID frames and retransmitted if the packet containing them is lost.

I think the respective code is here:

quiche/quiche/src/cid.rs

Lines 193 to 202 in 0ba4a74

if self.inner.len() <= 1 {
return Err(Error::OutOfIdentifiers);
}
Ok(self
.inner
.iter()
.position(|e| e.seq == seq)
.and_then(|index| self.inner.remove(index)))
}

The issue seems to be that inner.len() is checked before verifying that the contained sequence number is actually in the list.

quiche-client-duplicate-retire-connection-id.pcapng.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@jmuecke and others