threefoldtech / rmb-rs

RMB implementation in rust
Apache License 2.0
3 stars 1 forks source link

Federation works only for relays listening on the default port #149

Closed sameh-farouk closed 10 months ago

sameh-farouk commented 10 months ago

https://github.com/threefoldtech/rmb-rs/blob/933b64ba21d0ddc3d3dfb3861e381285814569c9/src/bins/rmb-peer.rs#L159-L165

https://github.com/threefoldtech/rmb-rs/blob/933b64ba21d0ddc3d3dfb3861e381285814569c9/src/peer/socket.rs#L97

https://github.com/threefoldtech/rmb-rs/blob/933b64ba21d0ddc3d3dfb3861e381285814569c9/src/relay/federation/router.rs#L23-L27

muhamadazmy commented 10 months ago

This is by design. Twin can only define his home relay by a full qualified name. Relays then can only relay messages to public relays that has SSL enabled.

In the code you shared above seems like we also can forward over http during tests if you run cargo test

sameh-farouk commented 10 months ago

Thank you for your reply. I admit that I was a bit confused about few things. But this means that we can make things clearer for everyone. Here are some points that I think we should address:

1- I misunderstood the idea that the relays open two ports, one of which can be customized by the relay flag listen for communication with peers over websocket, and the other of which is https 443 for secure transfers of encrypted traffic between relays.

This information should be documented more clearly.

2- A source of confusion is using the same information for two purposes. The relay option requires the relay address (IP or domain, no validation 🤷‍♂️) and uses it to:

I think it would be clearer to separate concerns and have a specific option to set the relay domain (only domain, not a full address) used to update the chain and relays communication (with added validation to ensure that user provided a domain). This would be less magic to user.

3- This is a question: why we do not support wss/tls for peer <-> relay communications but enforce tls communications between relays over https?

Could you please let me know what you think about them? If you agree that these are valid points that deserve attention, I can reopen the issue or create a new one to address them accordingly.

muhamadazmy commented 10 months ago

Your comment confuses me, i don't get what u mean.

sameh-farouk commented 10 months ago

@muhamadazmy Thank you so much for helping me out with this issue. I finally realized that I need a reverse proxy for TLS termination (your note), which was the key to solving my problem. I was confused because I thought the RMB would handle this, but I was wrong. Now I understand that the relay listens internally to an unsecure port like 8080 (the one used unless overridden by the listen flag), while the peers send the encrypted traffic to the reverse proxy, hence the two ports in the story.

This issue was mainly due to the lack of documentation. I know there are other issues open for improving the documentation, and I hope they will be resolved soon. Thank you for your time and patience.