Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

Added wss dialing #46

Closed
wants to merge 3 commits into from
Closed

Added wss dialing #46

wants to merge 3 commits into from

Conversation

Jorropo
Copy link

@Jorropo Jorropo commented May 28, 2019

This pr is a duplicate of #15 because it seems dead.
I chosed to only allow connection from dnsaddr (and dns4 and dns6) because wss is for browser and bypass mixed content error, so I recreated this so users will not get confused by working with go-ipfs and not in browser.
That require an useless work and complexification of libp2p and libp2p-swarm so that was aborted.

Also certificates came from system and is currently is only avaible on *nix due to a bug in golang's crypto/x509 library. The code need no change when the bug will be fixed to be windows compatible (just recompile). So on windows certificates will not be loaded and tls connection will be made even if certificate is wrong.
That not a bug anymore because this fonctionality isn't anymore.

To do :

  • Implementation
  • Add some test
    • Parsing
    • Connection
  • Test in real condition
    • libp2p
    • ipfs
  • Add a small docs about reverse proxy

Fix :

  • Fix the bug disabling wss when SSL certificate is not avaible by disabling SSL verification in this case.
  • Fix the bug making dns4 and dns6 like dnsaddr (currently its ignoring 4 and 6 and just taking the host)

Potential future upgrade :

  • Verify tls certificate of connection. (useless for security but good for minor compatibility with js-libp2p)
  • Allow listen by generating a random certificate. (useless and will create some fake hope by browser peer and slow them down in their discovery of the network, only usefull with next point)
  • Generate an TLS certificate for listening by derivating the libp2p private key with websocket-secure or wss protocol code and the host to have a constant one. (doesn't add more config for user, allow to not use a reverse proxy but still require manual approval of certificate)

@Stebalien
Copy link
Member

Stebalien commented May 29, 2019

I chosed to only allow connection from dnsaddr (and dns4 and dns6) because wss is for browser and bypass mixed content error, so I recreated this so users will not get confused by working with go-ipfs and not in browser.

We usually resolve dnsaddrs at a higher layer so I'm not sure if this will work without some tweaks to go-libp2p and go-libp2p-swarm. However, I may be wrong.

@Jorropo
Copy link
Author

Jorropo commented May 29, 2019

@Stebalien I havn't tested this yet but I'm sure there will be no problem with resolving because from my test a.ValueForProtocol(p[0].Code) return either the ip or the domain.
Maybe I'll have to tweak with tls connection (with a manual resolving).

Edit:
Finaly there might be a problem if

We usually resolve dnsaddrs at a higher layer

Means transforming dnsaddr to ip.

If system pool isn't avaible wss can be made but without certificate verification.
@Stebalien
Copy link
Member

Edit:

Unfortunately, that's what we do, IIRC. However, we may record both...


Regardless, I believe you're looking for /dns4 and /dns6, not /dnsaddr.

@Jorropo
Copy link
Author

Jorropo commented May 30, 2019

@Stebalien

Regardless, I believe you're looking for /dns4 and /dns6, not /dnsaddr.

Actualy i'm looking for any of these, the library just use the domain and resolve it after.
And you makes me thinked I have to limit to ip4 if dns4 is used and same for v6.

@Stebalien
Copy link
Member

dnsaddr works a bit differently. Given /dnsaddr/foobar.com/ipfs/QmPeer, we lookup for TXT records of the form dnsaddr=/.../ipfs/QmPeer on the _dnsaddr.foobar.com domain. That allows us to specify the port and transport inside the DNS record as well.

Unfortunately, that means that converting /dnsaddr/foobar.com/tcp/123 to wss://foobar.com:123.


For now, I think the simplest solution is to specify the TLSClientConfig websocket option and pass in a TLS config with InsecureSkipVerify set to true. We're not relying on this for security anyways.

This was done because of 3 reason :
 - This is useless for security and was just here to have the same comportement of the browser.
 - This require some pretty heavy change in libp2p and libp2p-swarm to allow declaration of a protocol requiring dns4|6 addrs not ip.
 - This save some time by simplyfing testsuite.
@Jorropo
Copy link
Author

Jorropo commented Jun 6, 2019

@Stebalien today I tryed to make it works with go-libp2p-example, and after a long time of learning what is mod I finaly get it to not work :(.
I followed your advice but an other problem shown, host of http 1.1.
Currently the server endpoint is on <http|ws[s]>://0.0.0.0/ wich makes a problem because if we use IP we can't send the host parameter to request on anything other than default.
But on my default web server index page I have somethings important and I don't wont to throw it.
(I originaly thinked creating a new domain ipfspeer.jorropo.ovh with a cname to my server and use vhost, but without the dns<4|6> I can't do that).

So I think we should rewrite ws[s] addrs like that :

/<ip|dns><4|6>/<enter here domain or ip>/ws[s]/<enter here endpoint>

This just have one draw back, you can only register endpoint under / and not in ipfs/endpoint if you would due to MA limitation (except if you can escape / in ma, in this case I'm interested about that).

That should be very easy (that a line to change in the dial part and one line in the listen part).
But I can't actualy figure out how to register a protocol with a parameters, I have looked go-tcp-transport and I doesn't see any transport registering in the repo so I guess this one is special and preregistered.
And I also looked go-utp-transport, here I clearly see where the protocol is registered but I don't see somethings very different explaining how to pass a param.

So how can I add an string endpoint params to ws[s] and do a.ValueForProtocol(p[2].Code) ?

EDIT 1: Keeping old notation will not be possible anymore because if MA is followed by /ipfs/Qmxxxx libp2p will understand ipfs as endpoint and Qmxxx as protocol.
But I think that will not be so hard to change because that only concern connection addrs, that should automaticaly update in the DHT except for bootstrap but they are few so that shouldn't be a problem.

EDIT 2: A much simpler solution is to only allow announce with wss with go-libp2p and not connection and wait until a solution is decided on go-libp2p and go-libp2p-swarm about protocol needing dns<4|6>.
Because again that for browser not go-libp2p.

EDIT 3: We can also (and that probably the best choice but I don't think it will ever happend) try to lunch a debate in the web certification authority reponsible of mixed content about adding a way to disable it for very targeted things where it can't be made accident (fetch and ws).
Because we can next add ws default in the configuration (or just print a message to users) and have a giant number of browser to ipfs bridge.

@Jorropo Jorropo mentioned this pull request Jun 13, 2019
4 tasks
@albrow albrow mentioned this pull request Jan 24, 2020
@raulk raulk mentioned this pull request Jan 4, 2022
@raulk
Copy link
Member

raulk commented Jan 4, 2022

This PR attempted to take #15 over the finish line, but was ultimately superseded by #72, which 0x has deployed in production.

@raulk raulk closed this Jan 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants