Fix connecting to minimal qubes as root

[bcduggan]

Ansible "become" modules don't allow becoming root on qubes without the qubes-core-agent-passwordless-root package installed, like minimal qubes, by design. Users should use the remote_user to login as root on minimal qubes. However, this functionality is broken due to a number of issues in the connection module and documentation.

This PR...

• Updates the qubes.VMRootShell policy in the README
• Adds valid choices for the remote_user option (discussion)
• Chooses the appropriate shell RPC for the given user

To login as root on minimal qubes, users must set remote_user: root on tasks and plays.

Closes QubesOS/qubes-issues#9939
Closes QubesOS/qubes-issues#9934
Closes QubesOS/qubes-issues#9938

[marmarek]

PipelineRetry

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.03%. Comparing base (f4ebe75) to head (bd2d72c).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   69.04%   70.03%   +0.99%     
==========================================
  Files           1        1              
  Lines         504      504              
==========================================
+ Hits          348      353       +5     
+ Misses        156      151       -5     

Flag	Coverage Δ
unittests	70.03% <ø> (+0.99%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fepitre]

@bcduggan are you able to add tests for the connection part? If not, I would manage to add some on top of you branch to validate it works without further breakages.

[bcduggan]

@fepitre Absolutely, I'll add tests as soon as I can.

[bcduggan]

@fepitre I added tests and a new fixture to create a minimalvm, tests/conftest.py::minimalvm. The tests succeed on my Qubes OS 4.2 test machine, but they fail in CI.

It looks like there's no minimal template available in the runner. I tried fedora-41-minimal and debian-12-minimal. Is that the source of the test failures in CI? How can a minimal template be added to the CI environment?

[marmarek]

Indeed minimal templates are not installed there. The tests are running in a fresh default installation of Qubes OS.
As a simple solution, you can install it just before running the test - see .gitlab-ci.yml, add qvm-template install --enablererepo=*testing debian-12-minimal to the before_script list.

[bcduggan]

Adding the minimal template to the test host worked great. Now a couple of connection tests actually fail.

The CLI tests for connecting with qubes.VMRootShell (remote_user: root) for both default VM and minimal VM fail in the same way. It looks like a permission error.

It feels like the mgmtvm policy for qubes.VMRootShell in the test environment might not set the user parameter to root. Where can I check the qrexec policy for the mgmtvm in the test environment?

[marmarek]

I have a better idea:QubesOS/qubes-core-agent-linux#579

[marmarek]

Can you rebase the PR to resolve conflicts?

[marmarek]

black complains: https://gitlab.com/QubesOS/qubes-ansible/-/jobs/10557873859

[marmarek]

The test fails... I did checked that qubes.VMRootShell service works correctly now even without user=root in the policy (with the debian-12-minimal template that gets installed there). So, I guess it still uses qubes.VMShell? Or maybe it fails for some other reason?

[marmarek]

Actually, is the test using debian-12-minimal template? Or default template? I think it's the default template, which would explain why it fails (default template in CI don't have relevant update included yet).

[bcduggan]

I wasn't sure the test should succeed, yet, actually. Sorry, I should have asked.

The test fails... I did checked that qubes.VMRootShell service works correctly now even without user=root in the policy (with the debian-12-minimal template that gets installed there). So, I guess it still uses qubes.VMShell? Or maybe it fails for some other reason?

I have only been able to test on 4.2 so far. I verified that it uses qubes.VMRootShell there before the rebase.

Actually, is the test using debian-12-minimal template? Or default template? I think it's the default template, which would explain why it fails (default template in CI don't have relevant update included yet).

I also verified that the test VM that test_cli::test_minimalvm_connection creates uses the debian-12-template, again on 4.2 and before the rebase. I also verified that the tests pass with force-user='root' in /etc/qubes/rpc-config/qubes.VMRootShell instead of user=root in the actual policy with VMs based on both minimal and default templates.

I haven't been able to locally test the rebased branch since qubesadmin.device_protocol isn't available on 4.2. I'll set up a 4.3 system from a signed weekly build and test there.

[marmarek]

The test_vm_connection test doesn't seem to use minimalvm fixture, so it's likely using default template, not the minimal one. And non-minimal template doesn't have that update yet. Don't worry, tomorrow there should be updated base image with the template updated, so the non-minimal should have fixed qubes.VMRootShell service too. I'll let you know if the test still fails after the update.

[bcduggan]

I see what you mean. Right, test_vm_connection uses the default template, test_minimalvm_connection uses a minimal template.

They both fail the remote_user: root test in the same way. It looks like qubes.VMRootShell on minimal VMs doesn't automatically use root right now, either?

[marmarek]

CI shows only one failure, test_minimalvm_connection passed: https://gitlab.com/QubesOS/qubes-ansible/-/jobs/10559665509

[bcduggan]

Oh, fantastic. I could swear CI showed two failures after I pushed the rebased (and then re-blacked) branch.

Great. Thanks, @marmarek !

[marmarek]

And now it's all green! :)

[marmarek]

@fepitre this appears to work now. AFAIR you raised a concern regarding a remote_user parameter instead of using become: True, do you request any change here?

[marmarek]

Note the remote_user appears to be standard attribute: https://docs.ansible.com/ansible/latest/inventory_guide/connection_details.html

[bcduggan]

Also calling attention to the test for setting tags on a VM in test_cli::test_properties_and_tags_playbook, which this PR currently disables. This PR enables ansible-playbook's JSON stdout callback to allow the become: true and remote_user: invalid_user tests to make sure they fail for the right reasons. But the JSON stdout callback also broke the tags test. I don't want to commit commented-out code, but I'm not sure how to handle this here.

The tags test looks for applied tags in the Ansible output, but I don't believe the qubesos module's status command currently reports a VM's tags. I think the tags test actually finds its tags in the "invocation" JSON that ansible-playbook includes with -vvv. So the tags test passes, but for the wrong reason.

If that functionality doesn't belong in the qubesos module, it might fit better in a qvm_tags_info module, like this qvm_prefs_info module.

Documented in QubesOS/qubes-issues#10010

[fepitre]

Also calling attention to the test for setting tags on a VM in test_cli::test_properties_and_tags_playbook, which this PR currently disables. This PR enables ansible-playbook's JSON stdout callback to allow the become: true and remote_user: invalid_user tests to make sure they fail for the right reasons. But the JSON stdout callback also broke the tags test. I don't want to commit commented-out code, but I'm not sure how to handle this here.

The tags test looks for applied tags in the Ansible output, but I don't believe the qubesos module's status command currently reports a VM's tags. I think the tags test actually finds its tags in the "invocation" JSON that ansible-playbook includes with -vvv. So the tags test passes, but for the wrong reason.

If that functionality doesn't belong in the qubesos module, it might fit better in a qvm_tags_info module, like this qvm_prefs_info module.

Documented in QubesOS/qubes-issues#10010

I'll manage that point.