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

libp2p-websocket: replace /wss with /tls/ws #2449

Closed
tomaka opened this issue Jan 23, 2022 · 11 comments · Fixed by #5523
Closed

libp2p-websocket: replace /wss with /tls/ws #2449

tomaka opened this issue Jan 23, 2022 · 11 comments · Fixed by #5523
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted

Comments

@tomaka
Copy link
Member

tomaka commented Jan 23, 2022

According to the multiaddr spec, the /wss specifier is deprecated and /tls/ws should be preferred.
libp2p-websocket only supports /wss.

(I've taken the liberty to mark this as "easy", as it's just about interpreting the multiaddr)

@mxinden mxinden added getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted labels Jan 24, 2022
@jochasinga
Copy link
Contributor

Shouldn't this change be made on the multiaddr crate too?

@mxinden
Copy link
Member

mxinden commented Feb 2, 2022

Shouldn't this change be made on the multiaddr crate too?

Should be done via multiformats/rust-multiaddr#48.

@jochasinga
Copy link
Contributor

I can tackle this @mxinden @tomaka.

@jochasinga
Copy link
Contributor

Looks to me like even /wss scheme isn't supported currently? Keep getting MultiaddrNotSupported("/ip6/::1/tcp/0/wss") running test.

Since multiformats/rust-multiaddr#48 added tls support, will this be just upgrading libp2p-websocket to use it?

@jochasinga
Copy link
Contributor

Does this test case for rust-multiaddr make sense @tomaka?

// rust-multiaddr/src/from_url.rs

#[test]
fn normal_usage_wss() {
    let addr = from_url("wss://127.0.0.1:8000").unwrap();
    assert_eq!(addr, "/ip4/127.0.0.1/tcp/8000/tls/ws".parse().unwrap());
    assert_eq!(addr, "/ip4/127.0.0.1/tcp/8000/wss".parse().unwrap());
}

Basically, when parsing a multiaddress, when a tls is found, we peek to see if the next one is "ws" and write that off as Protocol::Wss(..).

@tomaka
Copy link
Member Author

tomaka commented Feb 5, 2022

I don't think it's a good idea to make /tls/ws and /wss equal. This is too magic and could lead to violated assumptions.
In my opinion the only change needed is making libp2p-websocket support /tls/ws.

@navasvarela
Copy link

Hello everyone, are there any updates on TLS support for websockets?

@thomaseizinger
Copy link
Contributor

Hello everyone, are there any updates on TLS support for websockets?

Support is there and there is an example in the docs! This ticket is only about the deprecated multiaddr component.

@navasvarela
Copy link

Hello everyone, are there any updates on TLS support for websockets?

Support is there and there is an example in the docs! This ticket is only about the deprecated multiaddr component.

Thank you! Would you be able to point me to the example?

@thomaseizinger
Copy link
Contributor

Hello everyone, are there any updates on TLS support for websockets?

Support is there and there is an example in the docs! This ticket is only about the deprecated multiaddr component.

Thank you! Would you be able to point me to the example?

See https://docs.rs/libp2p-websocket/latest/libp2p_websocket/struct.WsConfig.html#examples. It is a bit hard to discover. We are in the process of making the APIs of our crates more consistent, see #2217. Help in doing that for libp2p-websocket would be appreciated :)

@thomaseizinger
Copy link
Contributor

Closing in favor of multiformats/rust-multiaddr#100.

@thomaseizinger thomaseizinger closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2023
@mergify mergify bot closed this as completed in #5523 Aug 8, 2024
mergify bot pushed a commit that referenced this issue Aug 8, 2024
To keep backward compatibility the following rules were implemented:

* On dialing, `/tls/ws` and `/wss` are equivalent.
* On listening:
* If user listens with `/tls/ws` then `/tls/ws` will be advertised.
* If user listens with `/wss` then `/wss` will be advertised.

Closes #2449
Closes #5509

Pull-Request: #5523.
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this issue Sep 14, 2024
To keep backward compatibility the following rules were implemented:

* On dialing, `/tls/ws` and `/wss` are equivalent.
* On listening:
* If user listens with `/tls/ws` then `/tls/ws` will be advertised.
* If user listens with `/wss` then `/wss` will be advertised.

Closes libp2p#2449
Closes libp2p#5509

Pull-Request: libp2p#5523.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants