Uses apt mirror for Tor via `securedrop-config` package #2441

conorsch · 2017-10-13T21:40:04Z

Status

Ready for review.

Description of Changes

Fixes #2106. Reimplements and supersedes #2113.

Changes proposed in this pull request:

Installs tor packages from tor-apt.freedom.press mirror.
Migrates existing apt configs on running hosts to use only the FPF tor mirror.
Leverages new securedrop-config package to handle changes

Testing

We need to test clean installs, which CI should do rather well. More important, however, we must ensure the upgrade 0.4.3 prod -> this PR is smooth. Given the imminent codefreeze for 0.4.4, we may have to utilize the QA period to make sure we've addressed the upgrade path sufficiently.

Deployment

Yes, major implications for deployment. Affects running instances and the method they use to install tor, which is critical for SecureDrop (see #2105 for a reminder).

Checklist

If you made changes to the app code:

Unit and functional tests pass on the development VM

If you made changes to the system configuration:

Configuration tests pass

If you made changes to documentation:

Doc linting passed locally

ghost · 2017-10-16T19:36:08Z

Testing process I'm following:

rebase against develop
git checkout -b release/0.4.3 origin/release/0.4.3
vagrant up --no-provision /staging/
make build-debs
vagrant provision /staging/
./testinfra/test.py app-staging
verify the source THS works
git checkout -b securedrop-config-package-for-tor-mirror origin/securedrop-config-package-for-tor-mirror
rm build/*
make build-debs
vagrant provision app-staging
./testinfra/test.py app-staging
verify the source THS works
vagrant ssh app-staging

# apt-cache policy tor
tor:
  Installed: 0.3.1.7-1~trusty+1
  Candidate: 0.3.1.7-1~trusty+1
  Version table:
 *** 0.3.1.7-1~trusty+1 0
        500 http://tor-apt.freedom.press/ trusty/main amd64 Packages
        100 /var/lib/dpkg/status
     0.2.4.27-1build0.14.04.1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty-updates/universe amd64 Packages
        500 http://security.ubuntu.com/ubuntu/ trusty-security/universe amd64 Packages
     0.2.4.20-1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty/universe amd64 Packages

conorsch · 2017-10-16T20:17:31Z

@dachary That sounds great! I'll be doing a similar operation on prod VMs. Noticed that our Ansible logic still installs the tor GPG pubkey, which it shouldn't. But the postinst hooks in securedrop-config remove the key, so it's not a large problem. I'll add a commit here to address.

ghost

Successfully ran the test plan. I also reviewed the implementation in the previous PR and found it sane. Great work on this delicate part :-)

redshiftzero · 2017-10-16T21:12:44Z

testinfra/common/test_tor_mirror.py

+sub   2048R/219EC810 2009-09-04 [expires: 2018-08-30]"""
+
+    assert c.rc == 0
+    assert tor_gpg_pub_key_info in c.stdout


@conorsch and I just had a brief discussion about this: this test is clearly passing but we had discussed a workflow where we resign Tor packages on our mirror with the SecureDrop release key, so this key should be gone. If we keep this as is, we'll need to act prior to the Valid-Until expires (as in, do QA and update the packages on the mirror before the package expires).

emkll · 2017-11-13T18:53:39Z

install_files/ansible-base/group_vars/all/securedrop

@@ -51,3 +51,4 @@ appserver_dependencies:
  - redis-server
  - supervisor

+tor_apt_repo_url: http://tor-apt.freedom.press


apt.freedom.press supports HTTPS. Why isn't it being used here?

so initially there was confusion on my behalf on how to do this with our subdomain and S3 buckets --- but after some much needed time away from that task. I know an excellent way to work around my issue to make it work. So yeah -- i agree, i dont think we should merge until this is https which requires a backend change

redshiftzero

Blocking pending a rework to update test_tor_mirror_fingerprint, switch to HTTPS per @emkll's comment, and resolve conflicts

conorsch · 2017-11-28T23:15:33Z

Bumped to 0.5.1 pending further QA. The 0.5 code freeze is hours away, so we'll focus on testing what we have—0.5 is already a substantial release—then make a special release for the Tor mirror strategy. New milestone for 0.5.1: https://github.com/freedomofpress/securedrop/milestone/37

conorsch · 2017-12-22T00:41:31Z

Rebased and added a few tweaks to address requested changes. Ready for re-review: @msheiny, @redshiftzero, @emkll, @dachary.

msheiny · 2017-12-22T16:08:12Z

working on reviewing this today

This addresses issue #2106 - mitigate side-effects from a tor package breaking securedrop instances in the field. The actual logic that creates our mirror is in source control in another repo.

We don't have the mirror reflected in the config yet, but these tests describe the system state we want. Time for some TDD!

New Debian package designed for shipping unattended upgrades that affect system state (rather than simply installing new software). We're breaking some of the Debian packaging rules, specifically by touching contents inside `/etc/apt/`, but SecureDrop is not an official Debian package, and we're highly opinionated about what gets to run on SecureDrop servers, so I'm comfortable with the lintian errors for now. Creates postinst for securedrop-config package Running the sed replace actions via postinst, so that current official Tor apt repo settings are replaced with the FPF mirror. Removes postinst tor repo logic from the `securedrop-app-code` package, since that would only have affected the Application Server.

msheiny · 2017-12-22T19:22:42Z

rebased on develop

Reusing the generic-pkg role to build the new `securedrop-config` package along with the others. Sets securedrop-config as dependency of securedrop-keyring Adds local pkg vars for securedrop-keyring. Since `securedrop-keyring` depends on `securedrop-config`, the latter must be installed first, so the order of items the local packages list var is important. Runs the usual collection of tests over the new deb package, as well. Includes entry in the `update_version.sh` script so that the config package will have updates made to it, as well.

@dachary

The package isn't architecture-dependent, so don't lie about it. The SecureDrop install scripts only setup "amd64" package access via the FPF tor repository. Will shake out any inconsistencies in pre-release QA with release candidate debs. Pointed out by @dachary during review. Further reading at: https://www.debian.org/doc/debian-policy/index.html#architecture Updates associated config tests, as well.

Previously tried to piggyback on securedrop-keyring, setting the securedrop-config package as a dependency thereof. It's more explicit to list securedrop-config as a dependency of packages required for the Application and Monitor Server roles: * securedrop-app-code * securedrop-ossec-agent * securedrop-ossec-server Therefore, removed the dependency in securedrop-keyring.

The parametrization was improperly implemented, causing the test suite to fail. Rather than recursive grep across all of `/etc/apt`—which will cause false positives in the tor keyring under /etc/apt/trusted.gpg.d—let's look only at the two rationale locations, and recursively search through /etc/apt/sources.list.d (since multiple Ansible versions could have written to different paths).

@msheiny

The backend recently moved around, so updating the deb package implementation to reflect the backend changes. Updated the URL from: tor-apt.ops.freedom.press to: tor-apt.freedom.press We also have "tor-apt-test.freedom.press" for QA purposes, but without templates for the Debian scripts, however, swapping the URLs for QA will need to be manual. We can circle back and smooth out that process in subsequent PRs—the priority now is to make sure we have mirror functionality working. Thanks to changes on the backend by @msheiny, we now have switched HTTP -> HTTPS as well.

Only takes effect when the Ansible logic is run, e.g. during first install or a health check. Since we're mirroring the Tor apt repository now and using the SecureDrop Release Signing Key for validation, we do *not* want the official Tor repo pubkey on the system. Includes substantial changes to the configuration tests to reflect the new strategy.

Previously end-of-line space characters on stderr were getting compared to our lovely error_text match. I suspect there is a difference in how testinfra handles strings in direct SSH vs. ansible mode. 🤷

The previous calls utilized were deprecated in recent releases of testinfra. We still have this style throughout our repository tests but as we add new test files, I think we should make sure we use the latest style.

msheiny · 2017-12-22T20:13:49Z

install_files/ansible-base/roles/tor-hidden-services/tasks/install_tor.yml

  tags:
    - apt
    - tor

 - name: Setup Tor apt repo.
  apt_repository:
-    repo: deb http://deb.torproject.org/torproject.org {{ ansible_lsb.codename }} main
+    filename: tor_apt_freedom_press
+    repo: deb {{ tor_apt_repo_url }} {{ ansible_lsb.codename }} main


This logic should be removed eventually since it is duplicating the logic of the new securedrop-config package. I'm not going to block on it for this review and instead I'll spin up a new ticket.

msheiny · 2017-12-22T20:24:55Z

hoping this is a one-off transient networking error .. it passed successfully on the parallel test 😕

fatal: [mon-staging]: FAILED! => {"changed": false, "failed": true, "module_stderr": "Shared connection to 54.219.150.128 closed.\r\n", "module_stdout": "Traceback (most recent call last):\r\n File "/tmp/ansible_tZsde9/ansible_module_apt_repository.py", line 565, in \r\n main()\r\n File "/tmp/ansible_tZsde9/ansible_module_apt_repository.py", line 553, in main\r\n cache.update()\r\n File "/usr/lib/python2.7/dist-packages/apt/cache.py", line 440, in update\r\n raise FetchFailedException(e)\r\napt.cache.FetchFailedException: W:Failed to fetch https://tor-apt.freedom.press/dists/trusty/main/binary-amd64/Packages HttpError403\r\n, E:Some index files failed to download. They have been ignored, or old ones used instead

msheiny

Nooiicee I really like the effort of putting this logic in a new config package and as always you are super awesome at adding new tests ❤️ and documenting all your git commits. I have some lingering concerns that require me to do some more testing. Ugggh we really need automated upgrade testing in CI

msheiny · 2017-12-22T20:25:45Z

install_files/ansible-base/group_vars/securedrop_application_server.yml

  - "securedrop-ossec-agent-2.8.2+{{ securedrop_app_code_version }}-amd64.deb"
  - "{{ securedrop_app_code_deb }}.deb"
  - "ossec-agent-2.8.2-amd64.deb"
-  - "securedrop-keyring-0.1.1+{{ securedrop_app_code_version }}-amd64.deb"


hah! the tables have turned! not in alphabetical order 😛 !

Actually the order here is important, since we're installing via dpkg -i there's no smart dependency resolution, so we must install the dependencies first, then the packages that depend on them!

...... but but..... dang it

msheiny · 2017-12-22T20:31:22Z

install_files/securedrop-ossec-server/DEBIAN/control

@@ -6,7 +6,7 @@ Homepage: https://securedrop.org
 Package: securedrop-ossec-server
 Version: 2.8.2+0.5
 Architecture: amd64
-Depends: ossec-server,securedrop-keyring
+Depends: ossec-server,securedrop-keyring,securedrop-config


soooo as a totally side discussion in my PR for #2592 - i made it a point not to install tor on ossec if one enables tor over local network. I don't know if that was the right call (im on the fence) - I'm just making a note that here you are making this package a pre-requisite for ossec server where in my PR I assume tor might not be installed on the mon server... Once again, totally outside the scope of this ticket but I would like to convo about what should be the right call there.

Hm, interesting. To be clear, the securedrop-config package does not actually install tor, it just configures the FPF apt mirror for tor. So there shouldn't be any conflict there.

But the discrepancy you point out is the direct result of trying to support customizations of deployments, so we haven't seen the last of that (related: #1129).

Hm, interesting. To be clear, the securedrop-config package does not actually install tor, it just configures the FPF apt mirror for tor. So there shouldn't be any conflict there.

Yeah thats a very good point but just something I figured I'd point out since it caught my attention.

Regarding #1129 though-- i really dont want administrators to make those kinds of unique changes. If they are doing that, we need to try to officially support the features they are trying to add.

msheiny · 2017-12-22T20:39:52Z

install_files/securedrop-config/DEBIAN/postinst

+    # Repoint tor repositories to FPF mirror
+    apt_security_list="/etc/apt/security.list"
+    if [ -f "$apt_security_list" ]; then
+        sed -i 's/deb\.torproject\.org\/torproject\.org/tor-apt.freedom.press/g' "$apt_security_list"


won't this add a duplicate apt source once the securedrop-config installs ?

a copy in /etc/apt/security.list and

a revision in /etc/apt/sources.list.d/tor.apt.freedom.press.list

I thought apt would throw a warning for this type of thing ... I have to confirm this scenario actually exists first though

/etc/apt/security.list is the list of repos referenced by cron-apt for nightly upgrades. The /etc/apt/sources.list.d/tor.apt.freedom.press.list location is where system apt finds the repository information for normal apt install tasks. You'll also see the FPF repo (apt.freedom.press) duplicated in security.list.

msheiny · 2017-12-22T20:40:42Z

install_files/ansible-base/roles/tor-hidden-services/tasks/install_tor.yml

-    state: present
-    data: "{{ lookup('file', 'tor-signing-key.pub') }}"
+    state: absent
+    id: A3C4F0F979CAA22CDBA8F512EE8CBC9E886DDD89


I think this removal logic should also be part of the effort of #2764 but I'm not blocking this marge on that . Good due diligence on removing it here thru ansible for now 👍

@redshiftzero

@redshiftzero's requests addressed in recent commits - still testing but that review is now stale

msheiny

okay -- @conorsch answered my questions really quickly. I think this is going to need a lot of hands on QA during release but as-is. I think this is ready to go in. I re-kicked the tires of the two failing CI issues cause they seemed to be flukes - after that passes I'm 👍 for marge.

conorsch requested a review from msheiny as a code owner October 13, 2017 21:40

conorsch requested review from a user and redshiftzero October 13, 2017 21:44

conorsch force-pushed the securedrop-config-package-for-tor-mirror branch 2 times, most recently from 7557b5d to a6c4ae2 Compare October 15, 2017 16:16

conorsch changed the title ~~Securedrop config package for tor mirror~~ Uses apt mirror for Tor via securedrop-config package Oct 15, 2017

ghost added the goals: Improve Ansible logic / smoother install label Oct 16, 2017

ghost previously approved these changes Oct 16, 2017

View reviewed changes

ghost added Ansible ops/deployment and removed goals: Improve Ansible logic / smoother install labels Oct 16, 2017

redshiftzero reviewed Oct 16, 2017

View reviewed changes

emkll reviewed Nov 13, 2017

View reviewed changes

redshiftzero added the release blocker label Nov 15, 2017

redshiftzero previously requested changes Nov 16, 2017

View reviewed changes

redshiftzero added the PR: Pending additional work label Nov 16, 2017

redshiftzero added this to the 0.5 milestone Nov 27, 2017

conorsch mentioned this pull request Nov 27, 2017

Support a .well-known www directory for external CA validation #2603

Merged

2 tasks

conorsch dismissed ghost ’s stale review via ad3aa4e November 27, 2017 19:44

conorsch force-pushed the securedrop-config-package-for-tor-mirror branch from a6c4ae2 to ad3aa4e Compare November 27, 2017 19:44

redshiftzero modified the milestones: 0.5, 0.5.1 Nov 28, 2017

ghost removed the Ansible label Dec 4, 2017

conorsch mentioned this pull request Dec 8, 2017

Pull in securedrop-grsec metapackage #2696

Closed

conorsch force-pushed the securedrop-config-package-for-tor-mirror branch from ad3aa4e to d80f1f5 Compare December 22, 2017 00:39

conorsch requested a review from kushaldas December 22, 2017 00:41

conorsch force-pushed the securedrop-config-package-for-tor-mirror branch 2 times, most recently from 1468bb7 to 9187b56 Compare December 22, 2017 03:12

msheiny and others added 4 commits December 22, 2017 14:21

Swap tor apt repo to point to our mirror

a370424

This addresses issue #2106 - mitigate side-effects from a tor package breaking securedrop instances in the field. The actual logic that creates our mirror is in source control in another repo.

Update tests to accompany tor repo mirror

b63171c

Creates config tests for FPF Tor apt mirror

2d1f062

We don't have the mirror reflected in the config yet, but these tests describe the system state we want. Time for some TDD!

msheiny force-pushed the securedrop-config-package-for-tor-mirror branch from 2ff6286 to 4ab2369 Compare December 22, 2017 19:22

Conor Schaefer and others added 8 commits December 22, 2017 14:33

Fix stderr str comparison on tor keyring test

b10a02d

Previously end-of-line space characters on stderr were getting compared to our lovely error_text match. I suspect there is a difference in how testinfra handles strings in direct SSH vs. ansible mode. 🤷

Convert test_tor_mirror file to use latest testinfra paradigm

0c2b1b3

The previous calls utilized were deprecated in recent releases of testinfra. We still have this style throughout our repository tests but as we add new test files, I think we should make sure we use the latest style.

msheiny force-pushed the securedrop-config-package-for-tor-mirror branch from 4ab2369 to 0c2b1b3 Compare December 22, 2017 19:33

msheiny reviewed Dec 22, 2017

View reviewed changes

msheiny mentioned this pull request Dec 22, 2017

New securedrop-config package logic duplicated in ansible #2764

Closed

msheiny reviewed Dec 22, 2017

View reviewed changes

msheiny approved these changes Dec 22, 2017

View reviewed changes

msheiny merged commit a197d41 into develop Dec 22, 2017

msheiny deleted the securedrop-config-package-for-tor-mirror branch December 22, 2017 21:45

redshiftzero mentioned this pull request Jan 10, 2018

Release SecureDrop 0.5.1 #2813

Closed

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uses apt mirror for Tor via `securedrop-config` package #2441

Uses apt mirror for Tor via `securedrop-config` package #2441

conorsch commented Oct 13, 2017 •

edited by ghost

ghost commented Oct 16, 2017 •

edited by ghost

conorsch commented Oct 16, 2017

ghost left a comment

redshiftzero Oct 16, 2017

emkll Nov 13, 2017

msheiny Nov 13, 2017

redshiftzero left a comment

conorsch commented Nov 28, 2017

conorsch commented Dec 22, 2017

msheiny commented Dec 22, 2017

msheiny commented Dec 22, 2017

msheiny Dec 22, 2017

msheiny commented Dec 22, 2017

msheiny left a comment

msheiny Dec 22, 2017

conorsch Dec 22, 2017

msheiny Dec 22, 2017

msheiny Dec 22, 2017

conorsch Dec 22, 2017

msheiny Dec 22, 2017

msheiny Dec 22, 2017

conorsch Dec 22, 2017

msheiny Dec 22, 2017

msheiny left a comment

Uses apt mirror for Tor via securedrop-config package #2441

Uses apt mirror for Tor via securedrop-config package #2441

Conversation

conorsch commented Oct 13, 2017 • edited by ghost

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the app code:

If you made changes to the system configuration:

If you made changes to documentation:

ghost commented Oct 16, 2017 • edited by ghost

conorsch commented Oct 16, 2017

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redshiftzero left a comment

Choose a reason for hiding this comment

conorsch commented Nov 28, 2017

conorsch commented Dec 22, 2017

msheiny commented Dec 22, 2017

msheiny commented Dec 22, 2017

Choose a reason for hiding this comment

msheiny commented Dec 22, 2017

msheiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msheiny left a comment

Choose a reason for hiding this comment

Uses apt mirror for Tor via `securedrop-config` package #2441

Uses apt mirror for Tor via `securedrop-config` package #2441

conorsch commented Oct 13, 2017 •

edited by ghost

ghost commented Oct 16, 2017 •

edited by ghost