Support ClientAuth (a.k.a 'stealth') on v3 onions with Tor 0.4.6.x

[mig5]

Closes #697
Closes #1112
Closes #1343
Closes #1396
Closes #1397

Good news! We forked upstream's Stem so that we could fix some issues in the setup.py to make it compatible with Poetry.

This means I can finally open my PR and have people test Client Auth (a.k.a stealth) with v3 onions!

Please note: Client Auth for v3 requires Tor 0.4.6.x on the OnionShare side. This is the major version that supports requesting ClientAuthV3 flag via the Tor control port when registering the onion service. It won't work on 0.4.5.x.

However, the other person running Tor Browser to access your onion service can be using Tor 0.4.5.x just fine, e.g (in the latest Tor Browser, which uses Tor 0.4.5.10).

On Debian 10, I was able to install Tor 0.4.6.7 with this as my apt torproject.list file:

deb https://deb.torproject.org/torproject.org buster main
deb-src https://deb.torproject.org/torproject.org buster main
deb https://deb.torproject.org/torproject.org tor-experimental-0.4.6.x-buster main
deb-src https://deb.torproject.org/torproject.org tor-experimental-0.4.6.x-buster main

Given that the latest version of Tor Browser still uses 0.4.5.10 at time of writing, so using TB as the Tor connection won't help you either. I guess this is less of an issue for our flatpak/snap packages which can build a newer version from source..

Please note: I haven't removed the 'basic auth' a.k.a 'use a password'. But I really if we should just do that, although it'll not be a backwards-compatible change e.g for persistent onion shares..

I also think that ClientAuth kind of makes #1343 unnecessary? We don't need to manage cookies instead of a password, if we have ClientAuth.. what do you think?

Turning on Client Auth is a setting in the share options in each tab ('Use client authorization'). When active, there'll be a button on an active share 'Copy ClientAuth' which contains the private key that is needed for the other user to paste into their Tor Browser dialog when prompted for it (or for configuring manually in your tor settings, for power users..)

Happy testing!

[micahflee]

This is amazing!

Given that the latest version of Tor Browser still uses 0.4.5.10 at time of writing, so using TB as the Tor connection won't help you either. I guess this is less of an issue for our flatpak/snap packages which can build a newer version from source..

This also isn't an issue for Windows and Mac, because we can just extract the tor 0.4.6.7 binary from Tor Browser 11.0a5. I'm fine with going forward and releasing this when it's done, and not worrying about Tor Browser's schedule.

Please note: I haven't removed the 'basic auth' a.k.a 'use a password'. But I really if we should just do that, although it'll not be a backwards-compatible change e.g for persistent onion shares..

Change: Let's make private OnionShare services just use v3 client auth, and remove the password altogether.

v3 client auth is more secure than the password (and will solve both #1396 and #1397), and it's redundant to use both. I'm not so concerned with backwards compatibility.

This means removing the --client-auth CLI flag and the "Use client authentication" checkbox, and changing the "Don't use a password" checkbox to say something like, "This is a public OnionShare service (disables client authentication)".

I also think that ClientAuth kind of makes #1343 unnecessary? We don't need to manage cookies instead of a password, if we have ClientAuth.. what do you think?

I agree. When this PR is finished it will also solve #1343.

I wanted to test error handling when my version of tor is out-of-date. With tor 0.4.5.6, when I try using client auth in CLI:

$ poetry run onionshare-cli --client-auth --receive
╭───────────────────────────────────────────╮
│    *            ▄▄█████▄▄            *    │
│               ▄████▀▀▀████▄     *         │
│              ▀▀█▀       ▀██▄              │
│      *      ▄█▄          ▀██▄             │
│           ▄█████▄         ███        -+-  │
│             ███         ▀█████▀           │
│             ▀██▄          ▀█▀             │
│         *    ▀██▄       ▄█▄▄     *        │
│ *             ▀████▄▄▄████▀               │
│                 ▀▀█████▀▀                 │
│             -+-                     *     │
│   ▄▀▄               ▄▀▀ █                 │
│   █ █     ▀         ▀▄  █                 │
│   █ █ █▀▄ █ ▄▀▄ █▀▄  ▀▄ █▀▄ ▄▀▄ █▄▀ ▄█▄   │
│   ▀▄▀ █ █ █ ▀▄▀ █ █ ▄▄▀ █ █ ▀▄█ █   ▀▄▄   │
│                                           │
│                  v2.3.3                   │
│                                           │
│          https://onionshare.org/          │
╰───────────────────────────────────────────╯

Connecting to the Tor network: 100% - Done
Your version of Tor is too old, stealth onion services are not supported

Traceback (most recent call last):
  File "/home/user/code/onionshare/cli/onionshare_cli/__init__.py", line 407, in main
    app.start_onion_service(mode, mode_settings)
  File "/home/user/code/onionshare/cli/onionshare_cli/onionshare.py", line 81, in start_onion_service
    self.onion_host = self.onion.start_onion_service(
  File "/home/user/code/onionshare/cli/onionshare_cli/onion.py", line 648, in start_onion_service
    raise TorTooOldStealth()
onionshare_cli.onion.TorTooOldStealth

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/user/code/onionshare/cli/onionshare_cli/__init__.py", line 413, in main
    print(e.args[0])
IndexError: tuple index out of range

Change: Properly handle the onionshare_cli.onion.TorTooOldStealth exception in the CLI.

The exception is handled great in the GUI.

When using a supported version of tor I think the UI in the CLI is good, but I think we should update the UI in the GUI. Here's what it looks like:

Change: I think the "Copy ClientAuth" button should be moved left, so it's next to "Copy Address".

Change: The instructions should change too to make it clear that both the address and the ClientAuth are needed.

Right now it says:

Anyone with this OnionShare address can join this chat room using the Tor Browser

Instead it should say this:

Anyone with this OnionShare address and ClientAuth can join this chat room using the Tor Browser

And the similar instructions for all the other modes should change too.

However, in public mode, the instructions should not mention the ClientAuth, and the text can stay the same for the instructions.

Here's what Tor Browser looks like asking for the ClientAuth:

Change: To make things clearer, we should use the same language as Tor Browser:

• In the CLI instead of saying, "Give this address and ClientAuth to the sender" it should say, "Give this address and private key to the sender"
• In the CLI instead of saying: "ClientAuth: [key]" it should just say, "Private Key: [key]"
• In the GUI instructions it should say, "Anyone with this OnionShare address and private key..."
• The GUI button should say, "Copy Private Key"

Change: With client auth, the QR code doesn't work anymore. It just includes the address but not the private key, and I don't think there's a standard way to include them in the same URL. Maybe a good solution is the "Show QR Code" button can actually show two QR codes, one for the address and one for the key, and they can be labeled.

Change: We'll need to rewrite a LOT of documentation, including the security design docs, and take new screenshots. If you want you can take a stab at it in this PR, or if you want you can open a separate issue for documentation and I can do it later.

[micahflee]

Should you make a 1.8.1 tag in the stem repo, and lock this to that tag, rather than to a branch?

[mig5]

Thanks. I think I have all those changes covered (except for the docs, but I'm happy to work on that later), care to review again when you can?

I'm not 100% happy with the QR codes, it seems as though the private key's one is always smaller than the URL - not sure how exactly to make them appear the same size.. maybe it's not a big deal..

Tests are failing on chat mode, I don't understand why.. I'm now annoyed at my computer so I'm gonna just walk away :) I'll have to come back to that another time.

Working on the tests has made me realise that we'll have to be super careful now because in local mode, there's essentially no difference between public vs non-public (other than I am setting a 'fake' client auth key so that the button appears in the GUI etc).. but we'll have to be careful to make sure that we don't do anything that 'breaks' the auth protection somehow. Still, I expect ClientAuth will be harder for us to break accidentally than with the HTTP auth stuff, since it's entirely delegated off to Stem/Tor.

It was nice removing a lot of code to do with rate-limiting/invalid passwords etc!

[mig5]

Found the test failure - I had forgotten to add a string to the locale for Chat Mode when using ClientAuth 🤦‍♂️

I might add more tests later to make sure we are testing that the Private Key button is not visible in public mode, etc.

[micahflee]

A few small things:

• Let's change, "This is a public OnionShare service (disables client authentication)" to "This is a public OnionShare service (disables private key)", to make it more clear
• The instructions strings for chat mode are backwards. When client auth is enabled it says, "Anyone with this OnionShare address can..." and when client auth is disabled it says, "Anyone with this OnionShare address and private key can..." -- these should be reversed. (They seem fine for share, receive, and website modes.)
• Make the QR code sizes match. I figured out how :)

diff --git a/desktop/src/onionshare/widgets.py b/desktop/src/onionshare/widgets.py
index ad54a22d..ad6b9272 100644
--- a/desktop/src/onionshare/widgets.py
+++ b/desktop/src/onionshare/widgets.py
@@ -139,6 +139,8 @@ class QRCodeDialog(QtWidgets.QDialog):
 
         self.qr_label_url = QtWidgets.QLabel(self)
         self.qr_label_url.setPixmap(qrcode.make(url, image_factory=Image).pixmap())
+        self.qr_label_url.setScaledContents(True)
+        self.qr_label_url.setFixedSize(350, 350)
         self.qr_label_url_title = QtWidgets.QLabel(self)
         self.qr_label_url_title.setText(strings._("gui_qr_label_url_title"))
         self.qr_label_url_title.setAlignment(QtCore.Qt.AlignCenter)
@@ -156,9 +158,15 @@ class QRCodeDialog(QtWidgets.QDialog):
 
         if auth_string:
             self.qr_label_auth_string = QtWidgets.QLabel(self)
             self.qr_label_auth_string.setPixmap(qrcode.make(auth_string, image_factory=Image).pixmap())
+            self.qr_label_auth_string.setScaledContents(True)
+            self.qr_label_auth_string.setFixedSize(350, 350)
             self.qr_label_auth_string_title = QtWidgets.QLabel(self)
             self.qr_label_auth_string_title.setText(strings._("gui_qr_label_auth_string_title"))
             self.qr_label_auth_string_title.setAlignment(QtCore.Qt.AlignCenter)
 
             auth_string_layout = QtWidgets.QVBoxLayout(self)

The reason they're different sizes though is because the address is longer than the private key, it contains more information, so it takes more blocks to encode. So when we resize them like this, the blocks in the address QR code are smaller than the blocks in the private key QR code. Still, I think it looks nicer this way.

[micahflee]

Also:

• Remove the flask_httpauth dependency, and rip out the HTTPAuth code from https://github.com/onionshare/onionshare/blob/develop/cli/onionshare_cli/web/web.py

[micahflee]

• The Web object doesn't need the REQUEST_INVALID_PASSWORD request type either now, or REQUEST_RATE_LIMIT

[mig5]

The Web object doesn't need the REQUEST_INVALID_PASSWORD request type either now, or REQUEST_RATE_LIMIT

Remove the flask_httpauth dependency, and rip out the HTTPAuth code from https://github.com/onionshare/onionshare/blob/develop/cli/onionshare_cli/web/web.py

I think I already removed these in this PR, in commit 0bf8f53

EDIT oh, maybe I didn't remove REQUEST_RATE_LIMIT but I think I did the others. Thanks

[mig5]

@micahflee right, I think all that is now addressed in the latest commits.

(I still don't know why my fork hides the Circle CI tests in PRs, but you can see the tests are passing here).

One thing I haven't done, that maybe you can tack on as a commit: the flatpak/snap yaml files will need the pynacl dependency, and probably the change to the Stem source location too to be from our git repo and 1.8.1 tag. I've not really done flatpak/snap stuff before so I wasn't sure if that was something you do manually or if you have some script that populates it based on the pyproject.toml or something.

I'll start working on docs but maybe that can be a separate PR as this one's getting pretty big.

BTW, perhaps also it's worth doing a 'squash' merge in case we need to revert this whole PR for some reason later..

[mig5]

I pushed some documentation updates in bca5bee after all. But there are probably screenshots that need updating too, which maybe you want to do later since yours tend to look smarter than mine on Qubes!

[micahflee]

This is great! I will work on a thorough edit of the documentation before we make the actual release, including taking new screenshots, but I think this is a great start.

And I don't think we'll want to revert this whole PR later for some reason. If we did decide to not rely on v3 client auth, it might make more sense to instead implement #1343, so I'm okay with not squashing merging.