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

Tor external process - Allows Trezor Suite use Tor from Tails OS #15642

Merged
merged 11 commits into from
Dec 16, 2024

Conversation

karliatto
Copy link
Member

@karliatto karliatto commented Nov 29, 2024

Description

This PR allows to run Trezor Suite with external Tor, in order to

  • allow it to work in operating systems like Tails.
  • allow custom configuration for bridges that are not develop into Tor in Trezor Suite so use can configure a custom one for their needs

Like every great power it comes with great responsibility, so when running Tor External user could do something wrong that ruins their privacy so user has to know what he is doing when using this feature.

How to test it:

  1. Enable Tor external experimental feature in Trezor Suite.
  2. Run Tor daemon in port 9050 - Not necessary in Tails since it is running by default - but you need to make sure the circuit are established - Tails will ask you for it when starting.
  3. Enable Tor in Trezor Suite.

When running and doing discover with a device connected you should be able to make sure that all the connection is going throw Tor by running watch -n 1 'lsof -i TCP | grep electron' and checking that there is not connection going to the open internet. You should see TCP connection to localhost:9050 and no connection to anything else that is not in localhost:xxxx.

Related Issue

Resolve #5819 After 2 years open. 🎉

@karliatto karliatto force-pushed the feat/tor-external-total branch from e58e29e to 41b74a6 Compare December 2, 2024 11:54
@karliatto karliatto changed the title wip wip: Tor external process - Allows Trezor Suite use Tor from Tails OS Dec 2, 2024
@karliatto karliatto force-pushed the feat/tor-external-total branch from 6c177a3 to 39fcd71 Compare December 2, 2024 14:31
@karliatto karliatto changed the title wip: Tor external process - Allows Trezor Suite use Tor from Tails OS Tor external process - Allows Trezor Suite use Tor from Tails OS Dec 3, 2024
@karliatto karliatto force-pushed the feat/tor-external-total branch 3 times, most recently from c5882bc to c6b8e46 Compare December 3, 2024 08:48
@karliatto karliatto marked this pull request as ready for review December 3, 2024 08:58
@karliatto karliatto force-pushed the feat/tor-external-total branch from c6b8e46 to cd010d9 Compare December 3, 2024 09:01
@karliatto karliatto added the build-desktop This will trigger the build of desktop apps for your PR label Dec 3, 2024
@karliatto karliatto force-pushed the feat/tor-external-total branch from cd010d9 to 0628e51 Compare December 3, 2024 12:51
Copy link
Contributor

@marekrjpolak marekrjpolak left a comment

Choose a reason for hiding this comment

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

Sorry, I haven't reviewed the most important parts yet, so I just put here some observations for now.

packages/request-manager/src/utils.ts Outdated Show resolved Hide resolved
packages/request-manager/src/controllerExternal.ts Outdated Show resolved Hide resolved
@karliatto karliatto force-pushed the feat/tor-external-total branch from 61c8bec to 8e2cbed Compare December 10, 2024 09:44
@karliatto
Copy link
Member Author

@marekrjpolak I had to rebase because new yarn version in develop was breaking the ci tests in this branch.

@karliatto
Copy link
Member Author

@marekrjpolak I added some commits to remove the experimental feature tor-snowflake since it has been decided to remove it in favor of tor-external, given the fact that with it the user can use whatever Tor bridge that they want and it does not require Trezor Suite to maintain it.

But the tor-external is the same, I have just removed snowflake stuff. And tested all the core functionalities and edge cases that I considered critical and they are OK.

Copy link
Contributor

@marekrjpolak marekrjpolak left a comment

Choose a reason for hiding this comment

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

Sorry for having to wait this long for my review.

  • Apart from some rather small comments, it looks good I believe
  • Politically, I don't think anything about removing Snowflake from Suite, so I'm willing to approve with or without it. 🤷
  • I still haven't tested it, so I'm gonna do it right now.

packages/suite/src/constants/suite/experimental.ts Outdated Show resolved Hide resolved
packages/request-manager/e2e/identities-stress.ts Outdated Show resolved Hide resolved
packages/request-manager/e2e/interceptor.test.ts Outdated Show resolved Hide resolved
packages/request-manager/src/controller.ts Show resolved Hide resolved
packages/request-manager/src/controllerExternal.ts Outdated Show resolved Hide resolved
packages/suite-desktop-core/src/modules/tor.ts Outdated Show resolved Hide resolved
@marekrjpolak
Copy link
Contributor

@karliatto it seems to work with external Tor for me 👍 . I'm sometimes able to break it by turning the external experimental feature off while Tor is on in Suite (so it probably doesn't start the internal Tor process?) but it's a corner case anyway, right?

@karliatto
Copy link
Member Author

@karliatto it seems to work with external Tor for me 👍 . I'm sometimes able to break it by turning the external experimental feature off while Tor is on in Suite (so it probably doesn't start the internal Tor process?) but it's a corner case anyway, right?

Yes, that is kind of corner case in my opinion. If you do that, you need to switch Tor off/on so it starts internal Tor process.

@karliatto karliatto force-pushed the feat/tor-external-total branch from df8ea40 to 977fe60 Compare December 16, 2024 17:20
@karliatto karliatto merged commit 147a20f into develop Dec 16, 2024
24 of 26 checks passed
@karliatto karliatto deleted the feat/tor-external-total branch December 16, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-desktop This will trigger the build of desktop apps for your PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trezor Suite not connecting from within Tails OS through TOR to Trezor Servers
2 participants