Updates Whonix-based templates 14 -> 15 #358
Conversation
Leveraging the script logic by @emkll to update the Whonix-based SDW components in-place, including shutting them down if necessary in order to clean up.
conorsch
force-pushed
the
300-update-to-whonix-15
branch
from
d71948f to
d1fad9c
Compare
All SDW components based on Whonix (Workstation & Gateway) should be based on the 15 series. These changes achieve that. In order to handle update-in-place (as opposed to a clean install), we'll have to script that separately. Includes updates to the config tests to verify the proper base templates are used.
The Python concurrent package was only necessary for a prior version of Salt, when running under Python2. Both Saltstack and Qubes have updated the relevant dependencies so that we don't need to ensure this package is present any longer.
Via the `qvm.anon-whonix` SLS file, we pull in all of the following: * qvm.anon-whonix * qvm.template-whonix-ws * qvm.sys-whonix * qvm.whonix-ws-dvm * qvm.sys-firewall The value of this dependency chain is that we can concisely ensure that all Whonix-related machines, both AppVMs and TemplateVMs, are up to date. There's an edge case where already-running VMs, such as `sys-whonix` (which is autostart=True by default) may not have their "template" config field updated, since changing that value while a TemplateVM is running is forbidden. Still, let's leverage the upstream config as much as possible, and we'll continue to handle the shutdown-to-change-template settings in our own scripts, as necessary.
Most of the config tests to date, reasonably, target the SDW-managed VMs. As we attempt to ensure up-to-date components across the board, we must also ensure that Qubes-provided VM configures are updated. These include: * sys-usb * sys-net * sys-firewall * sys-whonix If we check those VMs to match our expected definition of "up-to-date" templates, then we should have rather solid coverage. It's not as complete as testing for e.g. "any fedora-based VM should be based on the latest fedora template". Ideally we'll get to that point, but for now, that could report failures of components unrelated to the SDW config.
conorsch
force-pushed
the
300-update-to-whonix-15
branch
from
d1fad9c to
d49f1d9
Compare
|
During pre-review discussion, we weren't certain whether importing the upstream Qubes Salt config for the Whonix VMs would guarantee 15, rather than 14. Judging by the following, however:
It appears that dom0 updates will indeed move this value from 14 -> 15. |
There was a problem hiding this comment.
Thanks @conorsch ! This worked for me in a very specific scenario, but based on my testing on another Qubes workstation, it may not work in all cases (specifically if whonix-15 templates were not present/configured before):
- As part of this upgrade, we will also want to invoke the sls:
qvm.sys-whonix. This will ensuresys-whonixis up-to-date (which is important, since it is the updateVM for any whonix-based templates).qvm.anon-whonixdoes not perform this change. There are also some other options we could consider here:
- Replace the updateVM for all Whonix templates to sd-whonix
- eliminate sd-whonix and use sys-whonix instead (reducing by 1 the number of VMs on each system), but this decision depends assumptions on whether or not the workstation will be used for other purposes, requiring a larger conversation
-
I have observed (but cannot reproduce) an error that occurred when running
make cleanon this branch, after provisioning from this branch (while upgrading) from a fresh provisioning on master. I suspect it may have been a flake, as sd-whonix could not be destroyed as it would automatically reboot as it was being killed. This was because sd-proxy was not killed/destroyed, which is strange because the API returns the domains in alphabetical order. I don't think we should block merge here, but curious to see if you've experienced this issue (see [1] below) -
There are two comments inline, those changes were required to get the securedrop-handle-upgrade script to work properly on this branch.
Unrelated to this PR, make clean will fail if fedora-30-dvm doesnt exist. Setting the default DispVM to '' in the make clean would resolve (or searching for a default dvm template), opened #360 to track
[1] :
[user@dom0 securedrop-workstation]$ make clean
Deploying Salt config...
local:
----------
sd-workstation.top:
----------
status:
unchanged
qubes-prefs default_dispvm fedora-30-dvm
./scripts/destroy-vm --all
qvm-remove: error: Domain is not powered off: 'sd-proxy'
Destroying VM 'sd-proxy'... Traceback (most recent call last):
File "./scripts/destroy-vm", line 78, in <module>
destroy_all()
File "./scripts/destroy-vm", line 68, in destroy_all
destroy_vm(vm)
File "./scripts/destroy-vm", line 46, in destroy_vm
subprocess.check_call(["qvm-remove", "-f", vm.name])
File "/usr/lib64/python3.5/subprocess.py", line 271, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['qvm-remove', '-f', 'sd-proxy']' returned non-zero exit status 1
Makefile:128: recipe for target 'destroy-all' failed
make: *** [destroy-all] Error 1
[user@dom0 securedrop-workstation]$ qvm-shutdown sd-svs
[user@dom0 securedrop-workstation]$ make clean
dom0/securedrop-handle-upgrade
Outdated
|
|
||
| if qvm-check --quiet sd-whonix; then | ||
| BASE_TEMPLATE=$(qvm-prefs sd-whonix template) | ||
| if [[ ! $BASE_TEMPLATE =~ "buster" ]]; then |
There was a problem hiding this comment.
Since TemplateVM for sd-whonix should be whonix-gw-{14,15}, I think he we would want to compare BASE_TEMPLATE to '14'. otherwise sd-whonix will shut down on every run.
| - label: blue | ||
| - tags: | ||
| - add: | ||
| - sd-workstation | ||
| - sd-buster |
There was a problem hiding this comment.
I think we must require sls sd-upgrade-templates here , to ensure the template update was successful before cloning whonix-ws-15 to sd-proxy-buster-template
|
Great comments, @emkll. Will take another pass through and ping for re-review when ready.
My understanding is that importing the qvm.anon-whonix SLS does indeed result in qvm.sys-whonix being included, as well. See commit message in 15f5914 . If those statements aren't accurate, then yes we should indeed make an update here. Will monitor for specific tasks in the output when re-retesting this branch to confirm the behavior described in that commit. |
Addressed review comments by @emkll. Specifically * Check for "15" string in Whonix-based template checks, since "buster" doesn't occur in those template names * Require the upgrade state before configuring sd-proxy
|
@emkll Pushed up a tiny commit to address the changes you pointed out. Regarding the Note that the |
Based on our approach in #331, I think that addressing this issue in this PR would make the most sense, since it is
What do you think? |
Shuts down the sys-whonix template to ensure we can update the template settings. The Qubes-provided Salt logic only enforces template versions on first config of the sys-whonix VM; if it already exists, it will hang back on an older version of Whonix. That's unacceptable for us, so we'll add a reference to the "handle-upgrade" logic to shut it down, then via a more aggressive Salt task, enforce the new template setting.
|
Ready for your input again, @emkll. The 14 -> 15 transition is also handled now for |
|
Testing this one. |
|
Actually on |
|
@kushaldas if you make clean and make all on this branch, do you get the same error? It seems like the error you report is related to #352 (comment) , can you confirm you are pulling the latest buster template and that your dom0 salt packages are up-to-date?
|
|
My qubes is not letting me remove Buster (so that I can reinstall), saying the following vms are dependent on it: and nothing, next prompt in bash
Also, in this branch I got |
There was a problem hiding this comment.
Thanks for the changes @conorsch , I have tested in two separate scenarios:
- a clean install,
make allandmake testcomplete successfully, all tests run/pass - upgrade install (with
sys-whonixbased on whonix-14-gw):make allandmake testcomplete successfully, all tests run/pass
Diff looks good to me (see minor comment inline)
| if qvm-check --quiet sys-whonix; then | ||
| BASE_TEMPLATE=$(qvm-prefs sys-whonix template) | ||
| if [[ ! $BASE_TEMPLATE =~ "15" ]]; then | ||
| if qvm-check --quiet --running sys-whonix; then |
There was a problem hiding this comment.
nit: weird indentation for sys-whonix here, makes it a bit hard to read
There was a problem hiding this comment.
@emkll Ack, this is tabs-versus-spaces. Most of the scripts in the repo use 4 spaces for bash scripts, but this one and only this one uses tabs instead—I wasn't careful about making sure to force use of tabs when editing the script. Agreed, we should indeed clean it up to avoid large amounts of frustration. 😃
There was a mix of tabs and spaces as a result of collaboration. Since most other scripts in this repo use spaces, rather than tabs, let's stick with that to make the decision easy.
|
I ran through the test plan and hit an error after checking out the pr branch and running hmmmm... think I'm forgetting something but I'm following the Test Plan :/ |
|
ah, looks like setting the environment variables (again) fixes it |
on the second And these are the vms that are running: |
|
Thanks for the report @creviera , looks like there might be a race condition I suspect |
|
Thanks @emkll, I put in the sleep, and reran |
|
Finally |
|
My salt dump from dom0:
|
The qvm-kill task returns rather quickly, before the target machine is completely halted. Proceeding on to other tasks when the killed VM is still coming down will lead to subsequent errors, in this case: QubesVMNotHaltedError: Cannot change template while qube is running Use "sleep" to wait a bit for the machine to come down.
|
Added the requested "sleep" task based on @creviera's testing. @emkll please take another look! |
|
It seems like the salt package we are running are the same. In in |
There was a problem hiding this comment.
Tested upgrade scenario locally once again and works as expected, with all test passes. Visual diff lgtm. Thanks @conorsch for the changes and @creviera and @kushaldas for reviewing.
I am going to merge this PR since it appears the issue is limited to @kushaldas, and opened a follow-up ticket to track, investigate and if required, address these: #366
Status
Ready for review.
Closes #300. Closes #306.
Description
Updates all Whonix-related VMs to use Whonix 15 as base, rather than the EOL Whonix 14. Leverages the new upgrade-in-place logic introduced in #352.
Test plan
The upgrade-in-place scenario is the most important to test, since we want to ensure that automatic upgrades will handle the transition smoothly.
make clone && make clean && make allmake clone && make allmake testConfirm no errors.