Skip to content

Commit

Permalink
conn: ensure complete_io flushes pending writes.
Browse files Browse the repository at this point in the history
Previously, calls to `complete_io()` may return as if handshaking has
completed, but leave pending TLS writes queued that won't be sent until
a subsequent call to `complete_io()` is made.

This happens because `is_handshaking()` can begin to return false after
calls to `process_new_packets()` while there are final handshake packets
put in the connection's buffers, but not yet extracted to be sent to the
peer.

The end result is that calling `complete_io()` once is not
sufficient to fully complete a handshake with a peer. A second call
was required to flush the pending packets.

In this commit the `complete_io()` logic is updated to continue
processing IO when calling `process_new_packets()` has queued TLS
writes, only returning to the caller when all pending IO has been dealt
with and the handshake truly completed.

We can test this behaviour by updating the
`client_complete_io_for_handshake` and
`server_complete_io_for_handshake` unit tests to assert there are no
pending TLS writes after calling `complete_io()`. Prior to this commit
these assertions would fail, and with the updated logic they pass as
expected.
  • Loading branch information
cpu authored and djc committed Apr 17, 2023
1 parent ce933bb commit 60cfb9b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
10 changes: 9 additions & 1 deletion rustls/src/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,13 @@ impl<Data> ConnectionCommon<Data> {
Self: Sized,
T: io::Read + io::Write,
{
let until_handshaked = self.is_handshaking();
let mut eof = false;
let mut wrlen = 0;
let mut rdlen = 0;

loop {
let until_handshaked = self.is_handshaking();

while self.wants_write() {
wrlen += self.write_tls(io)?;
}
Expand Down Expand Up @@ -415,6 +416,13 @@ impl<Data> ConnectionCommon<Data> {
}
};

// if we're doing IO until handshaked, and we believe we've finished handshaking,
// but process_new_packets() has queued TLS data to send, loop around again to write
// the queued messages.
if until_handshaked && !self.is_handshaking() && self.wants_write() {
continue;
}

match (eof, until_handshaked, self.is_handshaking()) {
(_, true, false) => return Ok((rdlen, wrlen)),
(_, false, _) => return Ok((rdlen, wrlen)),
Expand Down
2 changes: 2 additions & 0 deletions rustls/tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,7 @@ fn client_complete_io_for_handshake() {
.unwrap();
assert!(rdlen > 0 && wrlen > 0);
assert!(!client.is_handshaking());
assert!(!client.wants_write());
}

#[test]
Expand Down Expand Up @@ -1390,6 +1391,7 @@ fn server_complete_io_for_handshake() {
.unwrap();
assert!(rdlen > 0 && wrlen > 0);
assert!(!server.is_handshaking());
assert!(!server.wants_write());
}
}

Expand Down

0 comments on commit 60cfb9b

Please sign in to comment.