quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

Stream ID is 0 for unistream `Stream::Readable` event. #1279

Closed TimonPost closed 2 years ago

TimonPost commented 2 years ago

I have some bug where I get a Stream Readable event from quic-proto but its stream id is 0 for unidirectional streams.

The Frame::Stream(frame) event in process_payload its stream id is 0. This triggers the event StreamReadable in StreamState::on_stream_frame with stream id 0.

Both on the server and on the client this stream id is 0. Whilst when I inspect the wireshark server capture the stream id is 2! As is expected for a uni stream.

Client

2022-01-03T15:25:13.337011Z TRACE quinn_proto::connection::streams: wrote 6 bytes stream=client unidirectional stream 0
2022-01-03T15:25:13.338893Z TRACE drive{id=0}:send{space=Data pn=12}: quinn_proto::connection::streams::state: STREAM id=client unidirectional stream 0 off=54 len=6 fin=false

Server

2022-01-03T15:25:13.340610Z TRACE drive{id=0}:recv{space=Data pn=12}: quinn_proto::connection: got stream frame id=client unidirectional stream 0 offset=54 len=6 fin=false

I added a trace log to process_payload and StreamState::on_stream_frame (which causes the readable stream event) to validate if this id is 0 there as well.

Server

2022-01-03T15:35:48.358698Z TRACE drive{id=0}:recv{space=Data pn=11}: quinn_proto::connection: got stream frame id=client unidirectional stream 0 offset=48 len=6 fin=false
2022-01-03T15:35:48.361942Z DEBUG drive{id=0}:recv{space=Data pn=11}:frame{ty=STREAM}: quinn_proto::connection: Connection::process_payload: stream id client unidirectional stream 0

Client

2022-01-03T15:35:47.850625Z TRACE drive{id=0}:send{space=Data pn=10}: quinn_proto::connection::streams::state: STREAM id=client unidirectional stream 0 off=42 len=6 fin=false

But.... Looking at the Wireshark snapshot

image

The stream has the id of 2! Any idea what's going on here? So summarized: The payload has the correct stream id while the payload parsing code doesn't seem to get it right.

Minimal code that reproduces the bug

Click to expand! ## **Server** ```rust mod common; use common::{make_client_endpoint, make_server_endpoint}; use rustls::{Certificate, PrivateKey, RootCertStore}; use std::{fs, thread}; use rustls::internal::msgs::codec::Codec; use std::sync::Arc; use quinn::{ClientConfig, Endpoint, ServerConfig}; use rustls::client::{ServerCertVerified, ServerCertVerifier}; use std::time::Duration; use std::net::SocketAddr; pub fn generate_self_signed_cert() -> (Certificate, PrivateKey) { let certificate = rcgen::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); let serialized_key = certificate.serialize_private_key_der(); let serialized_certificate = certificate.serialize_der().unwrap(); fs::write(&"cert.der", &serialized_certificate).expect("failed to write certificate"); fs::write(&"key.der", &serialized_key).expect("failed to write private key"); (Certificate(serialized_certificate), PrivateKey(serialized_key)) } fn default_server_config() -> ServerConfig { let (cert, key) = generate_self_signed_cert(); let mut store = RootCertStore::empty(); store.add(&cert); let config = ServerConfig::with_single_cert(vec![cert], key) .unwrap(); config } #[tokio::main] async fn main() -> Result<(), Box> { tracing::subscriber::set_global_default( tracing_subscriber::FmtSubscriber::builder() .with_env_filter("trace") .finish(), ) .unwrap(); let server_config = default_server_config(); let (endpoint, mut incoming) = Endpoint::server(server_config, "127.0.0.1:5000".parse().unwrap()).unwrap(); println!("Listening"); while let Some(conn) = incoming.next().await { println!("Incoming Connection"); let quinn::NewConnection { connection, mut bi_streams, .. } = conn.await?; println!("Connected: addr={}", connection.remote_address()); let (send, mut recv) = bi_streams.next().await.unwrap().unwrap(); loop { let mut buffer = [0u8; 1024]; recv.read(&mut buffer).await; println!("Data Received {:?}", buffer); } } Ok(()) } ``` ## Client ```rust //! This example intends to use the smallest amount of code to make a simple QUIC connection. //! //! Checkout the `README.md` for guidance. mod common; use common::{make_client_endpoint, make_server_endpoint}; use rustls::{Certificate, PrivateKey, RootCertStore, KeyLogFile}; use std::{fs, thread}; use rustls::internal::msgs::codec::Codec; use std::sync::Arc; use quinn::{ClientConfig, Endpoint}; use rustls::client::{ServerCertVerified, ServerCertVerifier}; use std::time::Duration; pub fn read_cert_from_file() -> (Certificate, PrivateKey) { let (cert, key) = fs::read(&"cert.der").and_then(|x| Ok((x, fs::read(&"key.der").unwrap()))).unwrap(); let cert = Certificate(cert); let key = PrivateKey(key); (cert, key) } #[tokio::main] async fn main() -> Result<(), Box> { let (cert, key) = read_cert_from_file(); tracing::subscriber::set_global_default( tracing_subscriber::FmtSubscriber::builder() .with_env_filter("trace") .finish(), ) .unwrap(); let server_addr = "127.0.0.1:5000".parse().unwrap(); let mut store = RootCertStore::empty(); store.add(&cert); let mut endpoint = Endpoint::client("127.0.0.1:5001".parse().unwrap()).unwrap(); let mut crypto = rustls::ClientConfig::builder() .with_safe_defaults() .with_custom_certificate_verifier(SkipServerVerification::new()) .with_no_client_auth(); crypto.key_log = Arc::new(KeyLogFile::new()); endpoint.set_default_client_config(ClientConfig::new(Arc::new(crypto))); // connect to server let quinn::NewConnection { connection, mut uni_streams, mut bi_streams, .. } = endpoint .connect(server_addr, "localhost") .unwrap() .await .unwrap(); println!("[client] connected: addr={}", connection.remote_address()); let mut uni = connection.open_uni().await.unwrap(); let mut count = 0; loop { uni.write(format!("test {0}", count).as_bytes()).await.unwrap(); thread::sleep(Duration::from_millis(500)); count +=1; } Ok(()) } struct SkipServerVerification; impl SkipServerVerification { fn new() -> Arc { Arc::new(Self) } } impl ServerCertVerifier for SkipServerVerification { fn verify_server_cert( &self, _end_entity: &rustls::Certificate, _intermediates: &[rustls::Certificate], _server_name: &rustls::ServerName, _scts: &mut dyn Iterator, _ocsp_response: &[u8], _now: std::time::SystemTime, ) -> Result { Ok(ServerCertVerified::assertion()) } } ```
Ralith commented 2 years ago

client unidirectional stream 0

The 0 here is a stream index. This is the human-readable decoding of a stream ID, which encodes the initiator, the directionality, and the index. See RFC 9000·§2.1.

TimonPost commented 2 years ago

Thanks! Now it makes sense. This is exactly what was wrong with my implementation.