All code using LOCALSTATEDIR "/run" is updated to use RUNSTATEDIR
instead. The exception is the remote driver client which still
uses LOCALSTATEDIR "/run". The client needs to connect to remote
machines which may not be using /run, so /var/run is more portable
due to the /var/run -> /run symlink.

Some duplicate paths in the apparmor code are also purged.

There's no functional change by default yet since both expressions
expand to the same value.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The way we're processing the return status, using WIFEXITED() and
friends, only works when we have the raw return status; however,
virCommand defaults to processing the return status for us. Call
virCommandRawStatus() before virCommandRun() so that we get the raw
return status and the logic can actually work.

This results in guest startup failures caused by AppArmor issues
being reported much earlier: for example, if virt-aa-helper exits
with an error we're now reporting

  error: internal error: cannot load AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6'

instead of the misleading

  error: internal error: Process exited prior to exec: libvirt:
  error : unable to set AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6'
  for '/usr/bin/qemu-system-x86_64': No such file or directory
Suggested-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Right now we're using the virRun() convenience API, but that
doesn't allow the kind of control we want. Use the virCommand
APIs directly instead.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When a vhost scsi device is hotplugged virt-aa-helper is called to
add the respective path.
For example the config:
  <hostdev mode='subsystem' type='scsi_host' managed='no'>
    <source protocol='vhost' wwpn='naa.50014059de6fba4f'/>
  </hostdev>
Will call it to add:
 /sys/kernel/config/target/vhost//naa.50014059de6fba4f

But in general /sys paths are filtered in virt-aa-helper.c:valid_path
To allow the path used for vhost-scsi we need to add it to the list of
known and accepted overrides.

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1829223Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This requires drivers to opt in to handle the raw modelstr
network model, all others will error if a passed in XML value
is not in the model enum.

Enable this feature for libxl/xen/xm and qemu drivers
Acked-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

To ease converting the net->model value to an enum, add
the wrapper functions:

virDomainNetGetModelString
virDomainNetSetModelString
virDomainNetStreqModelString
virDomainNetStrcaseeqModelString
Acked-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Further testing with more devices showed that we sometimes have a
different depth of pci device paths when accessing sysfs for device
attributes.

But since the access is limited to a set of filenames and read only it
is safe to use a wildcard for that.

Related apparmor denies - while we formerly had only considered:
apparmor="DENIED" operation="open"
  name="/sys/devices/pci0000:00/0000:00:02.1/uevent"
  requested_mask="r"

We now also know of cases like:
apparmor="DENIED" operation="open"
  name="/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/uevent"
  requested_mask="r"

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Further testing with different devices showed that we need more rules
to drive gl backends with nvidia cards. Related denies look like:

apparmor="DENIED" operation="open"
  name="/usr/share/egl/egl_external_platform.d/"
  requested_mask="r"
apparmor="DENIED" operation="open"
  name="/proc/modules"
  requested_mask="r"
apparmor="DENIED" operation="open"
  name="/proc/driver/nvidia/params"
  requested_mask="r"
apparmor="DENIED" operation="mknod"
  name="/dev/nvidiactl"
  requested_mask="c"

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Change fb01e1a4 "virt-aa-helper: generate rules for gl enabled
graphics devices" implemented the detection for gl enabled
devices in virt-aa-helper. But further testing showed
that it will need much more access for the full gl stack
to work.

Upstream apparmor just recently split those things out and now
has two related abstractions at
https://gitlab.com/apparmor/apparmor/blob/master:
- dri-common at /profiles/apparmor.d/abstractions/dri-common
- mesa: at /profiles/apparmor.d/abstractions/mesa

If would be great to just include that for the majority of
rules, but they are not yet in any distribution so we need
to add rules inspired by them based on the testing that we
can do.

Furthermore qemu with opengl will also probe the backing device
of the rendernode for attributes which should be safe as
read-only wildcard rules.

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1815452Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

We have this very handy macro called VIR_STEAL_PTR() which steals
one pointer into the other and sets the other to NULL. The
following coccinelle patch was used to create this commit:

  @ rule1 @
  identifier a, b;
  @@

  - b = a;
    ...
  - a = NULL;
  + VIR_STEAL_PTR(b, a);

Some places were clean up afterwards to make syntax-check happy
(e.g. some curly braces were removed where the body become a one
liner).
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

This will be extended in the future, so let's simplify things by
centralizing the checks.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

