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

Add feature(s) to use hyper-rustls and/or h3 in datacake-rpc #59

Open
ousado opened this issue Aug 3, 2023 · 15 comments
Open

Add feature(s) to use hyper-rustls and/or h3 in datacake-rpc #59

ousado opened this issue Aug 3, 2023 · 15 comments

Comments

@ousado
Copy link

ousado commented Aug 3, 2023

Initially I wanted to look into replacing TCP with QUIC, but since datacake-rpc uses hyper, it's probably better to wait for / experiment with https://github.com/hyperium/h3. For the time being, hyper-rustls would be a useful addition, though. Thoughts?

@ChillFish8
Copy link
Member

Do you have a particular use case for adding the TLS? It's not been something I've particularly looked at for a use case since generally, your inter-node RPC should be internal only rather than publically accessible where TLS becomes more significant.

@ousado
Copy link
Author

ousado commented Aug 3, 2023

The immediate use case is using datacake-rpc as a general purpose RPC framework, also for communicating with the cluster from outside (replace Axum with it in the sqlite example, for instance), and for sensitive data I don't want to rely on cloud-providers to secure their internal networks properly. Controlling cluster membership via mutual certificate authentication is also a good thing, so lots of reasons, really.

@ChillFish8
Copy link
Member

Seems reasonable, it is probably worth doing at some point then, unfortunately, I think the way the general connection system is handled in general could do with improvement as whole, since we have some interesting hacks in order to support turmoil simulation.

@ousado
Copy link
Author

ousado commented Aug 3, 2023

Yes, I was glad to see that. Presumably searching the places with #[cfg(feature = "simulation")] should help finding the relevant bits quickly.

@ChillFish8
Copy link
Member

Yes should be

@ousado
Copy link
Author

ousado commented Aug 4, 2023

I investigated a little, and surprisingly it seems hyper + tls is an largely an unsolved issue. The one crate I've found that

  • uses hyper with tls
  • supports client cert authentication
  • is flexible enough to access remote_addr and information on peer certificates in handlers
  • supports both clients and servers

is (hidden in) tonic

Others either don't have client support like tls-listener, or haven't found a way to offer the above mentioned flexibility rustls/hyper-rustls#198 .

@ltransom
Copy link
Contributor

I second the idea of incorporating TLS. Ultimately, it is critical to our application, meaning we cannot go to production without it.

Some industries require that any socket connections, even within an owned data-center, must use TLS.

@ousado, is this something you are taking on or just highlighting the need?

@ChillFish8
Copy link
Member

I have some free time this weekend where I'll be improving some things in datacake, as part of that I'll look into adding TLS support.

@ousado
Copy link
Author

ousado commented Sep 11, 2023

sorry, not sure why I haven't seen this notification earlier

I've looked a bit further, and the tonic client/server implementations already include and depend on some protobuf stuff. Then I found axum_server which allows to use a custom rustls configuration. For the client I'm back to using hyperrustls, but I ran out of time so I haven't tried to plug them in into datacake yet. I believe they'd be up to the task, though. It doesn't feel quite right to pull in two different dependencies for this, but that's what the ecosystem offers at this point, I'm afraid.

I've also thought about replacing https with a raw socket connection, tower and a simple codec, but I don't know which http features are used in datacake and would have to be reimplemented.

@ChillFish8
Copy link
Member

It should not require anything more than just adding hyper-tls and rustls.

Replacing the HTTP with a raw socket is impractical as we don't really gain anything from creating a custom protocol, we already make very heavy use of HTTP/2 multiplexing and hyper's efficient IO handling.

@ousado
Copy link
Author

ousado commented Sep 11, 2023

Apparently there have been some changes to hyper-rustls with respect to the issues that made it impossible to access the remote address as well as certificates when accepting a connection, so it might indeed be sufficient now. hyper-tls only supports native-tls and only the client side, so far.

Do you also want to support native-tls?

rustls/hyper-rustls#219
rustls/hyper-rustls#198

@ltransom
Copy link
Contributor

ltransom commented Apr 5, 2024

@ChillFish8 did you see the direct email I sent you regarding this issue?

@ChillFish8
Copy link
Member

ChillFish8 commented Apr 5, 2024

I did, I sent a reply back, not sure if you got my reply though :/

Anyway I can copy what I replied with:

What sort of application are you trying to use Datacake's RPC module with?

Is it behind a load balancer or similar? Or just for inter-node communication.

I ask this because I am currently working on the IO layer which will be primarily switching to QUIC which will support TLS v1.3 and be TLS by default.

@ltransom
Copy link
Contributor

ltransom commented Apr 5, 2024

That is really strange, I did not receive a response. Double checked my spam folder as well.

It is a specialized key value store, using the datacake example as a reference.

In terms of TLS support, it needs to be for all connections; the connection with the clients and the inter-node connections.

Supporting TLS 1.3 is key! Thank you!

I believe supporting TLS on the client side interface is straightforward. It is the datacake inter-node communication that concerns me, which it sounds like you are addressing.

In our architecture, a load-balancer is not necessary.

@ChillFish8
Copy link
Member

Cool, sounds good then, like I said I'm working towards replacing the current IO layer using QUIC, but naturally due to QUIC's relatively young age few load balancers support QUIC in that way. But for your use case, it should be fine.

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

No branches or pull requests

3 participants