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

[karliatto]

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.

• feat(node-utils): checkSocks5Proxy util - new utility function checkSocks5Proxy to make sure there is a socks5 proxy running in the port for External Tor.
• test(node-utils): checkSocks5Proxy tests - Testing new utility function checkSocks5Proxy
• feat(request-manager): support external Tor - refactor Tor controller and create new controller for external Tor.
• feat(suite-desktop-core): support external Tor - create new TorExternalProcess to allow handle external Tor and update the Tor module so it can change from bundled Tor to External Tor.
• feat(suite-desktop-api): support external Tor - new type in TorSettings useExternalTor
• feat(suite): external-tor experimental feature - Add External Tor to experimental features in Suite.
• Removing snowflake experimental feature in favor of external Tor since with it the user can configure whatever Tor bridge they want.

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. 🎉

[marekrjpolak]

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

[karliatto]

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

[karliatto]

@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.

[marekrjpolak]

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.

[marekrjpolak]

I wouldn't add new isDesktopOnly but use existing isDisabled (which seems unused right now) instead. Or the other way around, remove isDisabled? Anyway, the double filtering in Experimental.tsx seems unnecessary.

[karliatto]

That is true, I did not realized of that. Thanks
72259fa

[marekrjpolak]

Probably should be negated :)

[marekrjpolak]

This is much better, in my opinion. Just two things:

• timeout is a limit for one action attempt after which it throws instead, while gap is a delay between two consecutive attempts. It's perfectly fine to have both defined, I just want to make sure that it's what you want.
• I think there's no need for abortController check inside an action and aborting it from attemptFailureHandler, as the getIsStopped check could be done in the action itself, which makes attemptFailureHandler redundant.. If you're not against, I can try to propose an adjustment after the review.

[marekrjpolak]

Would this work for you?

        const checkConnection: ScheduledAction<boolean> = async () => {
            if (this.getIsStopped()) return false;
            const isConnected = await this.controlPort.connect();
            const isAlive = this.controlPort.ping();
            const isCircuitEstablished = this.getIsCircuitEstablished();
            // It is running so let's not wait anymore.
            if (isConnected && isAlive && isCircuitEstablished) {
                return true;
            }
            throw new Error('Tor not alive');
        };
        const params: ScheduleActionParams = {
            attempts: MAX_TRIES_WAITING,
            timeout: WAITING_TIME,
            gap: WAITING_TIME,
        };

[karliatto]

I understood the difference between timeout and gap, and think in this case both are necessary.

Regarding abortController if you want to propose and adjustment, it is welcome :)

[marekrjpolak]

I believe it could be enough to have two consts for two different processes here and select one or another based on useExternalTor, instead of finding in an array.

[karliatto]

Sure that works and simpler. 1d3da5a

[marekrjpolak]

@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]

@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.