Add onion client auth v3 support

[mig5]

Hi @atagar

We at OnionShare are excited to be working on support for adding Client Auth to v3 onions. For that we needed Tor to support it via the control port (which it mostly does now, as of 0.4.6.1-alpha (https://gitlab.torproject.org/legacy/trac/-/issues/30381) - and then of course we need Stem's create_ephemeral_hidden_service() to support sending the arg/flag too.

This PR (hopefully) adds support for adding Client Auth to v3 onions

Since the terminology is a bit confusing and although we use the word 'client' for adding auth even to a service (not just as a Tor client), I have named the new version check ONION_SERVICE_AUTH_ADD to differentiate the ONION_CLIENT_AUTH_ADD which is for adding auth client-side.

I see that the code has substantially changed since 1.8.0. I originally patched a copy of 1.8.0 which we are using in OnionShare, but then reworked it to suit what I am guessing is an upcoming 2.0.0...

I hope the tests are ok as well. Since Tor 0.4.6-alpha-1 drops support for v2 onions, I added a new test.requireversion_older_than() to skip tests that are for v2 only, so that running python3 run_tests.py --integ works successfully on the nightly Tor version (I am using 0.4.7.0-alpha-dev installed from the nightly Tor Debian repository)

I couldn't seem to run Stem master branch inside onionshare, maybe because there are other breaking changes as part of your post-1.8.0 work, I'm not sure. So, I've not literally tested this patch, but the tests themselves succeed..

Also please note, in stem/response/add_onion.py, I didn't bother setting an attribute for the ClientAuthV3 because what is returned from the control port is just the public key we've already sent as part of the request. E.G this works differently to BasicAuth on v2 onions, where Tor would send us the password that we need to know for using it. Since we already know the public key when adding client auth for v3 onions, I didn't see value in adding self.client_auth_v3 in the response, but let me know if you think otherwise.

Final note: there is a bug in Tor core right now that means that even though the control port accepts ClientAuthV3 and the V3Key flag in ADD_ONION, Tor is not actually adding the clientauth to the onion service it creates - meaning that the auth is not enforced when trying to visit the Onion Service. I have reported this separately, see:

https://lists.torproject.org/pipermail/tor-dev/2021-May/014548.html https://gitlab.torproject.org/tpo/core/tor/-/issues/40378 https://gitlab.torproject.org/tpo/core/tor/-/merge_requests/374

Hopefully when that fix gets merged in, we can actually not only test that we can add client auth via ADD_ONION with Stem, but that the onion service itself requires the client auth we set.

Still feel like a novice with Stem contributions, so if you think this PR needs reworking, I'm happy to try and fix it..

[atagar]

Still feel like a novice with Stem contributions, so if you think this PR needs reworking, I'm happy to try and fix it..

Thank you mig5, this patch looks perfect! I particularly appreciate the tests - thanks!

what I am guessing is an upcoming 2.0.0

I left Tor last month so sadly Stem's 2.x branch won't come out. If you send me a pull request for this applied against the 1.8.0 tag I'd be happy to make a maintenance branch off that if it's helpful to OnionShare.

[mig5]

@atagar

I left Tor last month so sadly Stem's 2.x branch won't come out. If you send me a pull request for this applied against the 1.8.0 tag I'd be happy to make a maintenance branch off that if it's helpful to OnionShare.

Ahh, sorry to hear! Thanks for all your work over the years!

I'll send another PR for 1.8.x shortly. Thanks for merging this in, I didn't expect it to be OK the first time :)

I wonder what that means for Stem going forward, I guess this is a question for the Tor project itself now rather than yourself of course.

[atagar]

Thanks for all your work over the years!

Thanks mig5! I really appreciate it. I'll still be around for questions or to merge pull requests like this. Working on Stem got lonely. I simply moved on to Wikipedia for my proactive volunteer work. :)

I wonder what that means for Stem going forward, I guess this is a question for the Tor project itself now rather than yourself of course.

The plan as I understand it is that Georg (GeKo) will own the maintenance branch off 1.8. In practice I wouldn't expect anything to happen unless necessary for a TPI initiative.

[mig5]

@atagar sorry to bother you. Thanks again for the 'maint' branch.

OnionShare uses Poetry for managing python dependencies, and I've been trying to get it to pull Stem in from the maint branch e.g from git directly (rather than from Pypi as it currently does), but I get an error to do with version parsing that can be read about in Poetry: https://github.com/python-poetry/poetry/issues/2176

I see two possible options:

1) Make setup.py work for Poetry in terms of version parsing (don't have it execute stem.__version__, - a slight loss of DRY principles), so that Poetry can pull the repo via the git branch, or

2) Actually make a 1.8.1 maintenance release on pypi (this may have been your intention all along)

Option 2 feels like a faster/simpler solution requiring less changes - just making a new point release. Is there any way I can help with that, if you don't have time? Are there other things you wanted to get into the maint branch (perhaps the 'other side' of ClientAuth e.g for adding auth a client connection to a remote onion service, or is that too big a change?) ?