This adds the virt-aa-helper support for gl enabled graphics devices to
generate rules for the needed rendernode paths.

Example in domain xml:
<graphics type='spice'>
  <gl enable='yes' rendernode='/dev/dri/bar'/>
</graphics>

results in:
  "/dev/dri/bar" rw,

Special cases are:
- multiple devices with rendernodes -> all are added
- non explicit rendernodes -> follow recently added virHostGetDRMRenderNode
- rendernode without opengl (in egl-headless for example) -> still add
  the node

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085Reviewed-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.

In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.

With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to  find the
author of a particular bit of code.

This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.

The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NYuri Chornoivan <yurchor@ukr.net>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

virt-aa-helper needs to grant QEMU access to VFIO MDEV devices.

This extends commit 74e86b6b which only covered PCI hostdevs for VFIO-PCI
assignment by now also covering vfio MDEVs.
It has still the same limitations regarding the device lifecycle, IOW we're
unable to predict the actual VFIO device being created, thus we need
wildcards.

Also note that the hotplug case, where apparmor is able to detect the actual
VFIO device during runtime, is already covered by commit 606afafb.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>

All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

Currently, the functions return a pointer to the
destination buffer on success or NULL on failure.

Not only does this kind of error handling look quite
alien in the context of libvirt, where most functions
return zero on success and a negative int on failure,
but it's also somewhat pointless because unless there's
been a failure the returned pointer will be the same
one passed in by the user, thus offering no additional
value.

Change the functions so that they return an int
instead.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

This patch extends the AppArmor domain profile with file paths
the swtpm accesses for state, log, pid, and socket files.

Both, QEMU and swtpm, use this AppArmor profile.
Signed-off-by: NStefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>

All callers pass 'false' now so it's no longer needed.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

As the aa-helper binary is supposed to be used only with libvirt, we can
fully remove it.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Currently the driver module loading code does not report an error if the
driver module is physically missing on disk. This is useful for distro
packaging optional pieces. When the daemons are split up into one daemon
per driver, we will expect module loading to always succeed. If a driver
is not desired, the entire daemon should not be installed.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

nvdimm memory is backed by a path on the host. This currently works only via
hotplug where the AppArmor label is created via the domain label callbacks.

This adds the virt-aa-helper support for nvdimm memory devices to generate
rules for the needed paths from the initial guest definition as well.

Example in domain xml:
  <memory model='nvdimm'>
    <source>
      <path>/tmp/nvdimm-base</path>
    </source>
    <target>
     <size unit='KiB'>524288</size>
     <node>0</node>
    </target>
  </memory>
Works to start now and creates:
  "/tmp/nvdimm-base" rw,

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Input devices can passthrough an event device. This currently works only via
hotplug where the AppArmor label is created via the domain label callbacks.

This adds the virt-aa-helper support for passthrough input devices to generate
rules for the needed paths from the initial guest definition as well.

Example in domain xml:
  <input type='passthrough' bus='virtio'>
          <source evdev='/dev/input/event0' />
  </input>
Works to start now and creates:
  "/dev/input/event0" rw,

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

In a recent change b932ed69: "virt-aa-helper: resolve yet to be created
paths" several cases with symlinks in paths were fixed, but it regressed
cases where the file being last element of the path was the actual link.

In the case of the last element being the symlink realpath can (and shall)
be called on the full path that was passed.

Examples would be zfs/lvm block devices like:
 <disk type='block' device='disk'>
    <driver name='qemu' type='raw'/>
    <source dev='/dev/mapper/testlvm-testvol1'/>
    <target dev='vdd' bus='virtio'/>
 </disk>
With the target being:
 /dev/mapper/testlvm-testvol1 -> ../dm-0

That currently is rendered as
 "/dev/mapper/testlvm-testvol1" rwk,
but instead should be (and is with the fix):
 "/dev/dm-0" rwk,

Fixes: b932ed69: "virt-aa-helper: resolve yet to be created paths"
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1756394Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

In certain cases a xml contains paths that do not yet exist, but
are valid as qemu will create them later on - for example
vhostuser mode=server sockets.

In any such cases so far the check to virFileExists failed and due to
that the paths stayed non-resolved in regard to symlinks.

But for apparmor those non-resolved rules are non functional as they
are evaluated after resolving any symlinks.

Therefore for non-existent files and partially non-existent paths
resolve as much as possible to get valid rules.

Example:
   <interface type='vhostuser'>
       <model type='virtio'/>
       <source type='unix' path='/var/run/symlinknet' mode='server'/>
   </interface>

