quininer / tokio-rustls

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

Upgrading from tokio-rustls 0.9.1 to 0.9.2 causes connections to stop processing unexpectedly #37

Closed khuey closed 4 years ago

khuey commented 5 years ago

Unfortunately my code is not open source, the versions on crates.io do not correspond to the version in this repository, and this crate doesn't appear to print any logging even at the trace level, so I can't report anything more useful than that.

quininer commented 5 years ago

Can you describe it in detail or provide a reproducible step?

quininer commented 5 years ago

Since there is no detail, I can only make some guesses.

In 0.9.1, read inside is blocked by write, which causes some unexpected situations. see https://github.com/quininer/tokio-rustls/issues/32

After 0.9.2, read is no longer blocked by write, so you may need to write write.and_then(flush).and_then(read) instead of write.and_then(read).

quininer commented 5 years ago

I realized that this is more like a behavior change than a bugfix, it should be a new version. :(

jerryz920 commented 5 years ago

something related to changes in #32?

haata commented 4 years ago

I'm having the same sort of issue between 0.9.1 and anything afterwards (0.9.2+). My use case is a bit more complicated as I believe it's actually the capnproto-futures that has the problem but I haven't been able to locate yet where the write is taking place.

Here's an example: https://github.com/haata/capntls/tree/tokio-rustls-0.9.1

cargo run server localhost:12345 &
cargo run client localhost:12345
    Finished dev [unoptimized + debuginfo] target(s) in 0.24s
     Running `target/debug/capntls client 'localhost:12345'`
tester@testserver.com:hello
tester@testserver.com:helloa

vs. https://github.com/haata/capntls/tree/tokio-rustls-0.9.2

cargo run server localhost:12345 &
cargo run client localhost:12345
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/capntls client 'localhost:12345'`
tester@testserver.com:hello
^C

Normally I'd just start using 0.9.1 and move on however I have some other project dependencies that require a newer version of ring so I also need to use a more recent version of tokio-rustls.

quininer commented 4 years ago

Have you tried 0.9.3? It fixes a bug about flush. https://github.com/quininer/tokio-rustls/commit/fe8fafecee85191efb73aa7edaed7021a5c8beff

haata commented 4 years ago

Hmmm, 0.9.3 doesn't help at all, there must be something else that changed from 0.9.1 to 0.9.2... (thanks for pointing that out!)

https://github.com/haata/capntls/tree/tokio-rustls-0.9.3

cargo run server localhost:12345 &
cargo run client localhost:12345
    Finished dev [unoptimized + debuginfo] target(s) in 0.21s
     Running `target/debug/capntls client 'localhost:12345'`
tester@testserver.com:hello
^C

Something odd that I did notice when using wireshark (I have SSLKEYLOGFILE setup) is that the data is sent from the server but the packet containing the response ends up receiving a TCP Retransmission because the client didn't ACK the response. The TCP retransmission is ACK'd but it has a bunch more TCP options set. I'll attach the trace shortly.

Server response packet, looks about what it should be (I'll need to run this again with 0.9.1). server_response

TCP Retransmission retransmission

ACK of retransmission final_ack

Here is the full trace as well. Packet 41 will show my@email.com:hello2 in the decoded form (was run on a different computer, was using hello2 instead of helloa, otherwise the code is the same). mykeys.txt trace1.pcapng.gz

quininer commented 4 years ago

Ah, this is my mistake. mio uses edge trigger, so premature read operations cause event loss.

I think this patch can solve problem and I will release a new version later.

quininer commented 4 years ago

I believe this problem has been resolved. if not, please reopen it or open a new issue.

khuey commented 4 years ago

I'm not seeing any issues on 0.10.