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

nixos/libvirtd: add option to run qemu as non-root #37281

Merged
merged 1 commit into from Aug 10, 2018

Conversation

xeji
Copy link
Contributor

@xeji xeji commented Mar 17, 2018

Motivation for this change

In current NixOS, libvirtd runs qemu processes as root by default. This increases security risk and is not necessary for most applications.

Users can of course manually modify the user for qemu processes using the virtualisation.libvirtd.qemuVerbatimConfig option, but that is rather inconvenient.

This change adds a new NixOS option virtualisation.libvirtd.qemuRunAsRoot.
If false (the default), qemu is run as a non-privileged user qemu-libvirtd.

Other distros with a similar default behaviour include Debian, Arch and Fedora.

Edit: As per discussion below, this now implements a first step by adding the qemuRunAsRoot option but setting it to false by default to avoid permission issues with existing guests.
The plan is to change the default to true later in a separate PR to ease the transition.

Things done

Manually tested.


/cc @fpletz @volth

@xeji
Copy link
Contributor Author

xeji commented Mar 18, 2018

yes, there are some cases that require qemu to run as root. another one is passthrough file ownership and permission of 9p mounts.

@xeji
Copy link
Contributor Author

xeji commented Apr 12, 2018

rebased and solved merge conflict

@xeji
Copy link
Contributor Author

xeji commented Aug 9, 2018

@peterhoeg this contributes to #41092 and has been around for a while so I would like to see it merged for 18.09. Any thoughts?

@peterhoeg
Copy link
Member

Does it work with DynamicUser = true; instead of the defined user?

@xeji
Copy link
Contributor Author

xeji commented Aug 10, 2018

No, qemu doesn't run as a systemd service. It is forked by libvirtd when a guest is started. Also, some persistent files (disk image, nvram, save files,...) will be owned by the user qemu runs as, so it must be static.

@xeji
Copy link
Contributor Author

xeji commented Aug 10, 2018

Yes, one-time manual intervention (chown) may be required. We can document this in release notes.

If we believe it's more important to avoid manual intervention, we can merge this with qemuRunAsRoot default to true. I'm fine with that. No problems but little impact. Most people won't even know the new option exists.

We could also change the default to false on new installs depending on stateVersion. Gives a smoother transition but is unnessarily complicated and can confuse people, so I don't really like it.

@xeji
Copy link
Contributor Author

xeji commented Aug 10, 2018

@volth good idea, thanks. So I'll change this PR to the non-breaking version and add the second one after this is merged.

New option virtualisation.libvirtd.qemuRunAsRoot (default: true).
If false, qemu is run as non-privileged user qemu-libvirtd,
reducing security risk
@xeji xeji changed the title nixos/libvirtd: don't run qemu as root by default nixos/libvirtd: add option to run qemu as non-root Aug 10, 2018
@xeji xeji merged commit a102b90 into NixOS:master Aug 10, 2018
@teto
Copy link
Member

teto commented Aug 15, 2018

The interactiosn between 9p/libvirt/qemu can be a real pain (dynamic_ownership being a false hope). After hours of fiddling the only configuration that worked for me to have the /nix/store owned by root while retaining the capaibily to share home folders with my VM was to mount these folders as mapped and run qemu as myself.

    qemuVerbatimConfig = ''
      namespaces = []
      user="teto"
      group="users"
      
      # Whether libvirt should dynamically change file ownership
      # to match the configured user/group above. Defaults to 1.
      # Set to 0 to disable file ownership changes.
      dynamic_ownership = 0
    '';

This PR prepends another user such that I have:

      user="qemu-libvirtd
      group="qemu-libvirtd"
      user="teto"
      group="users"

and apparently libvirtd only picks the first user since reverting the PR let my VM start contrary to the behavior with this PR:

[nix-shell:~/nixops]$ nxp start
main> starting...
libvirt: QEMU Driver error : internal error: process exited while connecting to monitor: 2018-08-15T08:19:45.216208Z qemu-system-x86_64: -drive file=/var/lib/libvirt/images/nixops-11975c3a-9f99-11e8-b944-309c233b770e-main.img,format=qcow2,if=none,id=drive-ide0-0-0: Could not open '/var/lib/libvirt/images/nixops-11975c3a-9f99-11e8-b944-309c233b770e-main.img': Permission denied
error: internal error: process exited while connecting to monitor: 2018-08-15T08:19:45.216208Z qemu-system-x86_64: -drive file=/var/lib/libvirt/images/nixops-11975c3a-9f99-11e8-b944-309c233b770e-main.img,format=qcow2,if=none,id=drive-ide0-0-0: Could not open '/var/lib/libvirt/images/nixops-11975c3a-9f99-11e8-b944-309c233b770e-main.img': Permission denied

@xeji
Copy link
Contributor Author

xeji commented Aug 15, 2018

apparently libvirtd only picks the first user

Yes, that seems to be the case. A (somewhat non-intuitive) workaround for your config is to set qemuRunAsRoot = true;. This removes the first set of user/group="qemu-libvirtd", leaving only your user and group from qemuVerbatimConfig .

@xeji xeji deleted the qemu-nonroot branch August 15, 2018 08:56
@xeji
Copy link
Contributor Author

xeji commented Aug 15, 2018

To make this case more intuitive we could put the user and group entries generated by qemuRunAsRoot = false; after the text from qemuVerbatimConfig in the config file to give settings in qemuVerbatimConfig priority.

@teto
Copy link
Member

teto commented Aug 15, 2018

haha right but doesn't look like a long term solution. some other modules allows to specify user/group so that might be better ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants