TorProcessManager: Check our permissions wrt Tor process

[kiminuo]

Fixes #9518

This is an idea how to fix the issue #9518. It's a bit hackish but I don't know about a better solution at the moment. Ideas are welcome.

Testing

On Windows:

• I have tested the PR by running Tor under admin account and starting WW by my non-priviledged account.
• I have tested starting WW to start a Tor process, terminating WW and starting WW again and it worked (with the running Tor process).

Clement is testing on macOS.

Not yet tested on Linux.

[turbolay]

This is not working if you start Wasabi on another account.
This line is throwing TorControlException: Cookie file doesn't exist:

WalletWasabi/WalletWasabi/Tor/TorProcessManager.cs

Line 124 in 27bd7c8

controlClient = await InitTorControlAsync(cancellationToken).ConfigureAwait(false);

It's because the tor related files are created at a different than expected location.

[turbolay]

@kiminuo this code is working (yours not):

diff --git a/WalletWasabi/Tor/TorProcessManager.cs b/WalletWasabi/Tor/TorProcessManager.cs
index bb4d519dd..16911b02f 100644
--- a/WalletWasabi/Tor/TorProcessManager.cs
+++ b/WalletWasabi/Tor/TorProcessManager.cs
@@ -120,6 +120,10 @@ public class TorProcessManager : IAsyncDisposable
 
                                if (isAlreadyRunning)
                                {
+                               
+                                       Logger.LogInfo($"Tor is already running on {Settings.SocksEndpoint}");
+                                       controlClient = await InitTorControlAsync(cancellationToken).ConfigureAwait(false);
+                                       
                                        // Tor process can crash even between these two commands too.
                                        int processId = await controlClient.GetTorProcessIdAsync(cancellationToken).ConfigureAwait(false);
                                        process = new ProcessAsync(Process.GetProcessById(processId));
@@ -128,8 +132,6 @@ public class TorProcessManager : IAsyncDisposable
                                        // Especially, we want to make sure that Tor is running under our user and not a different one.
                                        nint _ = process.Handle;
 
-                                       Logger.LogInfo($"Tor is already running on {Settings.SocksEndpoint}");
-                                       controlClient = await InitTorControlAsync(cancellationToken).ConfigureAwait(false);
                                }
                                else
                                {
@@ -180,7 +182,16 @@ public class TorProcessManager : IAsyncDisposable
                                else
                                {
                                        // If Tor was already started, we don't have Tor process ID (pid), so it's harder to kill it.
-                                       foreach (Process torProcess in Process.GetProcessesByName(TorSettings.GetTorBinaryFileName()))
+                                       Process[] torProcesses = Process.GetProcessesByName(TorSettings.GetTorBinaryFileName());
+                                       
+                                       // Tor was started by another user and we can't kill 
+                                       if (torProcesses.Length == 0)
+                                       {
+                                               setNewTcs = false;
+                                               exception = new TorControlException("Tor was started by another user and we can't use nor kill it");
+                                               throw;
+                                       }
+                                       foreach (Process torProcess in torProcesses)
                                        {
                                                try
                                                {

I think this nint _ = process.Handle; is now useless? As the exception is raised by InitTorControlAsync

[kiminuo]

We don't want to throw an exception doing dispose.

[kiminuo]

To mock it.

[kiminuo]

Clement: Thanks, very useful. I have modified it a bit more and added a test. Please test if you can.

I think this nint _ = process.Handle; is now useless? As the exception is raised by InitTorControlAsync

It works for the scenario with Tor being run under admin account on Windows and WW being run under a non-admin account. That's maybe not that common scenario but why not to cover it, I guess.

[kiminuo]

This is a key bit because on macOS it gives only yours processes, on linux it should be the same.

[molnard]

Before I review this: so with this, the user will get a crash report, that he will be able to see that somebody else running tor in a different user profile?

[kiminuo]

That's how it is supposed to work. Yes.

[molnard]

That's how it is supposed to work. Yes.

🙏 I will test it tomorrow, my mac is not here right now.

[turbolay]

tACK for specific case: Another user has wasabi (tor) started on macOS

I think this doesn't work in that case: Wasabi is launched using sudo.
With elevated right I think it would kill the tor process of the other Wasabi instance.
Not sure how big of a problem this is.

[molnard]

Not sure how big of a problem this is.

It is not.

[molnard]

tACK