Thanks!

[atagar]

Hi mig5. I don't use Poetry but from the docs it seems you can provide a git url and reference identifier.

https://python-poetry.org/docs/dependency-specification/#git-dependencies

Have you tried the following?

[tool.poetry.dependencies]
stem = { git = "https://git.torproject.org/stem.git", branch = "maint" }

[mig5]

Thanks @atagar . Unfortunately it gives the same result:

[RuntimeError]
Unable to retrieve the package version for /tmp/pypoetry-git-stem4g5w5m_4

What I think happens either with the CLI add tool or via this method, is Poetry attempts to parse the version from the setup.py, but it is not in a position to execute your stem.__version__ (which is how you've defined the version attribute in your setup.py) so it simply bails, unfortunately.

[mig5]

Ah no, I take it back! it may have actually worked despite the exception. Further down I then see this:

Collecting stem@ git+https://git.torproject.org/stem.git@maint
  Cloning https://git.torproject.org/stem.git (to revision maint) to /tmp/pip-install-f7jbuhx4/stem_95e3f27b68174e2ebcdea1ebe54fa489
  Running command git clone -q https://git.torproject.org/stem.git /tmp/pip-install-f7jbuhx4/stem_95e3f27b68174e2ebcdea1ebe54fa489
  Running command git checkout -b maint --track origin/maint
  Switched to a new branch 'maint'
  Branch 'maint' set up to track remote branch 'maint' from 'origin'.

Leave it with me then :)

EDIT ah, well it works, but it still complains on any subsequent poetry update style command, which will be annoying in the long term..

(venv) user@onionshare:~/git/onionshare/cli$ poetry update
Updating dependencies
Resolving dependencies... (9.7s)

[RuntimeError]
Unable to retrieve the package version for /tmp/pypoetry-git-stem2ssdoe29

There are other knock-on effects too:

Building wheels for collected packages: stem
  Building wheel for stem (setup.py) ... done
  Created wheel for stem: filename=stem-1.8.0_maint-py3-none-any.whl size=436387 sha256=6815fead89f04e044b92e6f0a6e36194ee0af88e7756ec6d650ff50e1fa9b5e2
  Stored in directory: /tmp/pip-ephem-wheel-cache-mg6ezww8/wheels/96/ad/1b/53138c38a40d28d566c20c5f464513748e2224350170ede135
  WARNING: Built wheel for stem is invalid: Metadata 1.2 mandates PEP 440 version, but '1.8.0-maint' is not
Failed to build stem
Installing collected packages: stem, onionshare-cli
  Attempting uninstall: stem
    Found existing installation: stem 1.8.0
    Uninstalling stem-1.8.0:
      Successfully uninstalled stem-1.8.0
    Running setup.py install for stem ... done
  DEPRECATION: stem was installed using the legacy 'setup.py install' method, because a wheel could not be built for it. A possible replacement is to fix the wheel build issue reported above. You can find discussion regarding this at https://github.com/pypa/pip/issues/8368.
Successfully installed onionshare-cli-2.3.2 stem-1.8.0-maint

Ultimately a point release via Pypi will be the most clean solution, if it can somehow be done..

[atagar]

Hi mig5. I'm unfamiliar with those poetry warnings but the first...

Metadata 1.2 mandates PEP 440 version, but '1.8.0-maint' is not

Is complaining about the version format. We've always used [version]-dev for the git repository so [version]-maint meets our convention. If Poetry rejects this due to 'Metadata 1.2' then maybe you can change that?

Ultimately a point release via Pypi will be the most clean solution, if it can somehow be done..

Our release process includes involvement by a half dozen package maintainers. I don't plan to issue a release for this.

[mig5]

@atagar ok thanks. I think most of those were just warnings in the end that we can ignore.

Ultimately the situation is this:

1) We can use the stem = { git = "https://git.torproject.org/stem.git", branch = "maint" } in pyproject.toml and poetry build will happily generate a wheel

2) Running pip install [path to wheel] seems to happily then clone stem from the maint branch. By doing this manually, we can actually use it.

3) Running poetry install or poetry update or basically any command other than poetry build sadly fails because of the version attribute in stem's setup.py not being a simple string . We use these poetry commands a fair bit to keep our dependencies up to date, but the version issue in stem's setup.py basically kills it.

It's 3) that is a bit of a nuisance for using Stem with Poetry. It would not really be as big an issue if Poetry moved on to install other dependencies, but it's a hard fail. That's the issue described in https://github.com/python-poetry/poetry/issues/2176

Setting the version to '1.8.0-maint' in the setup.py in maint branch might be enough (if Poetry is even happy with that syntax - not sure), but it's asking you to do something you probably don't want to do :)

I will have to leave it with @micahflee as to what to do. Perhaps we could even end up forking Stem and maintaining a pip package of it under a different name for our own purposes (I don't think anyone wants to really do this though)

Thanks again for your help and quick responses.