Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shutdown TLS streams #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

aljazerzen
Copy link

@aljazerzen aljazerzen commented Apr 26, 2024

When I tried updating to latest version of rustls, I've noticed the following test failure:

rustls::Custom {
  kind: UnexpectedEof,
  error: "peer closed connection without sending TLS close_notify: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof"
}

This is happening because in rustls 0.21.2, they started differentiating between "unexpected and expected EOF in Stream and StreamOwned". This follows TLS standard, as the close_notify message must be sent before closing the connection.

We weren't doing that I our test cases so they failed.

But even when I added AsyncWrite::poll_shutdown(socket).await to the tests, they were still failing, since we weren't propagating this call to the underlying sync TLS streams, but just calling .flush(). The problem here was that there is no sync equivalent to AsyncWrite::shutdown.

My solution was to add a new trait WriteShutdown that all sync TLS streams must implement. It turns out that OpenSSL, native TLS and rustls all have shutdown methods that they are expecting to be called.


This is a breaking change, because there are streams that previous versions deemed correct, but newer versions would return an error on.

This is fixes vulnerability to truncation attacks.

@aljazerzen
Copy link
Author

For anyone looking, I've forked this repo and published tls-api-2. This fork will probably be retired when these PRs are merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant