After this commit, this crate will support using TLS streams in a
half-closed state. Note that the TLS 1.3 spec in RFC 8446
says this should be supported:
```
Each party MUST send a "close_notify" alert before closing its write
side of the connection, unless it has already sent some error alert.
This does not have any effect on its read side of the connection. Note
that this is a change from versions of TLS prior to TLS 1.3 in which
implementations were required to react to a "close_notify" by discarding
pending writes and sending an immediate "close_notify" alert of their
own. That previous requirement could cause truncation in the read side.
Both parties need not wait to receive a "close_notify" alert before
closing their read side of the connection, though doing so would
introduce the possibility of truncation.
```
https://tools.ietf.org/html/rfc8446#page-87
The `rustls` crate raises such a clean closure of a
[`ClientSession`](https://docs.rs/rustls/0.18.0/rustls/struct.ClientSession.html#impl-Read)
or
[`ServerSesson`](https://docs.rs/rustls/0.18.0/rustls/struct.ServerSession.html#impl-Read)
read-side with `ErrorKind::ConnectionAborted`.
This crate's `TlsState` struct already encodes support for the
half-closed states `TlsState::ReadShutdown` and
`TlsState::WriteShutdown`, in addition to `TlsState::FullyShutdown`.
However, the current behavior of the `AsyncRead` implementation is that
it unconditionally shuts-down the write-half of a connection after the
read-half closes cleanly with `ErrorKind::ConnectionAborted`.
This change removes the `stream.session.send_close_notify()` and
`this.state.shutdown_write()` calls from `poll_read()`. Note that
`stream.session.send_close_notify()` is still called in
`poll_shutdown()`, which the application calls to cleanly shutdown the
write-half.
I highly suspect the logic of this can be simplified and cleaned up
further. Minimally, the edited match statement now has two identical
branches which could be combined into one. Additionally, perhaps the
`Stream` implementation should simply return `Ok(0)` for this case in
its implementation of
[`tokio::io::AsyncRead`](https://docs.rs/tokio/0.2/tokio/io/trait.AsyncRead.html),
since that's the defined way to indicate clean closure with EOF from
`AsyncRead`. However, I want to make the minimal changes and have them
reviewed for logical correctness first.
Co-authored-by: Braden Ehrat <braden@cloudflare.com>
The `wants_read` only changes after `process_new_packets`,
which means that not immediately calling `process_new_packets` may cause rustls to cache too much data.
nickelc
Roman Titov
quininer
Eliza Weisman
Jason Heeris
Lucio Franco
Cheng JIANG
Smit
Braden Ehrat
Kirill Fomichev
zzzdong
Denis Molokanov
aloucks
Gleb Pomykalov