-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(transport/websocket): support SOCKS proxy with wss #3137
Conversation
lgtm 👍 I guess setEnv doesn't work in CI? |
I didn't really debug, but SetEnv might be surprising if other tests are run in parallel. |
huh I was fairly certain that SetEnv only affects the current process and children, though I guess the tests may run in the same process as separate goroutines? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it's worth it to add a dependency on a socks5 server to test this end to end?
wsurl.Host = sni + ":" + wsurl.Port() | ||
// Setting the NetDial because we already have the resolved IP address, so we can avoid another resolution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel parseMultiaddr should handle this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR didnt introduce this, so it's fine to leave this as is.
I am going to make a test build of singularity against this for a functional test and report tmw. |
Not really. The fact that it hand shakes with the socks5 should be enough. If it doesn't work from there the issue is in the websocket library, not our setup. |
ok, so far I can say that we are able to build latest singularity against this commit and that many tools e.g. vole seem to pick it up correctly I cannot verify that it's working correctly with singularity dealpusher just yet, however I don't think that should stop merging as any issues are unlikely to be w libp2p at that point |
hmm actually can't verify correct behavior w/vole, hold please |
ok the behavior I am observing is that using HTTPS_PROXY specifically appears to work against a ws remote, while HTTP_PROXY does not; this doesn't seem quite right to me but I will take it I don't currently have a wss remote to test against but will work on that |
- pull in commit from libp2p/go-libp2p#3137 - update related dependencies
OK I can't say that this 💯 works just yet because we're getting proxy errors, but the fact that proxy use is attempted is a big win and the issue is probably with allowlists on our side:
draft PR for singularity here data-preservation-programs/singularity#450 |
I can make this work with HTTP_PROXY and Just curious, what is the reason you are using |
@MarcoPolo client is inside a secure network where all connections to the outside must go through SOCKS and only to allowlisted hosts -- this is actually not entirely uncommon in "enterprise" environments though it is a huge pain for a |
Testing this locally with vole using this branch of go-libp2p I was able to connect to your endpoint using
|
5843897
to
bddc624
Compare
This may not be a problem for your use case, but are you aware that the WebSocket protocol will mask all the application data you send it in order to protect against faulty intermediaries? This may have a noticeable performance impact as it introduces another copy of all data and a non-zero overhead for the masking operation. See https://datatracker.ietf.org/doc/html/rfc6455#section-5.1 for more details. Could you proxy TCP traffic through your SOCKS proxy? If so, we could add something like this to go-libp2p to support a |
@MarcoPolo we're only sending deal proposals so the payload size is trivial, using websockets from that perspective is fine separately I am finding that our proxy may not actually even have a limited SOCKS implementation... so 🤦 |
thank you! I am going to work on getting our proxy to... not suck and ideally have proper SOCKS transports so the normal tcp connections can also traverse it as in your example |
ok, upon review it seems that we may be able to get raw tcp through the proxy after all (once underlying blockage is fixed), which will help us avoid all the ws rough edges... is it possible to ship the TCP_PROXY suggestion @MarcoPolo? (the HTTP(S)_PROXY fix will actually matter for us elsewhere so that effort is much appreciated) EDIT: yes, we have confirmed:
so, if it's reasonably possible to use e.g. https://github.com/mwitkow/go-http-dialer the current use case can be fully supported, otherwise I will have to negotiate to rebuild squid with socks (which, in fairness, was the original problem statement so that could be a reasonable compromise) |
I think what makes the most sense is to allow a user to set a custom net.Dialer. Then they can use whatever custom logic they want. This would let you use https://github.com/mwitkow/go-http-dialer without tying the library to that specific case. |
Sounds like a good idea, it should allow us to pass in a http proxy supporting dialer. If we can ship TCP_PROXY support first I can get moving with an interim solution (like using a danted socks->http proxy chain shim) |
closes #286 (a 7 year old issue??)
This PR allows users to use a SOCKS5 proxy when dialing
/wss
addresses by using theHTTPS_PROXY
environment variable.Here's an example using vole to dial through a socks proxy: