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

adding bridge support to tor-container #74

Merged
merged 5 commits into from
Sep 18, 2019
Merged

adding bridge support to tor-container #74

merged 5 commits into from
Sep 18, 2019

Conversation

nickodev
Copy link
Contributor

@nickodev nickodev commented Sep 2, 2019

The relay network is censored in some places and we need to be able to use bridges to overcome this issue. my approach was adding a template file for torrc (called torrc.tpl) and a config file (called docker-tor.conf.tpl) for the user to add their bridges' addresses.
In both install-script.sh and upgrade-script.sh we will copy the torrc.tpl then append the user settings in docker-tor.conf.tpl to make the main torrc file.
Suppose user wants to disable or change the bridges, it could be simply done via dojo.sh upgrade command.

I have tested both installation and upgrade process and they both work fine. documentation needs more work I believe. what do you think?

@kenshin-samourai kenshin-samourai added enhancement New feature or request my-dojo Something related to my-dojo labels Sep 2, 2019
@kenshin-samourai
Copy link
Contributor

thanks for this pull request. much appreciated.

starting from your code, i've worked on a "revamp" implementing a more radical approach:

  • torrc file is removed and replaced by command line arguments in /docker/my-dojo/tor/restart.sh
  • docker-tor.conf file is restructured (same format as others config files)

it will provide a few benefits:

  • homogeneity of the format used by the config files
  • activation of modifications made in the config will require a simple restart of Dojo instead of a full upgrade
  • use of command line arguments instead of static torrc will make easier to implement new optional Tor features in the future

i've submitted a pull request to your repo with all these modifications. feel free to merge it if it's fine for you. then we'll merge your updated pull request into this repo.

@nickodev
Copy link
Contributor Author

nickodev commented Sep 6, 2019

Thank you for the "revamp" I really like the new approach specially the part that applies the new config without a full upgrade, that is perfect ux wise.
I have a question, in revamped docker-tor.conf.tpl we are now adding bridges info as separated parts(ip port cert mode), isn't it possible to have them in a single line like what I have proposed?

@kenshin-samourai
Copy link
Contributor

it's just a personal preference for config properties as simple as possible (semantically speaking).

anyway, i don't have strong feelings against a single config property per bridge (the connection strings are generated on a website managed by the torproject. thus we can reasonably expect that these strings continue to allow direct copy/pastes in tor config).

if you think that it would improve the ux, i can submit a new pull request implementing a single config property per bridge.

@nickodev
Copy link
Contributor Author

nickodev commented Sep 8, 2019

if you think that it would improve the ux, i can submit a new pull request implementing a single config property per bridge

I believe having a single line per bridge would be more intuitive for the users, if you agree let's make it that way.
I would do it myself but I really like your coding style and I want to learn 👍

@kenshin-samourai
Copy link
Contributor

done (commit d901b61)

* replace torrc file by command lines arguments set in restart script.
restructure docker-tor.conf for homogeneity.
define env vars in Dockerfile for easier tuning (for pi4 and others platforms)

* bump version of tor image
* update doc
* merge tor bridges config options
@burcakbaskan
Copy link

A most welcome addition, however, there's one problem. Tor is still being built by cloing the git repo at torproject.com. If access to torproject.com is blocked, this doesn't work. Alternative would be to download the source from a website clone. For example: https://www.theonionrouter.com/tor-0.3.5.8.tar.gz

If the script were changed to pull the source tar instead of cloning from git a potential blocked user could edit the dockerfile and enter a clone he has access to.

@kenshin-samourai
Copy link
Contributor

@burcakbaskan good point
i'm going to open a new issue for this

@nickodev
Copy link
Contributor Author

@burcakbaskan good observation. I have my friend test the installation from a place which not only has tor project censored but also restricted by docker from getting the docker and docker-compose due to u-s sanc tions. the easy solution for them is to use vpn during the setup. but they want to to be able to disconnect vpn after installation that is where the bridge idea came to my mind and I believe if someone is competent enough to install the dojo himself can handle the restricted access easily via vpn.
Also changing the url may not be effective since the new address could get restricted too after awhile

@burcakbaskan
Copy link

I agree and I normally use VPN to bypass restrictions. However, having built-in alternatives is a safer approach. Our government blocked all OpenVPN based ISPs during protests at one point and some of the more well-known VPNs are still blocked. Also, you might be using VPN on other devices and simply hit the VPN connection limit, etc. Seriously though, thank you to both you for bringing this up and to kenshin-samourai for acting on it. I've been preaching better tor support to every developer I meet and Samourai has been the first one to act on this.

Copy link
Contributor

@LaurentMT LaurentMT left a comment

Choose a reason for hiding this comment

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

ACK
Tests done:

  • connection to Tor network through the bridges
  • install of Dojo from scratch
  • upgrade of Dojo

@kenshin-samourai kenshin-samourai merged commit 98ca22f into Samourai-Wallet:develop Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request my-dojo Something related to my-dojo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants