Add v3 onion support to tor-hidden-services ansible role

[kushaldas]

Status

Ready for review

Description of Changes

Fixes #4628
Fixes #4667

Started adding related Ansible changes to have v3 onion services (including the authenticated ones).

Testing

v3 only

• Fresh install
• Rerun ./securedrop-admin install on a already installed system.

For both the options above, make the following two changes in install_files/ansible-base/group_vars/all/site-specific

v2_onion_services: false
v3_onion_services: true

For this case:

• No v2 address working and or in the system
• v3 source address works
• v3 journalist interface works (follow https://kushaldas.in/posts/setting-up-authorized-v3-onion-services.html about setting up Torrc)

v2/v3 alongside

v2_onion_services: true
v3_onion_services: true

For this:

• All old v2 address working and are in the app and mon servers
• v3 source address works
• v3 journalist interface works (follow https://kushaldas.in/posts/setting-up-authorized-v3-onion-services.html about setting up Torrc)

v2 only

There is also the third point to test which is not adding v2_onion_services or v3_onion_services to site-specific and instead the default values (v2 on, v3 off).

This should keep the current v2 systems as it is, and no v3 details should be generated in the servers.

Deployment

Any special considerations for deployment? Consider both:

1. Upgrading existing production instances.
2. New installs.

Checklist

If you made changes to the server application code:

• Linting (make lint) and tests (make -C securedrop test) pass in the development container

If you made changes to securedrop-admin:

• Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

• Configuration tests pass

If you made non-trivial code changes:

• I have written a test plan and validated it for this PR

If you made changes to documentation:

• Doc linting (make docs-lint) passed locally

[eloquence]

(Since it's a draft PR and cannot be merged anyway, I've removed "DO NOT MERGE" from the PR title)

[kushaldas]

While working on tailsconfig I figured one issue with the current logic.

In v3, we have to create at least 2 different set of public/private keys, one for the journalist UI, and the other for ssh service if required.

@redshiftzero Is it okay if I use same set of keys for both app and mon servers, or should we create 2 different sets for both the servers?

[redshiftzero]

@kushaldas I think we should use separate keys where possible, so separate keys for the SSH services on app and mon

[conorsch]

Can you provide a test plan, @kushaldas? I haven't yet tested in VMs, only provided a visual review as a first pass. Take a look at the duplication and see if you can reduce it, based on the previously implemented v2 logic. Happy to lend a hand or collaborate directly on this branch if you'd prefer.

[conorsch]

Just as we currently have two discrete files on the Admin Workstation for the v2 auth info, it'd be great to have the same for the v3 info. Doing so would simplify the lookup logic required in the dynamic-inventory script (not yet implemented in this PR), as well. Perhaps the function should receive hostname=None, and select the files that way.

[kushaldas]

I feel that one file for admin workstation with all 3 different key pairs (journalist, ssh app, and ssh mon) and a file for journalist workstations (only with the journalist keypair) is simpler. Also, this is only for the key information, if we want, we can also remove the ths files all together and just the .onion address from the JSON files iteself (we have to add them there).

[conorsch]

Having taken a closer look at the keygen logic, we're still going to need separate files for the dynamic-inventory support (OK to defer to #4629). With the v2 logic, if-and-only-if the -ths files are present on the Admin Workstation, then Onion URLs are used for the SSH connection. Since those -ths files are created on the Admin Workstation at the end of the install process, the logic is sound. For the v3 keygen logic, however, we need a way to denote whether the corresponding Onion URLs should be used for the SSH connection.

I suggest moving the keygen logic into Ansible via a script, so that the JSON file does not exist when Ansible builds the SSH connection, meaning local IPv4 addresses (or v2 Onion URLs) will be used. At the end of the run, we can sprinkle in appropriate -v3-ths files back onto the Admin Workstation.

We cannot relegate the v3 keygen logic solely to the securedrop-admin init, because that's only relevant for Tails environments, which means developer staging VMs will not have v3 connectivity.

[conorsch]

The else [] causes an empty list here—is that intentional? The other elements of tor_instances_v3 are dicts, not lists. Haven't tested functionally yet to observe an error here, but it seems inconsistent.

[kushaldas]

I copied that from the above lines in the same file https://github.com/freedomofpress/securedrop/blame/07fed0c56e89d7edb006a7aa9e0f850d3d516216/install_files/ansible-base/group_vars/securedrop_application_server.yml#L17 Seems to be added by @msheiny 2 years ago.

[conorsch]

The key-management tasks here are largely duplicated, with conditional checks for determining whether app or mon is the current host. Since the tasks will by default run against both hosts, and the facts will be set uniquely per-host, we don't need the duplication.

[conorsch]

As above, these templates don't need to be unique per-host—see for reference the existing v2 logic:

securedrop/install_files/ansible-base/roles/restrict-direct-access/tasks/fetch_tor_config.yml

Lines 21 to 31 in 20cbd52

	- name: Write Tor hidden service hostname files to Admin Workstation.
	local_action:
	module: template
	dest: "{{ role_path }}/../../{{ item.item.filename }}"
	src: ths_config.j2
	# Local action, so we don't want elevated privileges
	become: no
	with_items: "{{ tor_hidden_service_hostname_lookup.results }}"
	tags:
	- tor
	- admin

[conorsch]

Appended two commits, aiming to simplify the keypair-management logic within the Ansible roles. Please take a look, @kushaldas, and see if you agree that the terser version is suitable.

While working on this, I noticed that the branch here is outdated; it doesn't include the related Tor logic introduced in #4648:

$ git checkout develop
Switched to branch 'develop'
Your branch is up to date with 'origin/develop'.
$ grep -rF update_onion_version_config
admin/securedrop_admin/__init__.py:        self.update_onion_version_config()
admin/securedrop_admin/__init__.py:    def update_onion_version_config(self):
admin/tests/test_securedrop-admin.py:        site_config.update_onion_version_config()
admin/tests/test_securedrop-admin.py:        site_config.update_onion_version_config()
admin/tests/test_securedrop-admin.py:        site_config.update_onion_version_config()
[user@sd-dev:~/securedrop] [sd] develop+* ±
$ git checkout onion_onion_and_more_onions
Switched to branch 'onion_onion_and_more_onions'
Your branch is up to date with 'origin/onion_onion_and_more_onions'.
$ grep -rF update_onion_version_config
$ # nothing found

If you agree the added commits are work keeping, please rebase on top of latest develop, so we're testing with the most recently merged work.

The most important outstanding changes in my view are:

• justify version bump to "cryptography" dependency
• move v3 keygen logic out of securedrop-admin, otherwise v3 won't work for devs

Based on the discussion in #4628 (comment), it looks like you've already tried to the script-based approach. Let's try to get that working again. See example below for a slightly modified version of your script. If we can adjust the calls to get it working with the currently used version of cryptography, great, otherwise let's make sure to bump the requirement in the same contexts Ansible is used (i.e. also dev envs).

Example script generate-tor-v3-keypairs

diff --git a/install_files/ansible-base/roles/tor-hidden-services/files/generate-tor-v3-keypairs b/install_files/ansible-base/roles/tor-hidden-services/files/generate-tor-v3-keypairs
new file mode 100644
index 000000000..666eed601
--- /dev/null
+++ b/install_files/ansible-base/roles/tor-hidden-services/files/generate-tor-v3-keypairs
@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives.asymmetric import x25519
+
+
+def generate_x25519_keypair():
+    """This function generate new keys and returns them as tuple.
+
+    :returns: Tuple(public_key, private_key)
+    """
+
+    private_key = x25519.X25519PrivateKey.generate()
+    private_bytes = private_key.private_bytes(
+        encoding=serialization.Encoding.Raw,
+        format=serialization.PrivateFormat.Raw,
+        encryption_algorithm=serialization.NoEncryption())
+    public_key = private_key.public_key()
+    public_bytes = public_key.public_bytes(
+        encoding=serialization.Encoding.Raw,
+        format=serialization.PublicFormat.Raw)
+
+    public = base64.b32encode(public_bytes)[:-4].decode("utf-8")
+    private = base64.b32encode(private_bytes)[:-4].decode("utf-8")
+    return public, private
+
+
+def generate_new_tor_v3_keypairs():
+    """
+    This method will either read the old keys or generate a new
+    public/private key pair.
+    """
+    # No old keys, generate and store them first
+    app_journalist_public_key, app_journalist_private_key = generate_x25519_keypair()
+    # For app ssh service
+    app_ssh_public_key, app_ssh_private_key = generate_x25519_keypair()
+    # For mon ssh service
+    mon_ssh__public_key, mon_ssh_private_key = generate_x25519_keypair()
+    tor_v3_service_info = {
+            "app_journalist_public_key": app_journalist_public_key,
+            "app_journalist_private_key": app_journalist_private_key,
+            "app_ssh_public_key": app_ssh_public_key,
+            "app_ssh_private_key": app_ssh_private_key,
+            "mon_ssh_public_key": mon_public_ssh_key,
+            "mon_ssh_private_key": mon_ssh_private_key,
+    }
+    # Send results to stdout
+    print(json.dumps(tor_v3_service_info))
+
+
+if __name__ == "__main__":
+    generate_new_tor_v3_keypairs()

Once we have passable keypair-and-service generation in this PR, we can discuss specifics on the Tails-specific integration in #4629. Let me know your thoughts on the above, @kushaldas.

[kushaldas]

@conorsch I am okay with the approach you mentioned here. What should be the next step?

[conorsch]

Appended two commits

Oops, my mistake: I hadn't actually pushed! The two new commits are now present on the branch, see fea52e0 & 92e0949.

What should be the next step?

@kushaldas Recapping my comment above, the next steps should be:

• review the two commits I added
• rebase this branch on develop, to include changes from Fixes #4627 Adds v2 and v3 onion service variables #4648 and others
• explain why a version bump to the "cryptography" dependency is necessary; it's hard to understand in the correct branch why this change was made
• move v3 keygen logic out of securedrop-admin, into a script runnable by Ansible (see example above), otherwise v3 URLs won't work for devs

Please handle those tasks, and ping me when you feel the branch is ready for another look!

[kushaldas]

move v3 keygen logic out of securedrop-admin, into a script runnable by Ansible (see example above), otherwise v3 URLs won't work for devs

@conorsch Last time when I tried, I could not do that the above in a nice Ansible way. I will need your help in that too.

[msheiny]

move v3 keygen logic out of securedrop-admin, into a script runnable by Ansible (see example above), otherwise v3 URLs won't work for devs

What about tweaking the dev initialization entry-points to call a subset of securedrop admin scripts first? I don't think ansible is the right answer to every scriptable problem. Ansible is really good at running a bunch of tasks remotely. This task is run locally and would benefit from more in-depth unit testing. Recall that in the past at one point, the admin scripting was in ansible but that was deemed unmaintainable and was moved to python.

[conorsch]

The currently implemented securedrop-admin strategy cannot configure v3 URLs when testing against VMs, so the invocation must change. It needn't be pure Ansible—simply calling a script makes sense—but at no point other than Tails-based QA testing are devs regularly invoking the securedrop-admin logic. The current script logic uses python cryptography, which means required libraries will be present within the virtualenv that executes both securedrop-admin and Ansible, on the Admin Workstation.

If we moved to a leaner method like bash, we might wind up with more portable keygen logic, but that doesn't seem worthwhile—we'd have to be careful about system deps like openssl versions. As far as what calls the keygen logic script, the most important attributes are 1) it's automatic, so devs don't forget; 2) the same process is used on dev/admin workstations, as far as possible.

