Add Socks v5 support to daemon and wallet #9443
Conversation
|
Crap, looks like older versions of boost don't have some ASIO functionality I used. Crap, will have to update. |
|
What would be the minimum Boost version for this PR? Is it time to undust #9162 and bump the minimum Boost requirement? |
|
The minimum version for this feature would be 1.67. In this case, its not really critical, I should be able to work-around it. |
|
After many years, IPv6 has started to see substantial, and increasing, adoption, so I think this would be a reasonable addition. However, it's not urgent, and, based on the comments in #9162, I think you could safely assume Boost 1.67 (Ubuntu 18.04 continues to wither) and roll out this feature after a boost version upgrade to 1.67. Of course, you can do the extra work to support the current Boost version, but you may very well have better uses for your time. |
|
I just pushed a change to fix the Boost issues. Everything looks good for that. The functional tests failed, but it looks spurious right now. Testing locally to confirm, and may just push a dummy update to re-run the tests. |
|
Good for review! |
|
Closes #9443 |
|
Closes #9390 ? |
|
This PR needs rebased, and hopefully moves forward. Will be nice to support IPv6. |
vtnerd
force-pushed
the
feature/socks5
branch
from
64a613e to
cef3877
Compare
|
Force Push:
|
vtnerd
force-pushed
the
feature/socks5
branch
from
cef3877 to
ba2f8a4
Compare
|
Force pushed a size check fix. |
|
@vtnerd would it make sense to revert this change, since boost on master and release has been updated to 1.67+? |
|
Yes, looks like this is no longer needed, so I will try to revert that change. |
|
Force pushed a change to revert the workaround for older Boost versions. I'm not sure if this is a good idea, as officially the last supported version is Boost 1.66 @selsta , what do you think? |
|
Nevermind, I just checked again and this code should work with Boost 1.66+ so everything should be good. |
Also, i think we're raising min to 1.69 |
|
@nahuhh we will use 1.69 for release binaries but ideally work around the boost bug so we can keep a lower minimum |
|
I just verified this still compiles with Boost 1.66-1.87, so no issues with that. |
|
Bump. There's nothing preventing merging this? |
A bug |
which bug? |
|
@SyntheticBird45 the one vtnerd said he found and needed to fix/repush (and you + i said no rush) |
I don't remember. That was probably hacker impersonating my account. It happened approximately every 1/2 messages. |
|
Fixed the bug. I thought this was going to get merged first, but it looks like a re-review will be needed. I did NOT rebase, as this would make it harder to review. |
|
The commit message incorrectly said socks v4, corrected to v5. |
|
I guess I need to rebase, as the build is busted at this commit level. :/ |
|
What's going on? I got an email stating the build failed, but this is showing everything passed? |
|
Sorry for the spam. Everything looks good. A separate CI run on my private repo triggered, and that repo was outdated compared to this one. So the email was from my private repo, which I didn't even realize would run separately. How annoying. |
Socks v5 adds IPv6 support and basic username/password authentication. The user/pass authentication is a cheap way to prevent rogue applications from requesting proxied connections. Tor and SSH do not support authentication but I2P, Nym, and Dante do.
When I started this implementation, I didn't know Nym added Socks v4 support. This makes the patch somewhat less useful, as the original #8562 Socks v5 request was for Nym support. This patch might want to be rejected on that reason alone - the only useful features are user/pass and IPv6.
This includes a fairly extensive explanation of proxies in the daemon and cli wallets, so that might be useful for keeping regardless.