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.

Sign up for GitHub

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

Jump to bottom

Support for Tor onion services #736

Merged
merged 52 commits into from
Feb 8, 2019
Merged

Support for Tor onion services #736

merged 52 commits into from
Feb 8, 2019

Conversation

rorp
Copy link
Contributor

@rorp rorp commented Oct 17, 2018

No description provided.

@pm47
Copy link
Member

pm47 commented Oct 18, 2018

Awesome. Haven't looked at it yet, but it's great to have someone working on this!

@pm47 pm47 mentioned this pull request Oct 22, 2018
Closed
@rorp rorp changed the title [WIP] Tor hidden service Tor hidden service Nov 15, 2018
pm47 added 8 commits January 31, 2019 17:10
@pm47
renamed file tor_pk->tor.dat for consistency
3cf82a6
@pm47
moved constants to companion object
c57eba1
@pm47
only support v3 hidden service
bcc3d53 
Eclair can connect to v2/v3, but will only open a v3 service. Instead of
trying our best to be compatible, we just require a minimal version for
the tor daemon.

The rationale for this change is that it allows for a significant
simplification, and is recommended in the Tor documentation:

> Since Tor 0.3.2 and Tor Browser 7.5.a5 56-character long v3 onion
addresses are supported and should be used instead.

source: https://www.torproject.org/docs/tor-onion-service.html.en#four
@pm47
replacing auth SAFECOOKIE->PASSWORD
5e8d573
@pm47
use java.nio.Path instead of String
f32334e
@pm47
put socks proxy parameters in NodeParams
5eba5db
@pm47
rework of socks5 client
0f8d898 
Instead of doing the error management ourselves, use the let-it-crash
principle and let the supervisor handle and log failures.
@pm47
using TestUtils.BUILD_DIRECTORY everywhere
9602ea4
pm47
pm47 requested changes Feb 3, 2019
View reviewed changes
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed and committed changes on branch tor_hidden_service_pm.

Main changes:

  • Removed support for tor v2 hidden services (bcc3d53), because they are not recommended in Tor documentation. I'm curious to hear your rationale for supporting them.
  • Replaced SAFECOOKIE by PASSWORD authentication (5e8d573). Implementation is simpler, allows for an easier setup on windows, and makes it possible to have a separate tor server. I'm interested in your thoughts on this.

I noticed that you use java-style exception management in actors, I converted it to something more akka-esque (let it crash + supervision strategy).

Also, please confirm that I can apply the Apache 2 License to your files.

With those changes I think this can be merged quickly.

@rorp
Copy link
Contributor Author

rorp commented Feb 3, 2019

  • Removed support for tor v2 hidden services (bcc3d53), because they are not recommended in Tor documentation. I'm curious to hear your rationale for supporting them.

For example, supported Tor version in Ubuntu 18.04.1 LTS is 0.3.2.10, which is pre-v3. It's even earlier in raspbian. It's always easy to drop v2 support when v3 is enabled in most Linux distributions.

  • Replaced SAFECOOKIE by PASSWORD authentication (5e8d573). Implementation is simpler, allows for an easier setup on windows, and makes it possible to have a separate tor server. I'm interested in your thoughts on this.

Safe cookie authentication doesn't require any additional configuration on Mac OS X and minimal configuration on Linux (adding eclair user into tor group). I don't have Windows, but I believe the configuration should be similar to Mac OS X. The advantage in password authentication is the possibility of having a separate Tor server, though. So why not have them both? The code is already there.

I noticed that you use java-style exception management in actors, I converted it to something more akka-esque (let it crash + supervision strategy).

👍

Also, please confirm that I can apply the Apache 2 License to your files.

Yes, sure

@rorp
Copy link
Contributor Author

rorp commented Feb 3, 2019 •
edited
Loading

Also safe cookie authentication is way more secure, because the secrets are not stored anywhere and the cookie file gets updated all the time. Bitcoin Core is going to use a similar authentication technique for its RPC because of that fact.

Another point is that bitcoind uses safe cookie to create a hidden service as well, and even automatically connects to Tor if safe cookie authentication is enabled in Tor config. In case of safe cookie authentication the end users won't need to have separate Tor configurations for bitcoind and eclair.

@pm47
Copy link
Member

pm47 commented Feb 5, 2019

Those are very valid points, I added back onion service v2, and support for both SAFECOOKIE and PASSWORD authentication mechanism.

I simplified further the code, added licenses and fixed an issue with peer address storage (we need to store onion addresses). Still need to rework the doc.

Took the liberty to merge all this on this branch.

@pm47 pm47 changed the title Tor hidden service Support for Tor onion services Feb 7, 2019
@pm47 pm47 merged commit 808bf14 into ACINQ:master Feb 8, 2019
pm47 added a commit that referenced this pull request Mar 25, 2020
@pm47
fixup! Support for Tor onion services (#736)
5c2a120
pm47 added a commit that referenced this pull request Mar 26, 2020
@pm47
fixup! Support for Tor onion services (#736) (#1356)
0064aae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sstone sstone sstone left review comments

@pm47 pm47 pm47 approved these changes

Assignees

@pm47 pm47

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rorp @pm47 @sstone