[kushaldas]

The currently implemented securedrop-admin strategy cannot configure v3 URLs when testing against VMs, so the invocation must change.

Current means? The branch I pushed last week, did it amazingly :)

[kushaldas]

Also, we can just make the dynamic inventory to call the similar python function to get key pairs while doing from molecule.

[kushaldas]

@conorsch Added the changes requested. When I tried the scrip way, I could not find a way to get the correct cryptography into this script's environment.

[conorsch]

The currently implemented securedrop-admin strategy cannot configure v3 URLs when testing against VMs, so the invocation must change.

Current means? The branch I pushed last week, did it amazingly :)

I could have been more clear: the current strategy (accurate as of today on this branch) fails to configure v3 URLs on staging VMs unless a Tails VM is also used. That means that make staging will not configure v3 URLs, nor will the v3 service config be tested in CI. We'll need config tests for the v3 tor changes; consider writing those tests sooner rather than later to guide development!

When I tried the scrip way, I could not find a way to get the correct cryptography into this script's environment.

That sounds like a consequence of the crytography version bump. Although you added the new cryptography version to admin/requirements-ansible.in, you must also run make update-python{2,3}-requirements for that dependency to propagate to the relevant requirements.txt files. Once you do that, the script should work just fine for you, modulo linting errors from the copy/paste version currently in place. I already pushed a small commit fixing yamllint errors on the new Ansible logic.

