-
Notifications
You must be signed in to change notification settings - Fork 999
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
/dnsaddr
can not be properly used with secure WebSocket transport
#5529
Comments
This sounds like a bug in the WebSocket transport that isn't properly using the SNI in the provided multiaddr. |
4 tasks
mergify bot
pushed a commit
that referenced
this issue
Nov 5, 2024
## Description Returns `Error::InvalidMultiaddr` when `parse_ws_dial_addr` is called with `/dnsaddr`. As per its specification, `/dnsaddr` domains are not meant to be directly dialed, instead it should be appended with `_dnsaddr.` and used for DNS lookups afterwards Related: #5529 Fixes: #5601 ## Notes & open questions * Is it okay to return an error, or should I perform a DNS lookup and resolve that DNS afterwards if address has `/dnsaddr`? * If so, how should I handle that case where DNS lookup returns multiple multiaddrs? ## Change checklist - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have added tests that prove my fix is effective or that my feature works - [x] A changelog entry has been made in the appropriate crates --------- Co-authored-by: Darius Clark <[email protected]>
jxs
pushed a commit
to jxs/rust-libp2p
that referenced
this issue
Jan 6, 2025
## Description Returns `Error::InvalidMultiaddr` when `parse_ws_dial_addr` is called with `/dnsaddr`. As per its specification, `/dnsaddr` domains are not meant to be directly dialed, instead it should be appended with `_dnsaddr.` and used for DNS lookups afterwards Related: libp2p#5529 Fixes: libp2p#5601 ## Notes & open questions * Is it okay to return an error, or should I perform a DNS lookup and resolve that DNS afterwards if address has `/dnsaddr`? * If so, how should I handle that case where DNS lookup returns multiple multiaddrs? ## Change checklist - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have added tests that prove my fix is effective or that my feature works - [x] A changelog entry has been made in the appropriate crates --------- Co-authored-by: Darius Clark <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Currently there is no way to properly pass
/dnsaddr
to WSS transport and work as expected:libp2p::websocket::WsConfig
withlibp2p::dns::Transport
then you break secure WebSocket because DNS transport will resolve/dns4
to/ip4
.libp2p::dns::Transport
withlibp2p::websocket::WsConfig
then you break/dnsaddr
because it will never fetch the multiaddresses from DNS.Currently 2 is the recommended way (link).
Motivation
I want to be able to use
/dnsaddr
with secure WebSocket.Requirements
I want to be able to wrap
libp2p::websocket::WsConfig
withlibp2p::dns::Transport
which will resolve only/dnsaddr
before it passing the multiaddr to WebSocket transport.For example:
In case of
/dnsaddr
the above transport should have the following flow:/dnsaddr/da-bridge-1.celestia-arabica-11.com/p2p/12D3KooWGqwzdEqM54Dce6LXzfFr97Bnhvm6rN7KM7MFwdomfm4S
/dns/da-bridge-1.celestia-arabica-11.com/tcp/2122/tls/ws/p2p/12D3KooWGqwzdEqM54Dce6LXzfFr97Bnhvm6rN7KM7MFwdomfm4S
/dns
multiaddr to WebSocket transport./dns
multiaddr and passes multiaddr to TCP transport./dns
multiaddr to/ip4
multiaddr and establishes the connection.In case of
/dns
the above transport should have the following flow:/dns/da-bridge-1.celestia-arabica-11.com/tcp/2122/tls/ws/p2p/12D3KooWGqwzdEqM54Dce6LXzfFr97Bnhvm6rN7KM7MFwdomfm4S
/dns
multiaddr to WebSocket transport./dns
multiaddr and passes multiaddr to TCP transport./dns
multiaddr to/ip4
multiaddr and establishes the connection.Open questions
It would be hard to implement this kind of transport combination in
SwarmBuilder
.Are you planning to do it yourself in a pull request ?
Maybe
The text was updated successfully, but these errors were encountered: