Michal Privoznik [Wed, 11 Sep 2019 09:19:06 +0000 (11:19 +0200)]
security_stack: Perform rollback if one of stacked drivers fails
In order to have multiple security drivers hidden under one
virSecurity* call, we have virSecurityStack driver which holds a
list of registered security drivers and for every virSecurity*
call it iterates over the list and calls corresponding callback
in real security drivers. For instance, for
virSecurityManagerSetAllLabel() it calls
domainSetSecurityAllLabel callback sequentially in NOP, DAC and
(possibly) SELinux or AppArmor drivers. This works just fine if
the callback from every driver returns success. Problem arises
when one of the drivers fails. For instance, aforementioned
SetAllLabel() succeeds for DAC but fails in SELinux in which
case all files that DAC relabelled are now owned by qemu:qemu (or
whomever runs qemu) and thus permissions are leaked. This is even
more visible with XATTRs which remain set for DAC.
The solution is to perform a rollback on failure, i.e. call
opposite action on drivers that succeeded.
I'm providing rollback only for set calls and intentionally
omitting restore calls for two reasons:
1) restore calls are less likely to fail (they merely remove
XATTRs and chown()/setfilecon() file - all of these operations
succeeded in set call),
2) we are not really interested in restore failures - in a very
few places we check for retval of a restore function we do so
only to print a warning.
Michal Privoznik [Wed, 11 Sep 2019 08:33:19 +0000 (10:33 +0200)]
security_stack: Turn list of nested drivers into a doubly linked list
In near future we will need to walk through the list of internal
drivers in reversed order. The simplest solution is to turn
singly linked list into a doubly linked list.
We will not need to start from the end really, so there's no tail
pointer kept.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
Michal Privoznik [Wed, 11 Sep 2019 05:53:09 +0000 (07:53 +0200)]
security: Pass @migrated to virSecurityManagerSetAllLabel
In upcoming commits, virSecurityManagerSetAllLabel() will perform
rollback in case of failure by calling
virSecurityManagerRestoreAllLabel(). But in order to do that, the
former needs to have @migrated argument so that it can be passed
to the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
build: remove use of usleep gnulib module in favour of g_usleep
The usleep function was missing on older mingw versions, but we can rely
on it existing everywhere these days. It may only support times upto 1
second in duration though, so we'll prefer to use g_usleep instead.
The commandhelper program is not changed since that can't link to glib.
Fortunately it doesn't need to build on Windows platforms either.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
which declares the instance & class constructor functions
- Add an impl of the instance & class constructors
wiring up the finalize method to point to our dispose impl
In all files
- Replace VIR_AUTOUNREF(virIdentityPtr) with g_autoptr(virIdentity)
- Replace virObjectRef/Unref with g_object_ref/unref. Note
the latter functions do *NOT* accept a NULL object where as
libvirt's do. If you replace g_object_unref with g_clear_object
it is NULL safe, but also clears the pointer.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
src: add support for g_autoptr with virObject instances
Libvirt currently uses the VIR_AUTOUNREF macro for auto cleanup of
virObject instances. GLib approaches things differently with GObject,
reusing their g_autoptr() concept.
This introduces support for g_autoptr() with virObject, to facilitate
the conversion to GObject.
Only virObject classes which are currently used with VIR_AUTOREF are
updated. Any others should be converted to GObject before introducing
use of autocleanup.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: rewrite auto cleanup macros to use glib's equivalent
To facilitate porting over to glib, this rewrites the auto cleanup
macros to use glib's equivalent.
As a result it is now possible to use g_autoptr/VIR_AUTOPTR, and
g_auto/VIR_AUTOCLEAN, g_autofree/VIR_AUTOFREE interchangably, regardless
of which macros were used to declare the cleanup types.
Within the scope of any single method, code must remain consistent
using either GLib or Libvirt macros, never mixing both. New code
must preferentially use the GLib macros, and old code will be
converted incrementally.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: use glib string allocation/formatting functions
Convert the string duplication APIs to use the g_strdup family of APIs.
We previously used the 'strdup-posix' gnulib module because mingw does
not set errno to ENOMEM on failure
We previously used the 'strndup' gnulib module because this function
does not exist on mingw.
We previously used the 'vasprintf' gnulib module because of many GNU
supported format specifiers not working on non-Linux platforms. glib's
own equivalent standardizes on GNU format specifiers too.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Convert the VIR_ALLOC family of APIs with use of the g_malloc family of
APIs. Use of VIR_ALLOC related functions should be incrementally phased
out over time, allowing return value checks to be dropped. Use of
VIR_FREE should be replaced with auto-cleanup whenever possible.
We previously used the 'calloc-posix' gnulib module because mingw does
not set errno to ENOMEM on failure.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Applications are still encourged to match g_malloc/g_free, but it is no
longer a mandatory requirement for correctness, just stylistic. This is
explicitly clarified in
gmem: clarify that g_malloc always uses the system allocator
Applications can still use custom allocators in general, but they must
do this by linking to a library that replaces the core malloc/free
implemenentation entirely, instead of via a glib specific call.
This means that libvirt does not need to be concerned about use of
g_malloc/g_free causing an ABI change in the public libary, and can
avoid memory copying when talking to external libraries.
This patch probes for glib, which provides the foundation layer with
a collection of data structures, helper APIs, and platform portability
logic.
Later patches will introduce linkage to gobject which provides the
object type system, built on glib, and gio which providing objects
for various interesting tasks, most notably including DBus client
and server support and portable sockets APIs, but much more too.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Cole Robinson [Sat, 5 Oct 2019 23:45:21 +0000 (19:45 -0400)]
security: selinux: Restore image label for externalDataStore
Rename the existing virSecuritySELinuxRestoreImageLabelInt
to virSecuritySELinuxRestoreImageLabelSingle, and extend the new
ImageLabelInt handle externalDataStore
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Cole Robinson [Sat, 5 Oct 2019 23:47:43 +0000 (19:47 -0400)]
security: dac: Restore image label for externalDataStore
Rename the existing virSecurityDACRestoreImageLabelInt
to virSecurityDACRestoreImageLabelSingle, and extend the new
ImageLabelInt handle externalDataStore
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Cole Robinson [Fri, 4 Oct 2019 23:41:36 +0000 (19:41 -0400)]
storagefile: Fix backing format \0 check
From qemu.git docs/interop/qcow2.txt
== String header extensions ==
Some header extensions (such as the backing file format name and
the external data file name) are just a single string. In this case,
the header extension length is the string length and the string is
not '\0' terminated. (The header extension padding can make it look
like a string is '\0' terminated, but neither is padding always
necessary nor is there a guarantee that zero bytes are used
for padding.)
So we shouldn't be checking for a \0 byte at the end of the backing
format section. I think in practice there always is a \0 but we
shouldn't depend on that.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Cole Robinson [Fri, 4 Oct 2019 22:00:55 +0000 (18:00 -0400)]
storagefile: qcow1: Fix check for empty backing file
From f772b3d91fd the intention of this code seems to be to set
format=NONE when the image does not have a backing file. However
'buf' here is the whole qcow1 file header. What we want to be
checking is 'res' which is the parsed backing file path.
qcowXGetBackingStore sets this to NULL when there's no backing file.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
The /dev/tpmN file can be opened only once, as implemented in
drivers/char/tpm/tpm-dev.c:tpm_open() from the kernel's tree. Any
other attempt to open the file fails. And since we're opening the
file ourselves and passing the FD to qemu we will not succeed
opening the file again when locking it for seclabel remembering.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
security_dac: Allow selective remember/recall for chardevs
While in most cases we want to remember/recall label for a
chardev, there are some special ones (like /dev/tpm0) where we
don't want to remember the seclabel nor recall it. See next
commit for rationale behind.
While the easiest way to implement this would be to just add new
argument to virSecurityDACSetChardevLabel() this one is also a
callback for virSecurityManagerSetChardevLabel() and thus has
more or less stable set of arguments. Therefore, the current
virSecurityDACSetChardevLabel() is renamed to
virSecurityDACSetChardevLabelHelper() and the original function
is set to call the new one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
security: Try to lock only paths with remember == true
So far all items on the chown/setfilecon list have the same
.remember value. But this will change shortly. Therefore, don't
try to lock paths which we won't manipulate XATTRs for.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
Cole Robinson [Wed, 9 Oct 2019 18:21:24 +0000 (14:21 -0400)]
security: apparmor: Allow RO /usr/share/edk2/
On Fedora, already whitelisted paths to AAVMF and OVMF binaries
are symlinks to binaries under /usr/share/edk2/. Add that directory
to the RO whitelist so virt-aa-helper-test passes
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Ivan Kardykov [Sun, 15 Sep 2019 19:43:22 +0000 (21:43 +0200)]
libxl: add acpi slic table support
Libxl driver did not support setup additional acpi firmware to xen
guest. It is necessary to activate OEM Windows installs. This patch
allow to define in OS section acpi table param (which supported domain
common schema).
Signed-off-by: Ivan Kardykov <kardykov@tabit.pro>
[added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
src/driver.c: remove duplicated code in virGetConnect* functions
All the 6 virGetConnect* functions in driver.c shares the
same code base. This patch creates a new static function
virGetConnectGeneric() that contains the common code to
be used with all other virGetConnect*.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Thu, 10 Oct 2019 07:44:12 +0000 (09:44 +0200)]
qemu_process: Initialize domain definition for QMP query
When constructing QMP capabilities we allocate a dummy domain
object to pass to qemuMonitorOpen(). However, after 75dd595861
the function also expects domain definition to be allocated for
the domain object. The referenced commit already fixed
qemumonitortestutils.c but forgot to fix the other caller:
qemuProcessQMPConnectMonitor().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds the implementation of the ccf-assist pSeries
feature, based on the QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST
capability that was added in the previous patch.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
projects / libvirt.git / log