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

Enable apparmor for whonix gateway and workstation vms #124

Merged
merged 3 commits into from
Aug 23, 2018

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Aug 15, 2018

Fixes #108 and #122
Based on the instructions in https://www.whonix.org/wiki/Qubes/AppArmor, AppArmor is enabled on the whonix template prior to creating the sd-whonix and sd-journalist AppVMs

Based on the instructions in https://www.whonix.org/wiki/Qubes/AppArmor, AppArmor is enabled on the whonix template prior to creating the sd-whonix and sd-journalist AppVMs
@conorsch conorsch self-requested a review August 15, 2018 17:12
@conorsch
Copy link
Contributor

conorsch commented Aug 15, 2018

How's this for a test plan, @emkll?

  1. Run make all on master branch.
  2. Log into sd-whonix VM, run aa-status
  3. Observe that AppArmor is not enforced for the tor process
  4. cat /etc/os-release and observe the vm is using debian 8 (Jessie)
  5. Switch to this branch, run make all again.
  6. Log into sd-whonix again.
  7. Run aa-status and observe that the tor process is confined via AppArmor.
  8. cat /etc/os-release and observe the vm is using debian 9 (Stretch)
    We should probably add a config test to tests/test_sd_whonix.py, as well. Will comment more once I've tested locally.

@emkll
Copy link
Contributor Author

emkll commented Aug 15, 2018

@conorsch Good point, will add tests

@conorsch
Copy link
Contributor

Ran into some trouble with the test plan; after running make all:

ID: sd-whonix
Function: qvm-vm
Result: False
Comment: The following requisites were not found:
    require: 
        pkg: template-whonix-ws

The same error message appears on both master and this feature branch, so it appears we have some breakage that warrants a separate issue. Right now, it appears I'm blocked on reviewing the changes here, since I can't get the provisioning logic to complete.

Will poke more and see if any fixes in-line can be appended here to resolve; the need for such fixes is tracked in #122.

Provisioning logic would fail with an error pertaining to `template-whonix-ws`. Updating to whonix 14 workstation for Journalist VM and whonix 14 gateway for sd-whonix sidesteps this issue.
@emkll emkll force-pushed the 108-whonix-apparmor branch from 26ce86c to b17faf0 Compare August 20, 2018 16:23
@emkll
Copy link
Contributor Author

emkll commented Aug 20, 2018

Moving to whonix-14 fixes the issue @conorsch observed, but introduces a regression where /etc/tor/torrc or /usr/local/etc/torrc.d/50_user.conf do not get populated by sd-whonix-hidserv-key.sls

@conorsch
Copy link
Contributor

After a reboot, I'm seeing notification spam in dom0 related to the apparmor changes to whonix:

whonix-apparmor-spam

In light of #122, I'll note that I applied the apparmor changes described here to the old whonix vm, running Jessie. Adjusting the template to use whonix-gw-14 instead did not resolve the problem. Worse, it appears I've broken whonix entirely on my Qubes system with the template change.

@emkll can you recommend remediation steps to get whonix working on my system again? Might be best to put that discussion in #122. These tickets are going to be closely related either way.

@emkll
Copy link
Contributor Author

emkll commented Aug 21, 2018

Thanks for the report @conorsch , sorry for breaking your computer. Prior to submitting, I had tested the pr for both Whonix versions 13 and 14, and I have not seen the errors you are experiencing. I still am struggling with idempotency and machine state, as I am currently using Qubes for daily usage as well. If Qubes permits template installs (and use of community repos) within salt, we should consider doing so in the medium term.

I think the best way forward would be to have a single whonix-gw template (14, as 13 is EOL), and two whonix-ws based AppVMs based on this template (sys-whonix [*] and sd-whonix). Given the level of breakage that occurred, I think it should be addressed as part of this PR. At a high level, a new sys-whonix vm AppVM must be created, based on the whonix-gw-14 template. Renaming the sys-whonix in-place has been unreliable for me, here are the steps I took (which can certainly be optimized):

  1. Create sys-whonix-14 AppVM (provides internet) via template qubes-template-whonix-gw-14
  2. For any domain that uses sys-whonix , set netvm to sys-whonix-14
  3. Delete sys-whonix
  4. Delete version 13 of the qubes-template-whonix-ws in dom0 as it is dom0-provisioned and cannot be deleted via the Qubes Manager (rpm -qa | grep qubes-template-whonix and removing qubes-template-whonix-{ws,gw} that are not version 14)
  5. Rename sys-whonix-14 to sys-whonix : In my testing, the rename basically acted as a clone, and I had to change all sys-whonix-14 references back to sys-whonix and delete sys-whonix-14

[*] The reason we should maintain/preserve sys-whonix is that it's the default way Whonix retrieves updates (Template VMs go over Tor by default), using sys-whonix by default (as noted by the policy found dom0: /etc/qubes-rpc/policy/qubes.UpdatesProxy)

@conorsch
Copy link
Contributor

Pleased to report that @emkll's remediation steps have indeed resolved the whonix-related problems I encountered while reviewing this PR. I'll note that I also had to run:

sudo qubes-dom0-update --enablerepo=qubes-templates-community --action=reinstall qubes-template-whonix-ws-14

Thereafter, make all in dom0 on this branch passes without error for me. make test, however, fails dude to the salt config drift that @emkll reported above:

a regression where /etc/tor/torrc or /usr/local/etc/torrc.d/50_user.conf do not get populated by sd-whonix-hidserv-key.sls

We should fix that issue here, then we're good for merge.

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Almost there; let's update the salt config to write the HidServAuth line to sd-whonix.

See the upgrade docs for Whonix 13 -> 14 in Qubes. User additions should
now be written to `/usr/local/etc/torrc.d/50_user.conf`. Tested this
locally and confirmed access to the Journalist Interface via
`sd-journalist` VM.
@conorsch
Copy link
Contributor

Latest changes resolve the whonix config issues. Now able to run make all and make test in dom0 without issue. @emkll, if you can confirm successful access to Journalist Interface via sd-journalist using these latest changes, we should be good for merge.

@kushaldas
Copy link
Contributor

make all passed on this branch. I followed the steps given to have a version 14 of the whonix vm.

make tests fail for the test_dispvm as in Python2 there is not qubes.tests module, it is only available for Python3 in my dom0.

Just open python interpreter, and then try to import qubes.tests to verify.

@kushaldas
Copy link
Contributor

It seems we only have python3 version of the tests module coming from the qubes-core-dom0 package.

@joshuathayer
Copy link
Contributor

make all and make test work for me, aa-status on sd-whonix shows apparmor is installed. my existing whonix-based VMs seem to work fine without jumping through further hoops (ie, without upgrading sys-whonix)

so, looks good from my perspective!

@redshiftzero
Copy link
Contributor

Merging this, issues with python 2 vs python 3 can be tackled in a followup

@redshiftzero redshiftzero merged commit 59b89b2 into master Aug 23, 2018
@redshiftzero redshiftzero deleted the 108-whonix-apparmor branch August 23, 2018 20:08
charles-boyd pushed a commit to charles-boyd/securedrop-workstation that referenced this pull request Sep 5, 2018
…apparmor

Enable apparmor for whonix gateway and workstation vms
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.

5 participants