Got rendered as:
  "/var/run/symlinknet" rw,

But correct with "/var/run" being a symlink to "/run" is:
  "/run/symlinknet" rw,
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

The QEMU driver loadable module needs to be able to resolve all ELF
symbols it references against libvirt.so. Some of its symbols can only
be resolved against the storage_driver.so loadable module which creates
a hard dependancy between them. By moving the storage file backend
framework into the util directory, this gets included directly in the
libvirt.so library. The actual backend implementations are still done as
loadable modules, so this doesn't re-add deps on gluster libraries.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

The virt-aa-helper fails to parse the xmls with the memory/cpu
hotplug features or user assigned aliases. Set the features in
xmlopt->config for the parsing to succeed.
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Tested-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

This is now covered by DomainSetPathLabel being implemented in apparmor.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Some globbing chars in the domain name could be used to break out of
apparmor rules, so lets forbid these when in virt-aa-helper.

Also adding a test to ensure all those cases were detected as bad char.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Hot-adding disks does not parse the full XML to generate apparmor rules.
Instead it uses -f <PATH> to append a generic rule for that file path.

580cdaa7: "virt-aa-helper: locking disk files for qemu 2.10" implemented
the qemu 2.10 requirement to allow locking on disks images that are part of
the domain xml.

But on attach-device a user will still trigger an apparmor deny by going
through virt-aa-helper -f, to fix that add the lock "k" permission to the
append file case of virt-aa-helper.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Add helpers that will simplify checking if a backing file is valid or
whether it has backing store. The helper virStorageSourceIsBacking
returns true if the given virStorageSource is a valid backing store
member. virStorageSourceHasBacking returns true if the virStorageSource
has a backing store child.

Adding these functions creates a central points for further refactors.

To avoid any issues later on if paths ever change (unlikely but
possible) and to match the style of other generated rules the paths
of the static rules have to be quoted as well.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

libvirt allows spaces in vm names, there were issues in the past but it
seems not removed so the assumption has to be that spaces are continuing
to be allowed.

Therefore virt-aa-helper should not reject spaces in vm names anymore if
it is going to be refused causing issues then the parser or xml schema
should do so.
Apparmor rules are in quotes, so a space in a path based on the name works.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

If users only specified vendor&product (the common case) then parsing
the xml via virDomainHostdevSubsysUSBDefParseXML would only set these.
Bus and Device would much later be added when the devices are prepared
to be added.

Due to that a hot-add of a usb hostdev works as the device is prepared
and virt-aa-helper processes the new internal xml. But on an initial
guest start at the time virt-aa-helper renders the apparmor rules the
bus/device id's are not set yet:

p ctl->def->hostdevs[0]->source.subsys.u.usb
$12 = {autoAddress = false, bus = 0, device = 0, vendor = 1921, product
= 21888}

That causes rules to be wrong:
  "/dev/bus/usb/000/000" rw,

The fix calls virHostdevFindUSBDevice after reading the XML from
virt-aa-helper to only add apparmor rules for devices that could be found
and now are fully known to be able to write the rule correctly.

It uncondtionally sets virHostdevFindUSBDevice mandatory attribute as
adding an apparmor rule for a device not found makes no sense no matter
what startup policy it has set.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Things moved again, sigh.
Reviewed-By: NJamie Strandboge <jamie@canonical.com>
Michal Privoznik <mprivozn@redhat.com>

Testing qemu-2.10-rc3 shows issues like:
  qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
  7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100

There is an apparmor deny due to qemu now locking those files:
 apparmor="DENIED" operation="file_lock" [...]
 name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"

The profile needs to allow locking for loader and nvram files via
the locking (k) rule.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Testing qemu-2.10-rc2 shows issues like:
  qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest- \
  artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0:
  Failed to lock byte 100

It seems the following qemu commit changed the needs for the backing
image rules:

(qemu) commit 244a5668106297378391b768e7288eb157616f64
Author: Fam Zheng <famz@redhat.com>
    file-posix: Add image locking to perm operations

The block appears as:
 apparmor="DENIED" operation="file_lock" [...]
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-x86" requested_mask="k" denied_mask="k"

With that qemu change in place the rules generated for the image
and backing files need the allowance to also lock (k) the files.

Disks are added via add_file_path and with this fix rules now get
that permission, but no other rules are changed, example:
  -  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rw,
  +  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rwk
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>