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

bug: missing support for /tls multiaddr #2024

Closed
danisharora099 opened this issue Sep 6, 2023 · 5 comments
Closed

bug: missing support for /tls multiaddr #2024

danisharora099 opened this issue Sep 6, 2023 · 5 comments
Assignees
Labels
need/author-input Needs input from the original author

Comments

@danisharora099
Copy link

  • Version: 0.46.x
  • Platform: macos
  • Subsystem: Dialer

Severity: High

Description:

js-libp2p throws an error Unsupported protocol tsl when a multiaddr /ip4/192.168.68.58/tcp/433/tls/ws/p2p/16Uiu2HAkyZuLQy4sPRte7khemYHmkQewzjdTPRXZcxPH6NgjhR97 is dialed

The expectation is that a secure websocket multiaddr would have /wss/ but apparently wss was replaced by /tls/ws:

Steps to reproduce the error:

Try to dial /ip4/192.168.68.58/tcp/433/tls/ws/p2p/16Uiu2HAkyZuLQy4sPRte7khemYHmkQewzjdTPRXZcxPH6NgjhR97 through js-libp2p

@danisharora099 danisharora099 added the need/triage Needs initial labeling and prioritization label Sep 6, 2023
@maschad maschad self-assigned this Sep 6, 2023
@maschad maschad added this to js-libp2p Sep 6, 2023
@maschad maschad moved this to 🤨Needs Investigation in js-libp2p Sep 6, 2023
@maschad
Copy link
Member

maschad commented Sep 6, 2023

Could you share the example where you tried to dial this address? as well as the dependencies of the listening node and the environment?

You are correct in that wss was deprecated although we still utilize it internally in our monorepo tests,

but it's strange that you get this error since there's support for backwards compatibility that is tested in multiaddrs

I have tried to dial the multiaddr you shared but I am getting a timeout error ( I assume the node isn't running at that public IP at the moment).

But I am able to have two local nodes dial each other with the current format as such:

import { createLibp2p } from 'libp2p'
import { webSockets } from '@libp2p/websockets'
import { mplex } from '@libp2p/mplex'
import { noise } from '@chainsafe/libp2p-noise'
import { yamux } from '@chainsafe/libp2p-yamux'
import { tcp } from '@libp2p/tcp'
import { identifyService } from 'libp2p/identify'

const node1 = await createLibp2p({
   addresses: {
    listen: ['/ip4/127.0.0.1/tcp/8000/tls/ws']
  },
  transports: [
    tcp(),
    webSockets()
  ],
  streamMuxers: [yamux(), mplex()],
  connectionEncryption: [noise()],
  services: {
    identify: identifyService()
  }
})

const node2 = await createLibp2p({
   addresses: {
    listen: ['/ip4/127.0.0.1/tcp/8001/tls/ws']
  },
  transports: [
    tcp(),
    webSockets()
  ],
  streamMuxers: [yamux(), mplex()],
  connectionEncryption: [noise()],
  services: {
    identify: identifyService()
  }
})

await node1.start()
await node2.start()

const connection = await node1.dial(node2.getMultiaddrs()[0])

console.log('node1 is connected to node2:', connection.status)
// Status is logged as open

@maschad maschad added need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization and removed need/triage Needs initial labeling and prioritization labels Sep 6, 2023
@achingbrain
Copy link
Member

Could you also confirm the version of @multiformats/multiaddr you have installed? /tls support was only added in 11.4.0 -

$ npm ls @multiformats/multiaddr

@maschad maschad removed the need/triage Needs initial labeling and prioritization label Sep 12, 2023
@danisharora099
Copy link
Author

ah, found the problem!

it's actually coming from an older version of @multiformats/multiaddr-to-uri before it supported it

the problem is that @libp2p/websockets does not have the dependency upgraded to where it supports tls -- currently uses 9.0.2 while support for it was introduced on 9.0.5
multiformats/js-multiaddr-to-uri#120

@danisharora099
Copy link
Author

opened a PR for this here: #2059

@maschad
Copy link
Member

maschad commented Sep 18, 2023

Closing as #2059 (comment) should solve the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants