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

libvirt: apparmor denies reading from /var/lib/libvirt/images/ #2083

Open
funwun opened this Issue Jul 31, 2017 · 27 comments

Comments

Projects
None yet
@funwun
Copy link

funwun commented Jul 31, 2017

Issue Report

Bug

Container Linux Version

CoreOS: 1409.7.0

libvirt xml file

<?xml version="1.0"?>
<domain xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0" type="kvm">
  <name>test10</name>
  <uuid>3eee086d-511b-4488-a60c-04ea4b6a4959</uuid>
  <memory>1048576</memory>
  <currentMemory>1048576</currentMemory>
  <vcpu>1</vcpu>
  <os>
    <type arch="x86_64">hvm</type>
    <boot dev="hd"/>
  </os>
  <features>
    <acpi/>
    <apic/>
  </features>
  <cpu mode="custom" match="exact">
    <model>Skylake-Client</model>
  </cpu>
  <clock offset="utc">
    <timer name="rtc" tickpolicy="catchup"/>
    <timer name="pit" tickpolicy="delay"/>
    <timer name="hpet" present="no"/>
  </clock>
  <on_poweroff>destroy</on_poweroff>
  <on_reboot>restart</on_reboot>
  <on_crash>restart</on_crash>
  <pm>
    <suspend-to-mem enabled="no"/>
    <suspend-to-disk enabled="no"/>
  </pm>
  <devices>
    <emulator>/usr/bin/kvm-spice</emulator>
    <disk type="file" device="disk">
      <driver name="qemu" type="qcow2" cache="writeback"/>
      <source file="/var/lib/libvirt/images/coreos_production_qemu_image.img"/>
      <target dev="vda" bus="virtio"/>
    </disk>
    <controller type="usb" index="0" model="ich9-ehci1"/>
    <controller type="usb" index="0" model="ich9-uhci1">
      <master startport="0"/>
    </controller>
    <controller type="usb" index="0" model="ich9-uhci2">
      <master startport="2"/>
    </controller>
    <controller type="usb" index="0" model="ich9-uhci3">
      <master startport="4"/>
    </controller>
    <interface type="bridge">
      <source bridge="virbr0"/>
      <mac address="52:54:00:2e:c8:87"/>
      <model type="virtio"/>
    </interface>
    <input type="mouse" bus="ps2"/>
    <console type="pty"/>
  </devices>
  <qemu:commandline>
    <qemu:arg value="-fw_cfg"/>
    <qemu:arg value="name=opt/com.coreos/config,file=/var/lib/libvirt/images/config.ign"/>
  </qemu:commandline>
</domain>

QEMU emulator version 2.8.1.1

-rwxrwxrwx 1 libvirt-qemu kvm 262 Jul 31 12:49 config.ign

I get this error when I start vm
error: internal error: process exited while connecting to monitor: 2017-07-31T04:52:22.223191Z qemu-system-x86_64: -fw_cfg name=opt/com.coreos/config,file=/var/lib/libvirt/images/config.ign: can't load /var/lib/libvirt/images/config.ign

And I use this script: https://gist.github.com/euank/7c030ce123fc35d71361c25b69abe3d0, same result

@euank

This comment has been minimized.

Copy link
Contributor

euank commented Jul 31, 2017

Total shot in the dark, but if you have selinux enabled it might be denying it.

@funwun

This comment has been minimized.

Copy link
Author

funwun commented Jul 31, 2017

Host: Ubuntu 17.04 x64
SELinux status: disabled

grep CONFIG_FW_CFG_SYSFS /boot/config-$(uname -r)

CONFIG_FW_CFG_SYSFS=m
@lucab

This comment has been minimized.

Copy link
Member

lucab commented Jul 31, 2017

Does var/lib/libvirt/images/config.ign actually exist and is it a valid configuration?

@kudramh

This comment has been minimized.

Copy link

kudramh commented Aug 27, 2017

removed apparmor and kvm started properly

@funwun

This comment has been minimized.

Copy link
Author

funwun commented Aug 28, 2017

@lucab sure, the hostname only

@pfremm

This comment has been minimized.

Copy link

pfremm commented Sep 12, 2017

I also have this same issue. Ignition file exists and is valid. Here is version information from virsh. I'm on ubuntu 16.04
root@server:~/Documents/libvirt-3.7.0# virsh -c qemu:///system version
Compiled against library: libvirt 2.2.0
Using library: libvirt 2.2.0
Using API: QEMU 2.2.0
Running hypervisor: QEMU 2.6.2

@pfremm

This comment has been minimized.

Copy link

pfremm commented Sep 12, 2017

My issue ended loading the file ended up up being with /etc/libvirt/libvirtd.conf and needing to turn security off and then restarting libvirtd service.

@euank

This comment has been minimized.

Copy link
Contributor

euank commented Oct 12, 2017

@funwun did you ever figure this out?
Could you check if it was an apparmor issue? It looks like their wiki has good information, and checking if temporarily disabling apparmor makes a difference could determine if it's causing this issue.

@pfremm

This comment has been minimized.

Copy link

pfremm commented Oct 12, 2017

Yeah you are correct it is. If I check the app armor status I can see libvirt in enforcing mode:

apparmor module is loaded.
31 profiles are loaded.
31 profiles are in enforce mode.
   /sbin/dhclient
   /usr/bin/evince
   /usr/bin/evince-previewer
   /usr/bin/evince-previewer//sanitized_helper
   /usr/bin/evince-thumbnailer
   /usr/bin/evince-thumbnailer//sanitized_helper
   /usr/bin/evince//sanitized_helper
   /usr/lib/NetworkManager/nm-dhcp-client.action
   /usr/lib/NetworkManager/nm-dhcp-helper
   /usr/lib/connman/scripts/dhclient-script
   /usr/lib/cups/backend/cups-pdf
   /usr/lib/libvirt/virt-aa-helper
   /usr/lib/lightdm/lightdm-guest-session
   /usr/lib/lightdm/lightdm-guest-session//chromium
   /usr/lib/snapd/snap-confine
   /usr/lib/snapd/snap-confine//mount-namespace-capture-helper
   /usr/lib/telepathy/mission-control-5
   /usr/lib/telepathy/telepathy-*
   /usr/lib/telepathy/telepathy-*//pxgsettings
   /usr/lib/telepathy/telepathy-*//sanitized_helper
   /usr/lib/telepathy/telepathy-ofono
   /usr/sbin/cups-browsed
   /usr/sbin/cupsd
   /usr/sbin/cupsd//third_party
   /usr/sbin/ippusbxd
   /usr/sbin/libvirtd
   /usr/sbin/ntpd
   /usr/sbin/tcpdump
   docker-default
   webbrowser-app
   webbrowser-app//oxide_helper```
@purplesrl

This comment has been minimized.

Copy link

purplesrl commented Oct 23, 2017

+1 apparmor

[1117701.593979] audit: type=1400 audit(1508750831.558:441): apparmor="DENIED" operation="open" profile="libvirt-e2f1220a-5dd3-445d-8384-78e41f13fc6f" name="/var/lib/libvirt/images/node3.ign" pid=16870 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=106 ouid=0

Fixed by adding

  /var/lib/libvirt/images/* r,

In /etc/apparmor.d/abstractions/libvirt-qemu

@joshuacox

This comment has been minimized.

Copy link

joshuacox commented Jan 26, 2018

+1 on adding

/var/lib/libvirt/images/* r,

to /etc/apparmor.d/abstractions/libvirt-qemu and then systemctl restart apparmor.service

@lucab lucab changed the title -fw_cfg name=opt/com.coreos/config,file=/var/lib/libvirt/images/config.ign: can't load /var/lib/libvirt/images/config.ign libvirt: apparmor denies reading from /var/lib/libvirt/images/ Apr 11, 2018

@lucab

This comment has been minimized.

Copy link
Member

lucab commented Apr 11, 2018

So the problem is that our doc examples use /var/lib/libvirt/images/ for images and ignition config, but in some distros apparmor denies reading from there.

We should either find an allowed readable path and fix our the doc, or ask upstream to consider an entry for our usecase. For reference, the current upstream apparmor example config doesn't seem to offer us many options: https://github.com/libvirt/libvirt/blob/4d7384eb9ddef2008cb0cc165eb808f74bc83d6b/examples/apparmor/libvirt-qemu

/cc @cpaelzer @berrange

@berrange

This comment has been minimized.

Copy link

berrange commented Apr 11, 2018

The /var/lib/libvirt/images directory is the default path that libvirt defines for storage. Any distro that has a policy denying use of that directory is seriously broken and needs to be fixed.

@cpaelzer

This comment has been minimized.

Copy link

cpaelzer commented Apr 11, 2018

I agree that we have the default paths upstream and those should work everywhere.
For Ubuntu as an example there also are a few additional Ubuntu only paths I never brought upstream (because they make no sense for the overall community).
But I agree to @berrange that the default paths should always work.

@lucab

This comment has been minimized.

Copy link
Member

lucab commented Apr 11, 2018

@cpaelzer then I am not sure anymore, but people in this thread seem to be reporting that the ubuntu apparmor policy is denying that path. But I'm not familiar with apparmor nor libvirt, so I may be missing some details.

For clarity, we don't ship libvirt in ContainerLinux, and this ticket is about running our images via libvirt on other distros (two of them being ubuntu 16.04 and 17.04).

@cpaelzer

This comment has been minimized.

Copy link

cpaelzer commented Apr 11, 2018

Odd, let me outline how it is supposed to work.

Seeing that the workaround was to add to /etc/apparmor.d/abstractions/libvirt-qemu implies that not libvirt but the guest (=qemu) wasn't able to access the file.

virt-aa-helper is allowed to read these paths:

$ grep image /etc/apparmor.d/usr.lib.libvirt.virt-aa-helper 
  # For backingstore, virt-aa-helper needs to peek inside the disk image, so
  /var/lib/libvirt/images/ r,
  /var/lib/libvirt/images/** r,
  # nova base images (LP: #907269)
  /var/lib/nova/images/** r,
  /var/lib/uvtool/libvirt/images/** r,

Now when a guest is spawned and apparmor is set as the security modules two things happen:

  1. the guest gets created a custom apparmor profile by virt-aa-helper
    1. this includes the general allowance from /etc/apparmor.d/abstractions/libvirt-qemu
    2. it has entries for disks and devices of all sorts matching the definition of the guest
    3. later hotplug or other events call from libvirt to virt-aa-helper to append rules for these
  2. the guest (=qemu) gets started under that profile

In case a user is reporting an apparmor deny against that I'd assume for their case virt-aa-helper was unable to add the rule.
In those cases (e.g. the one by @purplesrl above with node3.ign) report it upstream (or here for this particular case).
Feel free to CC me - for those cases one would want to look at:

  1. the Deny from dmesg
  2. the related apparmor files which would be
    1. /etc/apparmor.d/abstractions/libvirt-qemu
    2. per guest file /etc/apparmor.d/libvirt/libvirt-$(virsh dominfo | awk '/UUID/ {print $2}').files
  3. the libvirt xml describing the guest virsh dumpxml <guestname>

To shortcut that for the case here this is a user/config error and not a libvirt bug, let me outline why.
At least the initial report had an xml with an .ign file and for now I assume that is how everyone does it.
Like:

  <qemu:commandline>
    <qemu:arg value="-fw_cfg"/>
    <qemu:arg value="name=opt/com.coreos/config,file=/var/lib/libvirt/images/config.ign"/>
  </qemu:commandline>

That is a manual qemu commandline addition outside of the scope what libvirt manages.
That most likely is the reason you are failing.
virt-aa-helper doesn't care about those "out of scope" entries so nothing adds a rule for this path.
If one (or some automation) adds such a path out-of-scope it needs to add the path as well to the apparmor rules.

Unfortunately there are no per-guest overrides (yet - I'm working on it as soon as confidtional if works in apparmor). So for now one has to edit /etc/apparmor.d/abstractions/libvirt-qemu to match what one added to the guest cmdline.

But for that I'd strictly vote against:
/var/lib/libvirt/images/* r,
This would allow ALL guests (until you can override per-guest) to read ALL files in there.
This is totally not what you want.
Until you have per guest overrides, at least make the rule a full path, so in this case
/var/lib/libvirt/images/config.ign r,

Or if you like maybe
/var/lib/libvirt/images/*.ign r,

That would mean a malicious attacker breaking out of a guest can at least only read those files instead of all other guests images.

To allow to fix that in libvirt we would need one of two things

  1. per guest override (in progress - will allow to add the rule just for the right guest)
  2. some generic tag that means, you don't want to let libvirt add any devices, but for consideration to e.g. the security modules to allow access
@berrange

This comment has been minimized.

Copy link

berrange commented Apr 11, 2018

Thanks for highlighting the -fw_cfg parameter usage here.

I should point out that although this is exposed to the end user via -fw_cfg, and looks like a convenient way to get information into the guest OS, this is generally discouraged by QEMU developers. The principal purpose of the "fw_cfg" mechanism is communication between QEMU and the firmware. There are a fairly limited number of slots available for passing information, and so long term there's no guarantee that there will be spare slots available for end user application usage in this way. Thus we're not likely to expose '-fw_cfg' in libvirt XML explicitly.

Recognising the usefulness of this conceptual approach though, I recently made it possible to use the SMBIOS "OEM strings" facility in QEMU and libvirt (https://libvirt.org/formatdomain.html#elementsSysinfo) to pass data into the guest. This will be available in QEMU 2.12, and libvirt 4.1.0. We're encouraging applications to include an application specific name prefix in the OEM strings entry so they can co-exist eg

<oemStrings>
      <entry>cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/</entry>
      <entry>anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os</entry>
</oemStrings>

So I'd encourage support of "OEM strings" as an alternative option to "fw_cfg", with eventual discontinuation of fw_cfg, once support for OEM strings is widely available in distros (will take a few years to penetrate)

@cpaelzer

This comment has been minimized.

Copy link

cpaelzer commented Apr 11, 2018

Thanks Daniel - good to know,
OEM strings are nice, but they are too generic to parse paths out of them right?
So virt-aa-helper still can't know when to derive a path to add it to the rules (let me know you think there is a reasonable way to detect a path is meant in those cases).

@berrange

This comment has been minimized.

Copy link

berrange commented Apr 11, 2018

@cpaelzer nothing todo from security pov since the data is passed inline, no references to external files on disk, so there is nothing to grant access to.

@lucab

This comment has been minimized.

Copy link
Member

lucab commented Apr 11, 2018

Thanks both for the detailed insights!

@berrange we'll look into OEM strings for the future. "fw_cfg" is being used by ignition for passing provisioning configuration since some time.

@cpaelzer then I guess we'll suggest adding a wildcard rule for ignition files to our doc. If per-guest overrides are under work, it would be nice in the future to have some standardized path for user provided guest-read-only stuff (either generically or specific for fw_cfg).

@cpaelzer

This comment has been minimized.

Copy link

cpaelzer commented Apr 11, 2018

I was unsure on your suggestion, so I polled for opinions on the libvirt mailing list

@dgonyeo

This comment has been minimized.

Copy link

dgonyeo commented Apr 12, 2018

@berrange I built qemu 2.12-rc3 and added a file to a vm with -smbios type=11,value=ignition:"$(sed -e 's/,/,,/g' ~/test.ign)", and accessed it via sudo dmidecode --oem-string 1, but it seems to be getting truncated to 1024 characters. Is this an expected limitation, or is dmidecode the one shortening it here?

Ignition configs definitely end up being bigger than 1 KB, so that's not really a workable replacement if we can't up that limit.

Also is there a chance of being able to reference a file (something like -smbios type=11,file=~/test.ign) in place of including the value on the command line? Ignition configs can have newlines and it would be great to be able to avoid the comma replacement step.

@berrange

This comment has been minimized.

Copy link

berrange commented Apr 13, 2018

@dgonyeo thanks for testing that, you've uncovered a horrible bug in QEMU. While SMBIOS has no limit on string length, QEMU's command line option parser is arbitrarily truncating options at 1024 bytes, with no warning, silently discarding any further data. I'm going to fix that to actually report an error message instead of continuing to run with garbage data. So yeah, unfortunately this means you can't use SMBIOS right now :-( Your suggestion to allow reference to an external file is a useful one, so I'll look at that too.

@bgilbert

This comment has been minimized.

Copy link
Member

bgilbert commented Apr 26, 2018

For the record, patch series removing length limits in QEMU command-line parsing. The changes did not make QEMU 2.12.

@cgwalters

This comment has been minimized.

Copy link
Member

cgwalters commented Jul 27, 2018

I think libvirt should do what I did in cgwalters/playground@f2b67e5#diff-f1a5248af69a24af73c52a6b2e565df6R75 and pass the config as a file descriptor. Since libvirt uses C it could even use memfd or O_TMPFILE rather than an unlinked real file.

@bgilbert

This comment has been minimized.

Copy link
Member

bgilbert commented Oct 31, 2018

The length limits were removed in QEMU 3.0.

@bgilbert

This comment has been minimized.

Copy link
Member

bgilbert commented Nov 1, 2018

Support for OEM strings filed as coreos/ignition#656.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.