These files need to be installed on the system for apparmor
support to work, so they don't belong with examples.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
mingw lacks localtime_r(); we were getting it from gnulib. But since
commit acf522e8 stopped linking examples against gnulib, we are
getting a build failure. Keep the examples standalone, and work
around mingw by using the non-reentrant localtime() (safe since our
examples are single-threaded), and add a necessary exemption to our
syntax check.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
mingw lacks sigaction(); we were getting it from gnulib. But since
commit acf522e8 stopped linking examples against gnulib, we are
getting a build failure. Keep the examples standalone, and work
around mingw by using signal() instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
mingw lacks %lld and %zu support in printf(); we were getting it
from gnulib. But since commit acf522e8 stopped linking examples
against gnulib, we are getting a build failure due to -Wformat
flagging these strings. Keep the examples standalone, and work
around mingw by using manual casts to types we can portably print.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit 0c6ad476 updated gnulib, which rearranged some of the
conditions in gnulib wrapper headers such that compilation
started failing on BSD systems when the normal system <unistd.h>
tried to include another system header but instead got a
gnulib wrapper header in an incomplete state; this is because
gnulib headers only work if <config.h> is included first.
Commit b6f78259 papered over the symptoms of that by including
<config.h> in all the examples. But this logic is backwards -
if our examples are truly meant to be stand-alone, they should
NOT depend on how libvirt was configured, and should NOT
depend on the gnulib fixes for system quirks. In particular,
if an example does not need to link against libgnulib.la,
then it also does not need to use -Ignulib in its compile
flags, and likewise does not need to include <config.h> since
none of the gnulib wrapper headers should be interfering.
So, revert (most of) b6f78259 (except for the bogus pre-patch
use of "config.h" in admin/logging.c: if config.h is included,
it should be via <> rather than "", and must be before any
system headers); then additionally nuke all mention of
<config.h>, -Ignulib, and -llibgnu.la, making all of the
examples truly standalone.
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pulling in gnulib just for the <verify.h> header is rather
expensive, especially since that header does not require us
to link against gnulib. It's better to make the event-test
example be standalone by just open-coding a more limited form
of a verify() macro that depends on modern gcc (we have enough
CI coverage that even though the verify is now a no-op in
older setups, we will still notice if we fail to add an event
- as a quick test, I was still able to provoke a compile
failure on Fedora 29 when deleting a line from domainEvents).
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our use of INCLUDES in Makefile.am hearkens back to when we had to
cater to automake 1.9.6 (thanks, RHEL 5) which lacked AM_CPPFLAGS.
Modern Automake flags a warning that INCLUDES is deprecated, and
now that we mandate RHEL 7 or better (see commit c1bc9c66), we no
longer have to cater to the old spelling. This change will also
make it easier to do per-binary CPPFLAGS.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since gnulib commit 6954995d unistd.h is included via stdlib.h
on BSD systems, which requires config.h to be included first.
Add config.h to the files that use it.
Part of this commit reverts commit 6ee918de74
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In 600462834f we've tried to remove Author(s): lines
from comments at the beginning of our source files. Well, in some
files while we removed the "Author" line we did not remove the
actual list of authors.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.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: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When libvirt configuration includes '--with-apparmor-profiles', the
make uninstall target fails
make[1]: Entering directory '/home/jim/upstream/libvirt/examples'
( cd '/etc/apparmor.d//abstractions' && rm -f libvirt-qemu libvirt-lxc )
( cd '/etc/apparmor.d/' && rm -f usr.lib.libvirt.virt-aa-helper usr.sbin.libvirtd )
make[1]: *** No rule to make target 'uninstall-apparmor-local', needed by
'uninstall-local'. Stop.
Add missing 'uninstall-apparmor-local' target to the examples Makefile.am.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Due to kernel upstream change 338d0be4 ("apparmor: fix ptrace read check")
libvirt now hits apparmor denies like:
apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd"
pid=4409 comm="libvirtd" requested_mask="read" denied_mask="read"
peer="libvirt-14e92a75-7668-4b97-8f92-322fc1b9c78a"
Extend the ptrace rule to also allow 'ptrace (read)' for libvirtd to work
with these newer kernels.
Fixes: https://bugs.launchpad.net/bugs/1788603
Reported-by: Thadeu Lima de Souza Cascardo <thadeu.cascardo@canonical.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Libvirt now tries to preserve all mounts under /dev in qemu namespaces.
The old rules only listed a set of known paths but those are no more enough.
I found some due to containers like /dev/.lxc/* and such but also /dev/console
and /dev/net/tun.
Libvirt is correct to do so, but we can no more predict the names properly, so
we modify the rule to allow a wildcard based pattern matching what libvirt does.
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Several cases were found needing /tmp, for example ceph will try to list /tmp
This is a compromise of security and usability:
- we only allow generally enumerating the base dir
- enumerating anything deeper in the dir is at least guarded by the
"owner" restriction, but while that protects files of other services
it won't protect qemu instances against each other as they usually run
with the same user.
- even with the owner restriction we only allow read for the wildcard
path
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
If a guest runs unconfined <seclabel type='none'>, but libvirtd is
confined then the peer for signal can only be detected as
'unconfined'. That triggers issues like:
apparmor="DENIED" operation="signal"
profile="/usr/sbin/libvirtd" pid=22395 comm="libvirtd"
requested_mask="send" denied_mask="send" signal=term peer="unconfined"
To fix this add unconfined as an allowed peer for those operations.
I discussed with the apparmor folks, right now there is no better
separation to be made in this case. But there might be further down the
road with "policy namespaces with scope and view control + stacking"
This is more a use-case addition than a fix to the following two changes:
- 3b1d19e6 AppArmor: add rules needed with additional mediation features
- b482925c apparmor: support ptrace checks
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: intrigeri <intrigeri+libvirt@boum.org>
virt-manager's UI connection will need socket access for openGraphicsFD
to work - otherwise users will face a failed connection error when
opening the UI view.
Depending on the exact versions of libvirt and qemu involved this needs
either a rule from qemu to libvirt or vice versa.
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The most important part is LIBVIRTD_PATH env var fix. It is used
in virFileFindResourceFull() from tests. The libvirtd no longer
lives under daemon/.
Then, libvirtd-fail test was still failing (as expected) but not
because of missing config file but because it was trying to
execute (nonexistent) top_builddir/daemon/libvirtd which
fulfilled expected outcome and thus test did not fail.
Thirdly, lcov was told to generate coverage for daemon/ dir too.
Fourthly, our compiling documentation was still suggesting to run
daemonn/libvirtd.
And finally, some comments in a systemtap file and a probes file
were still referring to daemon/libvirtd.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The filter purpose is to simulate isolated private VLAN.
The behavior can be achieved by limiting network traffic
to traffic between VM and gateway. Because there is no
concept of the PVLAN in the linux bridge.
The filter also contains parts from clean-traffic
to prevent VM from spoofing its IP and MAC address.
To use this filter the user just needs to set
the GATEWAY_MAC variable to gateway MAC address.
Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
These two examples do not use any definition from config.h, and by
removing it these examples can be compiled standalone.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The commit of 1fff379ff6 forgot comma at the end of the line
rendering the file invalid in syntax.
Signed-off-by: Michal Privoznik <mprivozn@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: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The base vfio has not much functionality but to provide a custom
container by opening this path.
See https://www.kernel.org/doc/Documentation/vfio.txt for more.
Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-hotplug
right from the beginning. But if the guest initially had no hostdev at
all it will run into the following deny before the security module
labelling callbacks will make the actual vfio device (like /dev/vfio/93)
known.
Example of such a deny:
[ 2652.756712] audit: type=1400 audit(1491303691.719:25):
apparmor="DENIED" operation="open"
profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
As reported on https://bugs.debian.org/892431, without this rule, when launching
a QEMU KVM instance, an error occurs immediately upon launching the QEMU
process such as:
Could not open backing file: Could not open
'/var/lib/nova/instances/_base/affe96668a4c64ef380ff1c71b4caec17039080e':
Permission denied
The other instance disk images are already covered by the existing rule:
/**/disk{,.*} r
Signed-off-by: intrigeri <intrigeri@boum.org>
Historically we have used a bare lxc:/// URI for connecting to LXC. This
is different from our practice with QEMU, UML, Parallels, Libxl, BHyve
and VirtualBox drivers, which all use a path of '/system' or '/session'
or both.
By making LXC allow '/system', we have fully standardized on the use of
either '/system' or '/session' for all the stateful drivers that run
inside libvirtd.
Support for lxc:/// is of course maintained for back-compat.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Due to mediation of socket and signal activity currently qemu:///session
connections calling qemu_bridge_helper fail.
We need the profile for libvirtd itself and the subprofile for
qemu-bridge-helper to be able to talk/notify to each other via unix socket and
signals.
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1754871
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
If a system has sasl GSSAPI plugin available qemu with sasl support will
try to read /etc/gss/mech.d/.
It is required to allow that to let the modules fully work and it should
be safe to do so as it only registers/configures plugins but has no secrets.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
/usr/bin/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -qmp unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait -pidfile /var/lib/libvirt/qemu/capabilities.pidfile -daemonize
libvirtd needs to be allowed to kill these processes, otherwise they
remain running.
Required to generate correct profiles when using usb passthrough.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/565691
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@ubuntu.com>
Acked-by: Intrigeri <intrigeri@boum.org>
While libvirtd might do so, qemu itself as a guest will not need
to call qemu-nbd so remove it from the profile.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf
If people use non-default paths they should use local overrides but the
suggested defaults we should open up.
This is the default path as referenced by src/qemu/qemu.conf in libvirt.
While doing so merge the several places we have to cover PKI access into
one.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Allows (multi-arch enabled) access to libraries under the
/usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu
qemu-block-extra package and all such libs for the paths
of rpm qemu-block-* packages.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Prevent denial messages related to attempted reads on lttng
files from spamming the logs.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1432644
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
On live migration with --p2p like:
$ virsh migrate --live --p2p kvmguest-bionic-normal \
qemu+ssh://10.6.221.80/system
We hit an apparmor deny like:
apparmor="DENIED" operation="file_inherit"
profile="/usr/sbin/libvirtd" pid=23477 comm="ssh" family="unix"
sock_type="stream" protocol=0 requested_mask="send receive"
denied_mask="send" addr=none peer_addr=none peer="unconfined"
The rule is not perfect, but can't be restricted further at the moment
(new upstream kernel features needed). For now the lack of a profile on the
peer as well as comm not being a conditional on rules do not allow to filter
further.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
virt-aa-helper needs read access to the disk image to resolve symlinks
and add the proper rules to the profile. Its profile whitelists a few
common paths, but users can place their images anywhere.
This commit helps users allowing access to their images by adding their
own rules in apparmor.d/local/usr.lib.libvirt.virt-aa-helper.
This commit also adds rules to allow reading files named:
- *.raw as this is a rather common disk image extension
- /run/libvirt/**[vd]d[a-z] as these are used by virt-sandbox
Noticed the following denial in audit.log when shutting down
an apparmor confined domain
type=AVC msg=audit(1512002299.742:131): apparmor="DENIED"
operation="open" profile="libvirt-66154842-e926-4f92-92f0-1c1bf61dd1ff"
name="/proc/1475/cmdline" pid=2958 comm="qemu-system-x86"
requested_mask="r" denied_mask="r" fsuid=469 ouid=0
Squelch the denial by allowing read access to /proc/<pid>/cmdline.
Since qemu 2.9 via 9103f1ce "file-posix: Consider max_segments for
BlockLimits.max_transfer" this is a new access that is denied by the
qemu profile.
It is non fatal, but prevents the fix mentioned to actually work.
It should be safe to allow reading from that path.
Since qemu opens a symlink path we need to translate that for apparmor from
"/sys/dev/block/*/queue/max_segments" to
"/sys/devices/**/block/*/queue/max_segments"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Right-aligning backslashes when defining macros or using complex
commands in Makefiles looks cute, but as soon as any changes is
required to the code you end up with either distractingly broken
alignment or unnecessarily big diffs where most of the changes
are just pushing all backslashes a few characters to one side.
Generated using
$ git grep -El '[[:blank:]][[:blank:]]\\$' | \
grep -E '*\.([chx]|am|mk)$$' | \
while read f; do \
sed -Ei 's/[[:blank:]]*[[:blank:]]\\$/ \\/g' "$f"; \
done
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
In bf3a4140 "virt-aa-helper: fix libusb access to udev usb data" the
libusb access to properly detect the device/bus ids was fixed.
The path /run/udev/data/+usb* contains a subset of that information we
already allow to be read and are currently not needed for the function
qemu needs libusb for. But on the init of libusb all those files are
still read so a lot of apparmor denials can be seen when using usb host
devices, like:
apparmor="DENIED" operation="open" name="/run/udev/data/+usb:2-1.2:1.0"
comm="qemu-system-x86" requested_mask="r" denied_mask="r"
Today we could silence the warnings with a deny rule without breaking
current use cases. But since the data in there is only a subset of those
it can read already it is no additional information exposure. And on the
other hand a future udev/libusb/qemu combination might need it so allow
the access in the default apparmor profile.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Commit b482925c added ptrace rule for the apparmor profiles,
but one was missed in the libvirtd profile for dnsmasq. It was
overlooked since the test machine did not have an active libvirt
network requiring dnsmasq that was also set to autostart. With
one active and set to autostart, the following denial is observed
in audit.log when restarting libvirtd
type=AVC msg=audit(1507320136.306:298): apparmor="DENIED" \
operation="ptrace" profile="/usr/sbin/libvirtd" pid=5472 \
comm="libvirtd" requested_mask="trace" denied_mask="trace" \
peer="/usr/sbin/dnsmasq"
With an active network, I suspect a libvirtd restart causes access
to /proc/<dnsmasq-pid>/*, hence the resulting denial. As a nasty
side affect of the denial, libvirtd thinks it needs to spawn a
dnsmasq process even though one is already running for the network.
E.g. after two libvirtd restarts
dnsmasq 1683 0.0 0.0 51188 2612 ? S 12:03 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
root 1684 0.0 0.0 51160 576 ? S 12:03 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
dnsmasq 4706 0.0 0.0 51188 2572 ? S 13:54 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
root 4707 0.0 0.0 51160 572 ? S 13:54 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
dnsmasq 4791 0.0 0.0 51188 2580 ? S 13:56 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
root 4792 0.0 0.0 51160 572 ? S 13:56 0:00 \
/usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/default.conf \
--leasefile-ro --dhcp-script=/usr/lib64/libvirt/libvirt_leaseshelper
A simple fix is to add a ptrace rule for dnsmasq.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-By: Guido Günther <agx@sigxcpu.org>
libusb as used by qemu needs to read data from /run/udev/data/ about usb
devices. That is read once on the first initialization of libusb_init by
qemu.
Therefore generating just the device we need would not be sufficient as
another hotplug later can need another device which would fail as the
data is no more re-read at this point.
But we can restrict the paths very much to just the major number of
potential usb devices which will make it match approximately the detail
that e.g. an lsusb -v would reveal - that is much safer than the
"/run/udev/data/* r" blanket many users are using now as a workaround.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Andrea Bolognani