Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions dc/s2n-quic-dc/src/psk/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ pub const DEFAULT_MTU: u16 = DEFAULT_BASE_MTU;
pub const DEFAULT_PTO_JITTER_PERCENTAGE: u8 = 33;
const DEFAULT_INITIAL_RTT: Duration = Duration::from_millis(1);
const DC_QUIC_VERSION: u32 = 0;
/// Application error code the client uses to close a connection whose dcQUIC handshake did not
/// complete when `ConfirmComplete` failed or timed out.
const DC_HANDSHAKE_INCOMPLETE_ERROR: u32 = 1;
/// Number of threads used to make progress on the TLS handshake
pub const DEFAULT_THREAD_COUNT: usize = 0;

Expand Down Expand Up @@ -534,10 +537,22 @@ impl HandshakeQueue {
}
Ok(Err(e)) => {
// ConfirmComplete::wait_ready failed. We should treat the handshake as failed.
//
// Explicitly close instead of letting `connection` drop, which would emit a
// clean (no-error) CONNECTION_CLOSE. A clean close is the signal the server
// uses to complete the dc handshake when the token ACK is lost; since the
// handshake did not complete here, we must not send it. Any explicit close is
// emitted as an application CONNECTION_CLOSE (`connection::Error::Application`),
// which the server does not treat as completion. If the connection is already
// closed this is a no-op.
Comment thread
boquan-fang marked this conversation as resolved.
connection.close(DC_HANDSHAKE_INCOMPLETE_ERROR.into());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check what this looks like on the server side in terms of rendering? I assume the default is going to be an integer or just 'application error', in which case it would be good to intercept that somewhere and include a better message in our logging.

return Err(e);
}
Err(_elapsed) => {
// Handshake timeout occurred. We should treat the handshake as failed.
//
// Close with an explicit error. This is the same reason as the first error case.
connection.close(DC_HANDSHAKE_INCOMPLETE_ERROR.into());
Comment thread
boquan-fang marked this conversation as resolved.
Outdated
return Err(io::Error::new(
io::ErrorKind::TimedOut,
"ConfirmComplete handshake timeout",
Expand Down
Loading
Loading