rathole
rathole copied to clipboard
QUIC support
Since @juzi5201314 indicated that he would close https://github.com/rapiz1/rathole/pull/64 I now bring this PR. Many thanks to juzi for his PR, looking at your approach and reading the comments in the PR was very helpful!
This PR is aware of https://github.com/rapiz1/rathole/pull/93 and an effort has been made to ensure QuicTransport::accept
is cancel safe. But honestly, it is very difficult to determine if that is in fact the case.
This PR is standalone and does not immediately require merging of https://github.com/rapiz1/rathole/pull/95 which is much more experimental.
Just to kickstart the review, I want to address some comments in https://github.com/rapiz1/rathole/pull/64
rust_tls vs native_tls
I may be possible to drop in a native_tls
version of quinn-proto::crypto
, but that is a lot of work, that I don't think I can put in, for this particular feature. Also, I might have to rewrite certain other parts of quinn as well that seems to, only be available when using the rust_tls
feature of the quinn
crate.
If the compilation time / executable size is a problem, then I would propose making QUIC a non-default feature.
Releasing UDP sockets in tests
I have spent a long time trying to get the tests working without using a pre-determined sleep between shutdown and restart of the server. This is what I came up with:
- make sure that
rathole::run
does not exit before the currently running server instance coroutine has joined - add new
async fn close(a: Self::Acceptor)
method to theTransport
trait such that graceful termination of the QUIC connections can be awaited - await
close
inServer::run
to ensure that socket is released before coroutine joins
I hope that the above solution is acceptable. The only other option is to rebind to a new random UDP port from a drop
on the Acceptor
.
examples/tls/testserver.pfx
looks like a slipping file?
This PR is aware of #93 and an effort has been made to ensure QuicTransport::accept is cancel safe. But honestly, it is very difficult to determine if that is in fact the case.
Seems safe to me 🤔
examples/tls/testserver.pfx
looks like a slipping file?
No, identity.pfx
does not provide a certificate that QUIC will accept. AFAIK QUIC requires that the server cert contains a DNS SAN. identity.pfx
has a SAN, but it is IP type.
There are two files, examples/tls/test_server.pfx
and examples/tls/testserver.pfx
. Only the former has been used in toml files.
Ahh, yeah that file. that was not supposed to be committed. I will delete
Squashed the branch since rebasing was getting unwieldy with so many small commits and reverts
I see tests failing.
Can only test linux locally, and that passes on my desktop...
Dunno why the tests are failing
Jan 18 02:53:19.113 INFO handle{service=echo}: rathole::client: Starting 092c79e8f80e559e404bcf660c48f3522b67aba9ff1484b0367e1a4ddef7431d
Jan 18 02:53:19.113 INFO handle{service=pingpong}: rathole::client: Starting c78862c4afddaa20fd3ff12e5e270480706499341ca5d1d7437ec9668556805b
Jan 18 02:53:20.031 INFO test{config_path="tests/for_udp/quic_transport.toml" t=Udp}: integration_test: start the server
Jan 18 02:53:20.039 INFO config_watcher{path="tests/for_udp/quic_transport.toml"}: rathole::config_watcher: Start watching the config
Jan 18 02:53:20.040 INFO rathole::server: Listening at 0.0.0.0:2332
Jan 18 02:53:20.182 INFO test{config_path="tests/for_tcp/quic_transport.toml" t=Tcp}: integration_test: start the server
Jan 18 02:53:20.189 INFO config_watcher{path="tests/for_tcp/quic_transport.toml"}: rathole::config_watcher: Start watching the config
Jan 18 02:53:20.194 INFO rathole::server: Listening at 0.0.0.0:2333
Jan 18 02:53:22.059 INFO test{config_path="tests/for_udp/quic_transport.toml" t=Udp}: integration_test: echo
test udp ... FAILED
Jan 18 02:53:22.231 INFO test{config_path="tests/for_tcp/quic_transport.toml" t=Tcp}: integration_test: echo
error: test failed, to rerun pass '--test integration_test'
test tcp ... FAILED
Jan 18 02:53:19.113 INFO handle{service=echo}: rathole::client: Starting 092c79e8f80e559e404bcf660c48f3522b67aba9ff1484b0367e1a4ddef7431d
shows that the client tried to connect to the server, but by the end of the test, it had not succeeded nor returned any feedback. This is possibly caused by a IPv6 bounded socket connecting to a IPv4 address.
I think this reveals two potential issues to address:
- QUIC
connect
can't return an error immediately when the target port is not listened, due to the nature of UDP, which is the cause of no feedback. This can be solved either by- Enforce a timeout at https://github.com/emillynge/rathole/blob/7f083c21b7f0f637b28337c48c74f43830488aa4/src/client.rs#L393-L397
- Utilize some QUIC features or enforce a timeout in the transport
connect
implementation. I wonder if there's already some timeout setting for QUICconnect
.
- As I previously mentioned, a IPv4 bind socket can't connect to a IPv6 address or vice versa. See https://docs.rs/quinn/latest/quinn/struct.Endpoint.html#method.client about dual stack.
Hi @emillynge What's the status of this? I think we are really close to get this merged.
Hi @emillynge What's the status of this? I think we are really close to get this merged.
I'm on vacation now, and then next week my parental leave is over and I'll have to go to work. I will try to wrap this up in the coming month, but my time is very limited now unfortunately.
Hey @emillynge may I ask nicely what the situation is with this PR? 👀
@emillynge if you don't mind, I can pick up this and finish this PR.
@emillynge if you don't mind, I can pick up this and finish this PR.
@cssivision I would be very pleased if you do so.
Can I transfer/share ownership of PR and branch somehow?
@cssivision can fork your repository with the PR branch and open a new one at least. But i think you can also add him as a member to your forked rathole repository so he can push changed. Correct me if i'am wrong
What is the future of this pull request? @rapiz1
The reason I'm asking this is that I'm thinking about implementing a new feature of QUIC as a transport type for rathole
should I submit a whole new pull request?