search

quininer / tokio-rustls

Asynchronous TLS/SSL streams for Tokio using Rustls.
142 stars 38 forks source link

Correctly implement close_notify semantic #34

Closed jerryz920 closed 5 years ago

jerryz920 commented 5 years ago

Hi,

I mentioned in #33 that bi-directional copy can lead to an unexpected connection reset. The reason is that the master branch (commit b6e39450ce48d4b19c0095952df867e31fe5a51d) currently sets stream state to SHUTDOWN after the client calls AsyncWrite.shutdown(). This state will make read call to return Ok(0). So the client thoughts EOF came and drops the socket. However, the server still has data to write at the moment, so a connection reset is thrown.

This is a specification problem: TLS 1.2 requires the close_notify to also close read. However, the presence of middle boxes make this behavior inappropriate, and I believe many TLS implementation like OpenSSL actually allows "half-close" semantic. In TLS 1.3 this should have been changed: https://tools.ietf.org/id/draft-ietf-tls-tls13-25.html#closure-alerts. So this means the shutdown state should not affect read any more. Further, EoF should also not affect shutdown. I guess it will make sense to use bit-mask to allow both state to be set or separate the enum to ReadState and WriteState.

thanks

jerryz920 commented 5 years ago

oh this can be closed as well ;)