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 Resolver interface to transport #1719

Merged
merged 15 commits into from
Sep 13, 2022
Merged

Conversation

MarcoPolo
Copy link
Collaborator

@MarcoPolo MarcoPolo commented Aug 25, 2022

fixes #1597

This adds a new Resolver interface that can be optionally implemented by transports. This lets a transport resolve a single multiaddr into many multiaddrs, while still letting the swarm handle concurrency in the actual dialing step.

This PR also fixes secure websockets (wss) by having the websocket transport resolve a multiaddr with a dns hostname and /wss into a multiaddr that explictly defines the sni component. e.g. converts /dns4/example.com/tcp/1234/wss into /dns4/example.com/tcp/1234/tls/sni/example.com/ws. This is useful so that when the swarm resolves the dns4 address the websocket transport still has access to the hostname so that it can properly set the SNI. e.g. the swarm converts /dns4/example.com/tcp/1234/tls/sni/example.com/ws into /ip4/1.2.3.4/tcp/1234/tls/sni/example.com/ws, and the websockets transport still knows what the SNI should be when it's asked to dial.

Tests will fail until multiformats/go-multiaddr#182 is merged and released.

@MarcoPolo MarcoPolo force-pushed the marco/1597/resolve-in-transports branch from 54e85e1 to 2dc2ace Compare September 7, 2022 22:41
@MarcoPolo MarcoPolo marked this pull request as ready for review September 8, 2022 03:07
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning to add the WebTransport logic in a follow-up PR?

p2p/net/swarm/swarm.go Outdated Show resolved Hide resolved
p2p/net/swarm/swarm_dial.go Show resolved Hide resolved
p2p/net/swarm/testing/testing.go Outdated Show resolved Hide resolved
fallthrough
case ma.P_DNS6:
fallthrough
case ma.P_DNSADDR:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to:

case ma.P_DNS, ma.P_DNS4, ma.P_DNS6, ma.P_DNSADDR:

Right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe easier: maddr.ValueForProtocol

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be multiple loops to get the value, right?

p2p/transport/websocket/websocket.go Outdated Show resolved Hide resolved
p2p/transport/websocket/websocket.go Outdated Show resolved Hide resolved
p2p/transport/websocket/websocket.go Outdated Show resolved Hide resolved
p2p/transport/websocket/websocket.go Outdated Show resolved Hide resolved
p2p/transport/websocket/websocket_test.go Outdated Show resolved Hide resolved
p2p/transport/websocket/websocket_test.go Show resolved Hide resolved
@marten-seemann
Copy link
Contributor

Tests will fail until multiformats/go-multiaddr#182 is merged and released.

Have you considered requiring the repo by its commit hash? That would allow you to run CI (and would have caught at least one of the issues here, the copying of the mutex).

MarcoPolo and others added 2 commits September 8, 2022 08:26
Co-authored-by: Marten Seemann <martenseemann@gmail.com>
@MarcoPolo
Copy link
Collaborator Author

Are you planning to add the WebTransport logic in a follow-up PR?

Yep!

@MarcoPolo MarcoPolo merged commit f654b4b into master Sep 13, 2022
This was referenced Sep 19, 2022
lidel added a commit to libp2p/go-libp2p-relay-daemon that referenced this pull request Oct 5, 2022
includes
libp2p/go-libp2p#1719
which fixes secure websockets (wss) by having the websocket transport
resolve a multiaddr with a dns hostname and /wss into a multiaddr that
explictly defines the sni component. e.g. converts
/dns4/example.com/tcp/1234/wss into
/dns4/example.com/tcp/1234/tls/sni/example.com/ws
lidel added a commit to libp2p/go-libp2p-relay-daemon that referenced this pull request Oct 5, 2022
includes
libp2p/go-libp2p#1719
which fixes secure websockets (wss) by having the websocket transport
resolve a multiaddr with a dns hostname and /wss into a multiaddr that
explictly defines the sni component. e.g. converts
/dns4/example.com/tcp/1234/wss into
/dns4/example.com/tcp/1234/tls/sni/example.com/ws
lidel added a commit to libp2p/go-libp2p-relay-daemon that referenced this pull request Oct 5, 2022
includes
libp2p/go-libp2p#1719
which fixes secure websockets (wss) by having the websocket transport
resolve a multiaddr with a dns hostname and /wss into a multiaddr that
explictly defines the sni component. e.g. converts
/dns4/example.com/tcp/1234/wss into
/dns4/example.com/tcp/1234/tls/sni/example.com/ws
lidel added a commit to libp2p/go-libp2p-relay-daemon that referenced this pull request Oct 5, 2022
includes
libp2p/go-libp2p#1719
which fixes secure websockets (wss) by having the websocket transport
resolve a multiaddr with a dns hostname and /wss into a multiaddr that
explictly defines the sni component. e.g. converts
/dns4/example.com/tcp/1234/wss into
/dns4/example.com/tcp/1234/tls/sni/example.com/ws
@p-shahi p-shahi deleted the marco/1597/resolve-in-transports branch March 26, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

DNS resolution should be moved to the transports
2 participants