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

Remove v2 onion service support; fix Whonix repo list filename #694

Merged
merged 1 commit into from
May 7, 2021

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented May 6, 2021

Status

Ready for review

Description of Changes

Fixes #692
Fixes #693

  • Removes support for v2 onion services (we don't even recognize the format anymore - cue Phil Collins "No Son of Mine")
  • Updates Whonix repo list filename to ensure tests pass

Testing

Estimated testing time: 30-60 minutes

  1. make clone this branch into dom0 and sudo dnf reinstall (or make staging if you don't have a previously provisioned environment) the freshly built RPM from rpm-build/RPMS/noarch.
  2. Attempt to update your /usr/share/securedrop-workstation-dom0-config/config.json with a v2 config (contents of config.json.example in current main should do) and run sdw-admin --apply
    • Observe moderately helpful error message & script exiting
  3. Now put in something that looks nothing like a v2 or v3 config for the hostname and key options (e.g., random characters), and re-run sdw-admin --apply
    • Observe same error message
  4. Put in a v3 config and run sdw-admin --validate (or --apply if you are patient)
    • Observe success
  5. Make sure your currently provisioned config.json and sd-journalist.sec are present in your securedrop-workstation checkout directory in dom0, then run make test
    • Observe tests passing (or at least that test failures are unrelated to this PR - dom0 tests can be a bit finicky)

If you have made changes to the provisioning logic

  • All tests (make test) pass in dom0 of a Qubes install

@@ -1,8 +1,8 @@
{
"submission_key_fpr": "65A1B5FF195B56353CC63DFFCC40EF1228271441",
"hidserv": {
"hostname": "avgfxawdn6c3coe3.onion",
"key": "Il8Xas7uf6rjtc0LxYwhrx"
"hostname": "sdolvtfhatvsysc6l34d65ymdwxcujausv7k5jk4cy5ttzhjoi6fzvyd.onion",
Copy link
Member Author

Choose a reason for hiding this comment

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

securedrop.org website onion service

"hostname": "avgfxawdn6c3coe3.onion",
"key": "Il8Xas7uf6rjtc0LxYwhrx"
"hostname": "sdolvtfhatvsysc6l34d65ymdwxcujausv7k5jk4cy5ttzhjoi6fzvyd.onion",
"key": "5U4JPYSZ34N2ZDSOUAL2YLEX2NPI5BLL2Y66QJW24KLSH7R3FEPQ"
Copy link
Member Author

Choose a reason for hiding this comment

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

randomly generated key in base32

)
raise
Copy link
Member Author

Choose a reason for hiding this comment

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

IMO this script shouldn't display tracebacks during expected validation errors, but that's the current pattern. See #683 as well for suggested refactoring of this whole logic.

class SD_Whonix_Tests(SD_VM_Local_Test):
def setUp(self):
self.vm_name = "sd-whonix"
self.whonix_apt_list = "/etc/apt/sources.list.d/whonix.list"
self.whonix_apt_list = "/etc/apt/sources.list.d/derivative.list"
Copy link
Member Author

Choose a reason for hiding this comment

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

That's what fixes #693

@conorsch
Copy link
Contributor

conorsch commented May 7, 2021

Looks great. I found one more place in the tests where updating the repo filepath makes sense:

diff --git a/tests/test_proxy_vm.py b/tests/test_proxy_vm.py
index 37e84c8..989c620 100644
--- a/tests/test_proxy_vm.py
+++ b/tests/test_proxy_vm.py
@@ -63,7 +63,9 @@ class SD_Proxy_Tests(SD_VM_Local_Test):
         Guard against regressions by ensuring the old Whonix apt list
         is missing.
         """
+        # Whonix project changed the repo filename ~2021-05, so check both.
         assert not self._fileExists("/etc/apt/sources.list.d/whonix.list")
+        assert not self._fileExists("/etc/apt/sources.list.d/derivative.list")
 
     def test_logging_configured(self):
         self.logging_configured()

Will amend as part of review.

@conorsch conorsch force-pushed the 692-v2-is-no-more branch from 420213f to bc87a69 Compare May 7, 2021 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Whonix apt repo source list filename has changed Remove v2 support
2 participants