Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deploy: configure sd-whonix by package #1051

Merged
merged 8 commits into from
Jun 5, 2024
Merged

Conversation

cfm
Copy link
Member

@cfm cfm commented May 29, 2024

Status

Ready for review

Description of Changes

Closes #1039 by configuring sd-whonix using freedomofpress/securedrop-client#2032's securedrop-whonix-config rather than Salt.

Closes #936.

Testing

If testing after freedomofpress/securedrop-client#2032:

  1. make clone && make dev

...and if testing in conjunction with freedomofpress/securedrop-client#2032:

  1. On failure in sd-base-bookworm-template: manually install securedrop-qubesdb
  2. make dev again
  3. On failure in whonix-gateway-17: manually install securedrop-qubesdb and securedrop-whonix-config
  4. make dev again

Either way, finally:

  1. You're able to log into the Client.

Checklist

If you have made changes to the provisioning logic

  • All tests (make test) pass in dom0

If you have added or removed files

  • I have updated MANIFEST.in and rpm-build/SPECS/securedrop-workstation-dom0-config.spec

@cfm cfm force-pushed the sdw-1039-qubesdb-packages branch 3 times, most recently from 1712f32 to 0e18ed1 Compare May 29, 2024 18:07
@legoktm legoktm force-pushed the sdw-1039-qubesdb-packages branch from 0e18ed1 to ecd6afb Compare May 31, 2024 19:10
@legoktm
Copy link
Member

legoktm commented May 31, 2024

The debs are now on apt-test so I've rebased to kick off CI.

@legoktm
Copy link
Member

legoktm commented May 31, 2024

Do we need a cleanup stage to uninstall the packages? Admittedly I can't really see anything that was cleaning it up before...

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running make dev doesn't invoke the newly added salt thing, I had to manually run sudo qubesctl --show-output --skip-dom0 --targets whonix-gateway-17 state.highstate and then it all worked (and I could log in and auth_private was provisioned correctly).

I think this is because the template is not tagged as sd-workstation so it needs an explicit salt run thing.

We also need a salt state to remove the packages (purge ideally) to restore it to original state on uninstall.

@legoktm legoktm force-pushed the sdw-1039-qubesdb-packages branch from 176bb82 to 96e6036 Compare May 31, 2024 19:59
@legoktm legoktm force-pushed the sdw-1039-qubesdb-packages branch from 96e6036 to 3114359 Compare June 3, 2024 15:26
@cfm cfm self-assigned this Jun 3, 2024
@cfm
Copy link
Member Author

cfm commented Jun 3, 2024

Thanks, all. Let me tackle the rebase after #1048 and then I'll turn this back over to you, unless you'd rather I address #1051 (comment) at the same time.

@legoktm
Copy link
Member

legoktm commented Jun 3, 2024

and then I'll turn this back over to you, unless you'd rather I address #1051 (comment) at the same time.

No, I double checked with Ro as well and we can defer it to after 4.2 (but before GA).

@cfm cfm force-pushed the sdw-1039-qubesdb-packages branch 2 times, most recently from 9303b7a to c5c629e Compare June 3, 2024 21:49
@cfm
Copy link
Member Author

cfm commented Jun 3, 2024

Rebased from main after #1048.

@cfm cfm removed their assignment Jun 3, 2024
@cfm cfm requested review from legoktm, deeplow and rocodes June 3, 2024 21:51
legoktm
legoktm previously approved these changes Jun 3, 2024
Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just waiting on a green check from SDW CI.

cfm and others added 2 commits June 3, 2024 19:40
Despite what's documented[1], Tor seems happy to configure client
authentication for an onion address ending in ".onion".  That is, I'm
able to break "app-journalist.auth_private" by mangling it in other
ways, but it works both with and without ".onion".

If need be, we can always change the semantics of the
"/vm-config/SD_HIDSERV_HOSTNAME" QubesDB key to distinguish between
versions with and without the ".onion" suffix; or remove it and let
applications add it where they need it.

[1]: https://gitlab.torproject.org/tpo/core/tor/-/blob/7a5d94bcf842299534b667433424ac7a1133d371/doc/man/tor.1.txt#L3659
And also clean up the stuff we install and provision.
@legoktm legoktm force-pushed the sdw-1039-qubesdb-packages branch from c5c629e to 5628be9 Compare June 3, 2024 23:40
@cfm
Copy link
Member Author

cfm commented Jun 4, 2024

https://ws-ci-runner.securedrop.org/2024-06-03-233933015542-5628be940faa556ca8480ec1c7d8715289c0ba61-Qubes_4.2_B-update_20240603044409.log.txt:

INFO:[2024-06-04-00:08:49:204996]             ID: update-apt-cache-with-stable-change
INFO:[2024-06-04-00:08:49:205027]       Function: cmd.run
INFO:[2024-06-04-00:08:49:205055]           Name: apt-get update --allow-releaseinfo-change
INFO:[2024-06-04-00:08:49:205092]         Result: False
INFO:[2024-06-04-00:08:49:205123]        Comment: Command "apt-get update --allow-releaseinfo-change" run
INFO:[2024-06-04-00:08:49:205152]        Started: 00:06:47.633926
INFO:[2024-06-04-00:08:49:205185]       Duration: 1199.845 ms
INFO:[2024-06-04-00:08:49:205213]        Changes:   
INFO:[2024-06-04-00:08:49:205255]                 ----------
INFO:[2024-06-04-00:08:49:205289]                 pid:
INFO:[2024-06-04-00:08:49:205317]                     1823
INFO:[2024-06-04-00:08:49:205349]                 retcode:
INFO:[2024-06-04-00:08:49:205382]                     100
INFO:[2024-06-04-00:08:49:205410]                 stderr:
INFO:[2024-06-04-00:08:49:205442]                     E: Could not get lock /var/lib/apt/lists/lock. It is held by process 1578 (apt-get)
INFO:[2024-06-04-00:08:49:205475]                     E: Unable to lock directory /var/lib/apt/lists/
INFO:[2024-06-04-00:08:49:205507]                 stdout:
INFO:[2024-06-04-00:08:49:205539]                     Reading package lists...

