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 Socks Proxy (for Tor/i2pd/Kovri) Support to Wallets #5090

Merged
merged 1 commit into from Mar 27, 2019

Conversation

Projects
None yet
6 participants
@vtnerd
Copy link
Contributor

vtnerd commented Jan 24, 2019

Allows wallets to connect to daemon over tor/i2pd/kovri. If proxy usage is enabled, the daemon address must be onion/i2p based or use --daemon-cert-file. The idea is to prevent root CA shadiness over these networks in addition to the authenticated/unencrypted problems. A root CA can be combined with an onion/i2p address (the former can definitely be signed by a few CAs) by using --daemon-address https://xyz.onion. Additionally, --daemon-address https://... and --daemon-cert-file will work with ipv4 or ICANN hostnames.

Will have to coordinate with @moneromooo-monero for the SSL changes. The first commit (for p2p tor/socks) will be rebased away once its merged.

@vtnerd vtnerd changed the title Add Socks Proxy Support to Wallets Add Socks Proxy (for Tor/i2pd/Kovri) Support to Wallets Jan 24, 2019

@vtnerd vtnerd force-pushed the vtnerd:feature/tor_rpc branch from d37ef70 to ff04506 Jan 25, 2019

@vtnerd

This comment has been minimized.

Copy link
Contributor Author

vtnerd commented Jan 25, 2019

Pushed some tests and rebased against the parent Tor PR.

@fuwa0529

This comment has been minimized.

Copy link
Contributor

fuwa0529 commented Jan 25, 2019

I think the restriction on domain names / CA is a bit too strict, or even irrelevant. It's always possible to ask the socks5 server to resolve the domain on behalf of the user, and assuming the socks5 server is doing the right thing, the user is just as safe as using an IP. I also might want to use a different socks5 server other then tor/i2p.

Is it possible to decouple the commit into smaller ones for educational purpose?

@vtnerd

This comment has been minimized.

Copy link
Contributor Author

vtnerd commented Jan 25, 2019

I think the restriction on domain names / CA is a bit too strict, or even irrelevant. It's always possible to ask the socks5 server to resolve the domain on behalf of the user, and assuming the socks5 server is doing the right thing, the user is just as safe as using an IP. I also might want to use a different socks5 server other then tor/i2p.

The socks server is always doing the domain resolution. If a user wants to use a socks proxy in some other context, they can download the certificate of the CA for that site and specify with --daemon-cert-file. This is a bit restrictive, but allowing all root CAs on outbound proxy connections seemed risky without some nudging.

Is it possible to decouple the commit into smaller ones for educational purpose?

There are already two commits - one from the PR for p2p+tor, and a second for socks rpc. Did you want this broken down further?

@fuwa0529

This comment has been minimized.

Copy link
Contributor

fuwa0529 commented Jan 25, 2019

Thanks for the explanation @vtnerd. I see now that the default is to not trust root CA, then this seems to be the best practice.

Yes, I was hoping the commits can be broken down further more, if possible?

@jtgrassie

This comment has been minimized.

Copy link
Contributor

jtgrassie commented Jan 29, 2019

@fuwa0529

Yes, I was hoping the commits can be broken down further more, if possible?

Why? The 2 commits are very logical units of work.

@fuwa0529

This comment has been minimized.

Copy link
Contributor

fuwa0529 commented Jan 29, 2019

@jtgrassie Maybe because I don't understand it, and was a bit frustrated about myself. But if you think these are of reasonable size, then sure I'll stop complaining.

m_ssl_stream.emplace(m_io_service);
m_ssl_stream->socket.next_layer() = connection.get();
m_deadline.cancel();
m_deadline.expires_at(std::chrono::steady_clock::time_point::max());

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jan 29, 2019

Contributor

Is this scheduling an event for essentially never ?

@vtnerd vtnerd force-pushed the vtnerd:feature/tor_rpc branch from ff04506 to 758cb58 Jan 29, 2019

@fluffypony

This comment has been minimized.

Copy link
Collaborator

fluffypony commented Mar 4, 2019

@vtnerd as soon as #4852 is rebased and merged I'll let you know, and this can be rebased and merged!

@MaxXor

This comment has been minimized.

Copy link
Contributor

MaxXor commented Mar 7, 2019

It is merged.

@fluffypony

This comment has been minimized.

Copy link
Collaborator

fluffypony commented Mar 14, 2019

@vtnerd please rebase

@moneromooo-monero moneromooo-monero referenced this pull request Mar 21, 2019

Open

IPv6 support #4851

@fluffypony

This comment has been minimized.

Copy link
Collaborator

fluffypony commented Mar 24, 2019

@vtnerd still waiting on a rebase :)

@vtnerd vtnerd force-pushed the vtnerd:feature/tor_rpc branch from 758cb58 to 749ddd6 Mar 24, 2019


An anonymity network can be configured to forward incoming connections to a
`monerod` RPC port - which is independent from the configuration for incoming
P2P anonymity connections. The anonymity network (tor, i2pd, kovri, etc.) is

This comment has been minimized.

Copy link
@jtgrassie

jtgrassie Mar 24, 2019

Contributor

Minor request, can we change "(tor, i2pd, kovri, etc.)" to simply "tor/i2p"? This keeps implementation names out.

This comment has been minimized.

Copy link
@vtnerd

vtnerd Mar 24, 2019

Author Contributor

Will update shortly.

This comment has been minimized.

Copy link
@jtgrassie

jtgrassie Mar 24, 2019

Contributor

Thanks

> HiddenServiceDir /var/lib/tor/data/monero
> HiddenServicePort 18081 127.0.0.1:18081
Then the wallet will be configured to use a tor/i2pd/kovri proxy and onion/i2p

This comment has been minimized.

Copy link
@jtgrassie

jtgrassie Mar 24, 2019

Contributor

Same as above comment.

@vtnerd

This comment has been minimized.

Copy link
Contributor Author

vtnerd commented Mar 24, 2019

@vtnerd still waiting on a rebase :)

Yeah, I got caught working on the SSL PR. This has reverted prior changes for SSL, and is rebased to mainline. @moneromooo-monero may want to review shortly since it technically has changed, although the "new" changes are primarily just reverting changes that were previously made.

@vtnerd vtnerd force-pushed the vtnerd:feature/tor_rpc branch from 749ddd6 to 29c97d2 Mar 24, 2019

@vtnerd vtnerd force-pushed the vtnerd:feature/tor_rpc branch 2 times, most recently from 0ad6651 to 7acfa9f Mar 25, 2019

@vtnerd

This comment has been minimized.

Copy link
Contributor Author

vtnerd commented Mar 25, 2019

Ok, finally rebased entirely (hopefully) with @jtgrassie changes.

@fluffypony
Copy link
Collaborator

fluffypony left a comment

Reviewed

@fluffypony fluffypony merged commit 7acfa9f into monero-project:master Mar 27, 2019

3 of 10 checks passed

buildbot/monero-linux-armv7 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-static-win64 Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
buildbot/monero-linux-armv8 Build done.
Details
buildbot/monero-static-osx-10.13 Build done.
Details
buildbot/monero-static-ubuntu-amd64 Build done.
Details

fluffypony added a commit that referenced this pull request Mar 27, 2019

Merge pull request #5090
7acfa9f Added socks proxy (tor/i2pd/kovri) support to wallet (Lee Clagett)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.