quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.57k stars 364 forks source link

Connection close during handshake by server is not transmitted to client properly #1766

Closed zdave-parity closed 4 months ago

zdave-parity commented 4 months ago

You can see this if you edit the connection example to simply drop the incoming connection before completing the handshake:

let endpoint2 = endpoint.clone();
tokio::spawn(async move {
    let incoming_conn = endpoint2.accept().await.unwrap();
    //let conn = incoming_conn.await.unwrap();
    //println!(
    //    "[server] connection accepted: addr={}",
    //    conn.remote_address()
    //);

The server sends a CONNECTION_CLOSE frame, but the client doesn't appear to recognise it:

[2024-02-24T18:38:31Z TRACE quinn_proto::endpoint] connection incoming id=0 icid=17e122e488db138f188f6a4088bc5b50a9deb80b
[2024-02-24T18:38:31Z TRACE quinn_proto::connection] connection closed
[2024-02-24T18:38:31Z DEBUG quinn::connection] drive; id=0
[2024-02-24T18:38:31Z TRACE tracing::span::active] -> drive;
[2024-02-24T18:38:31Z TRACE quinn_proto::connection::packet_builder] send; space=Data pn=0
[2024-02-24T18:38:31Z TRACE quinn_proto::connection] sending CONNECTION_CLOSE
[2024-02-24T18:38:31Z TRACE quinn_proto::connection::packet_builder] PADDING * 1
[2024-02-24T18:38:31Z TRACE tracing::span] -- send;
[2024-02-24T18:38:31Z TRACE quinn_proto::connection] sending 30 bytes in 1 datagrams
[2024-02-24T18:38:31Z TRACE tracing::span::active] <- drive;
[2024-02-24T18:38:31Z TRACE tracing::span] -- drive;
[2024-02-24T18:38:31Z DEBUG quinn::connection] drive; id=0
[2024-02-24T18:38:31Z TRACE tracing::span::active] -> drive;
[2024-02-24T18:38:31Z TRACE tracing::span::active] <- drive;
[2024-02-24T18:38:31Z TRACE tracing::span] -- drive;
[2024-02-24T18:38:31Z DEBUG quinn::connection] drive; id=0
[2024-02-24T18:38:31Z TRACE tracing::span::active] -> drive;
[2024-02-24T18:38:31Z DEBUG quinn_proto::connection::packet_crypto] discarding unexpected Data packet (30 bytes)

I believe what's happening is something like:

Ralith commented 4 months ago

Thanks for the report! I agree with your analysis. The spec has guidance that we should implement:

Prior to confirming the handshake, a peer might be unable to process 1-RTT packets, so an endpoint SHOULD send a CONNECTION_CLOSE frame in both Handshake and 1-RTT packets. A server SHOULD also send a CONNECTION_CLOSE frame in an Initial packet.

Ralith commented 4 months ago

Fix drafted at https://github.com/quinn-rs/quinn/pull/1767.