@cfm
Copy link
Member Author

cfm commented Jun 4, 2024

Hmm. @legoktm, in the latest CI failure you flagged:

INFO:[2024-06-04-00:44:18:944941] ERROR: test_v3_auth_private_file (test_sd_whonix.SD_Whonix_Tests.test_v3_auth_private_file)
INFO:[2024-06-04-00:44:18:944972] ----------------------------------------------------------------------
INFO:[2024-06-04-00:44:18:945006] Traceback (most recent call last):
INFO:[2024-06-04-00:44:18:945038]   File "/home/user/securedrop-workstation/tests/test_sd_whonix.py", line 24, in test_v3_auth_private_file
INFO:[2024-06-04-00:44:18:945071]     self.assertFileHasLine("/var/lib/tor/authdir/app-journalist.auth_private", line)
INFO:[2024-06-04-00:44:18:945099]   File "/home/user/securedrop-workstation/tests/base.py", line 139, in assertFileHasLine
INFO:[2024-06-04-00:44:18:945133]     remote_content = self._get_file_contents(remote_path)
INFO:[2024-06-04-00:44:18:945165]                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INFO:[2024-06-04-00:44:18:945202]   File "/home/user/securedrop-workstation/tests/base.py", line 98, in _get_file_contents
INFO:[2024-06-04-00:44:18:945251]     return subprocess.check_output(cmd).decode("utf-8")
INFO:[2024-06-04-00:44:18:945287]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INFO:[2024-06-04-00:44:18:945321]   File "/usr/lib64/python3.11/subprocess.py", line 466, in check_output
INFO:[2024-06-04-00:44:18:945354]     return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
INFO:[2024-06-04-00:44:18:945391]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INFO:[2024-06-04-00:44:18:945424]   File "/usr/lib64/python3.11/subprocess.py", line 571, in run
INFO:[2024-06-04-00:44:18:945452]     raise CalledProcessError(retcode, process.args,
INFO:[2024-06-04-00:44:18:945487] subprocess.CalledProcessError: Command '['qvm-run', '-p', 'sd-whonix', 'sudo /bin/cat /var/lib/tor/authdir/app-journalist.auth_private']' returned non-zero exit status 1.
INFO:[2024-06-04-00:44:18:945517]

I've tacked on 9835669 to test the "Qubes feature to QubesDB variable" path more rigorously. But I also see one of our rebases lost the bash -c from the securedrop-whonix-config service unit's ExecStartPost, which (as I learned and thought I had fixed last week!) doesn't run in a shell. I'll push that fix up now, but I won't be able to test it locally until tomorrow:

@cfm
Copy link
Member Author

cfm commented Jun 4, 2024

freedomofpress/securedrop-client#2056 may help here; with it I'm not able to reproduce this failure locally. However, Workstation CI does not appear to be running on new commits here....

@cfm cfm force-pushed the sdw-1039-qubesdb-packages branch from 981557c to 93d4a1a Compare June 4, 2024 17:40
@cfm cfm force-pushed the sdw-1039-qubesdb-packages branch from 93d4a1a to b2c17c3 Compare June 4, 2024 18:11
@legoktm
Copy link
Member

legoktm commented Jun 4, 2024

Since CI isn't cooperating I'm just going to verify it manually and then merge it.

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I found a race condition, the /var/lib/tor/authdir directory is created by anon-gw-anonymizer-config.service, so we need a Requires/After (don't remember of the top of my head which one). Otherwise it'll error that the directory doesn't exist.

see https://github.com/Whonix/anon-gw-anonymizer-config/blob/master/etc/torrc.d/65_gateway.conf#L32-L39 and https://github.com/Whonix/anon-gw-anonymizer-config/blob/master/usr/libexec/anon-gw-anonymizer-config/make-sure-torrc-exist#L72.

Once that lands this should be ready to go, 🤞🏾

@cfm
Copy link
Member Author

cfm commented Jun 4, 2024

Thanks! On it.

@cfm
Copy link
Member Author

cfm commented Jun 5, 2024

@legoktm in #1051 (review):

Unfortunately, I found a race condition, the /var/lib/tor/authdir directory is created by anon-gw-anonymizer-config.service, so we need a Requires/After (don't remember of the top of my head which one). Otherwise it'll error that the directory doesn't exist.

see https://github.com/Whonix/anon-gw-anonymizer-config/blob/master/etc/torrc.d/65_gateway.conf#L32-L39 and https://github.com/Whonix/anon-gw-anonymizer-config/blob/master/usr/libexec/anon-gw-anonymizer-config/make-sure-torrc-exist#L72.

Once that lands this should be ready to go, 🤞🏾

Great catch: it looks like this was the cause of the original CI failure I wasn't able to reproduce locally. Fixed proposed and explained (with one FIXME for follow-up) in freedomofpress/securedrop-client#2058.

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@legoktm legoktm added this pull request to the merge queue Jun 5, 2024
Merged via the queue into main with commit fbaed0d Jun 5, 2024
9 checks passed
@legoktm legoktm deleted the sdw-1039-qubesdb-packages branch June 5, 2024 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
4 participants