Testing locally, I was able to invoke the keygen script successfully via Ansible using the following command:

ansible -c local -i localhost, all -m script -a 'install_files/ansible-base/roles/tor-hidden-services/files/generate-tor-v3-keypairs'

After cleaning up the aforementioned linting errors, that script executes well, and returns JSON (albeit escaped in the interactive terminal, since Ansible renders task results as JSON by default). So, for next steps, please do the following:

1. Rebase this branch on top of latest develop, to include changes from Fixes #4627 Adds v2 and v3 onion service variables #4648 and others.
2. Expand comment justifying cryptography version bump. (Remember, this is for future maintainers: which calls were not present in the old API? Why are those calls necessary?)
3. Update *.txt requirements files via the Makefile targets, then update your local venv to use the new reqs.
4. Modify the generate-tor-v3-keypairs script to execute successfully (has a few missing imports right now).
5. Run the script inside the Ansible tasks, if-and-only-if the JSON file doesn't exist locally. That will ensure the JSON info is present, and we can read it in for the host config, as the branch already does.
6. Write testinfra tests to validate to the tor v3 service state. These may need to wait until v3 is "on-by-default"; still, we should define "successful" service behavior in the tests, even if we temporarily disable them.

Based on previous discussion, there may be outstanding questions regarding the implementation of step 5. Please set aside some time to give it your best shot. If you get stuck and it makes sense for us to pair on Ansible work for a bit, we can find a time to do so.

[conorsch]

Rebased on top of latest develop (a9087a2), to accommodate for changes from #4665. Specifically, these were minor discrepancies in the pip deps:

• setuptools, 41.1.0 -> 41.2.0
• requests, 2.20.0 -> 2.22.0

Now that conflicts are resolved, let's make sure CI passes, then we're good to go.

Changes requested by @zenmonkeykstop have been addressed

[conorsch]

Re-approving. Satisfied with behavior in the following scenarios (all using Tails VM against prod VMs):

Scenario 1

1. Clean install with v2-only.
2. Enable v3 (in addition to v2), install again.

Scenario 2

1. Clean install with v2-only and ssh-over-lan
2. Enable v3 (in addition to v2), preserve ssh-over-lan, install again.

Scenario 3

1. Clean install with v3-only
2. [did not run a second install, since connection depends on Adds support for v3 Onion urls to ./securedrop-admin tailsconfig` #4675]

CI is passing, as well, so calling this one a wrap. Thanks for all your hard work on this one, @kushaldas & @redshiftzero!