Skip to content

Commit

Permalink
fix(websocket): don't dial /dnsaddr addresses (#5613)
Browse files Browse the repository at this point in the history
## 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 <dariusc93@users.noreply.github.com>
  • Loading branch information
zetsuboii and dariusc93 authored Nov 5, 2024
1 parent 8387749 commit d021ce2
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 10 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ libp2p-upnp = { version = "0.3.1", path = "protocols/upnp" }
libp2p-webrtc = { version = "0.8.0-alpha", path = "transports/webrtc" }
libp2p-webrtc-utils = { version = "0.3.0", path = "misc/webrtc-utils" }
libp2p-webrtc-websys = { version = "0.4.0-alpha.2", path = "transports/webrtc-websys" }
libp2p-websocket = { version = "0.44.0", path = "transports/websocket" }
libp2p-websocket-websys = { version = "0.4.0", path = "transports/websocket-websys" }
libp2p-websocket = { version = "0.44.1", path = "transports/websocket" }
libp2p-websocket-websys = { version = "0.4.1", path = "transports/websocket-websys" }
libp2p-webtransport-websys = { version = "0.4.0", path = "transports/webtransport-websys" }
libp2p-yamux = { version = "0.46.0", path = "muxers/yamux" }
multiaddr = "0.18.1"
Expand Down
5 changes: 5 additions & 0 deletions transports/websocket-websys/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.4.1

- fix: Return `None` when extracting a `/dnsaddr` address
See [PR 5613](https://github.com/libp2p/rust-libp2p/pull/5613)

## 0.4.0

- Implement refactored `Transport`.
Expand Down
2 changes: 1 addition & 1 deletion transports/websocket-websys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-websocket-websys"
edition = "2021"
rust-version = "1.60.0"
description = "WebSocket for libp2p under WASM environment"
version = "0.4.0"
version = "0.4.1"
authors = ["Vince Vasta <vince.vasta@gmail.com>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
9 changes: 7 additions & 2 deletions transports/websocket-websys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ fn extract_websocket_url(addr: &Multiaddr) -> Option<String> {
}
(Some(Protocol::Dns(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dns4(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dns6(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dnsaddr(h)), Some(Protocol::Tcp(port))) => {
| (Some(Protocol::Dns6(h)), Some(Protocol::Tcp(port))) => {
format!("{}:{}", &h, port)
}
_ => return None,
Expand Down Expand Up @@ -549,6 +548,12 @@ mod tests {
.unwrap();
assert!(extract_websocket_url(&addr).is_none());

// Check `/dnsaddr`
let addr = "/dnsaddr/example.com/tcp/2222/ws"
.parse::<Multiaddr>()
.unwrap();
assert!(extract_websocket_url(&addr).is_none());

// Check non-ws address
let addr = "/ip4/127.0.0.1/tcp/2222".parse::<Multiaddr>().unwrap();
assert!(extract_websocket_url(&addr).is_none());
Expand Down
5 changes: 5 additions & 0 deletions transports/websocket/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.44.1

- fix: Return `Error::InvalidMultiaddr` when dialed to a `/dnsaddr` address
See [PR 5613](https://github.com/libp2p/rust-libp2p/pull/5613)

## 0.44.0

- Implement refactored `Transport`.
Expand Down
2 changes: 1 addition & 1 deletion transports/websocket/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-websocket"
edition = "2021"
rust-version = { workspace = true }
description = "WebSocket transport for libp2p"
version = "0.44.0"
version = "0.44.1"
authors = ["Parity Technologies <admin@parity.io>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
9 changes: 7 additions & 2 deletions transports/websocket/src/framed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,7 @@ fn parse_ws_dial_addr<T>(addr: Multiaddr) -> Result<WsAddress, Error<T>> {
}
(Some(Protocol::Dns(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dns4(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dns6(h)), Some(Protocol::Tcp(port)))
| (Some(Protocol::Dnsaddr(h)), Some(Protocol::Tcp(port))) => {
| (Some(Protocol::Dns6(h)), Some(Protocol::Tcp(port))) => {
break (format!("{h}:{port}"), tls::dns_name_ref(&h)?)
}
(Some(_), Some(p)) => {
Expand Down Expand Up @@ -993,6 +992,12 @@ mod tests {
assert_eq!(info.server_name, "::1".try_into().unwrap());
assert_eq!(info.tcp_addr, "/ip6/::1/tcp/2222".parse().unwrap());

// Check `/dnsaddr`
let addr = "/dnsaddr/example.com/tcp/2222/ws"
.parse::<Multiaddr>()
.unwrap();
parse_ws_dial_addr::<io::Error>(addr).unwrap_err();

// Check non-ws address
let addr = "/ip4/127.0.0.1/tcp/2222".parse::<Multiaddr>().unwrap();
parse_ws_dial_addr::<io::Error>(addr).unwrap_err();
Expand Down

0 comments on commit d021ce2

Please sign in to comment.