If built without attr support removing any image will trigger
 qemuBlockRemoveImageMetadata (the one that emits the warning)
   -> qemuSecurityMoveImageMetadata
     -> virSecurityManagerMoveImageMetadata
       -> virSecurityDACMoveImageMetadata
         -> virSecurityDACMoveImageMetadataHelper
           -> virProcessRunInFork (spawns subprocess)
             -> virSecurityMoveRememberedLabel

In there due to !HAVE_LIBATTR virFileGetXAttrQuiet will return
ENOSYS and from there the chain will error out.

That is wrong and looks like:
  libvirtd[6320]: internal error: child reported (status=125):
  libvirtd[6320]: Unable to remove disk metadata on vm testguest from
  /var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda)

This change makes virSecurityDACMoveImageMetadataHelper and
virSecuritySELinuxMoveImageMetadataHelper accept that
error code gracefully and in that sense it is an extension of:
5214b2f1 "security: Don't skip label restore on file systems lacking XATTRs"
which does the same for other call chains into the virFile*XAttr functions.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The feature was never completed and is not really being pursued. Remove
the storage driver integration.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When a QEMU process dies in the middle of a hotplug, then we fail
to restore the seclabels on the device. The problem is that if
the thread doing hotplug locks the domain object first and thus
blocks the thread that wants to do qemuProcessStop(), the
seclabel cleanup code will see vm->pid still set and mount
namespace used and therefore try to enter the namespace
represented by the PID. But the PID is gone really and thus
entering will fail and no restore is done. What we can do is to
try enter the namespace (if requested to do so) but if entering
fails, fall back to no NS mode.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814481Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Mores <pmores@redhat.com>

Our decision whether to remember seclabel for a disk image
depends on a few factors. If the image is readonly or shared or
not the chain top the remembering is suppressed for the image.
However, the virSecurityManagerSetImageLabel() is too low level
to determine whether passed @src is chain top or not. Even though
the function has the @parent argument it does not necessarily
reflect the chain top - it only points to the top level image in
the chain we want to relabel and not to the topmost image of the
whole chain. And this can't be derived from the passed domain
definition reliably neither - in some cases (like snapshots or
block copy) the @src is added to the definition only after the
operation succeeded. Therefore, introduce a flag which callers
can use to help us with the decision.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

There are some cases where we want to remember the original owner
of a file but we fail to lock it for XATTR change (e.g. root
squashed NFS). If that is the case we error out and refuse to
start a domain. Well, we can do better if we disable remembering
for paths we haven't locked successfully.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

chown and some stat constants are not available on
the Windows platform.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Files inside /dev/vfio/ can't be opened more than once, meaning
that any subsequent open calls will fail. This behavior was
introduced in kernel v3.11, commit 6d6768c61b39.

When using the VFIO driver, we open a FD to /dev/vfio/N and
pass it to QEMU. If any other call attempt for the same
/dev/vfio/N happens while QEMU is still using the file, we are
unable to open it and QEMU will report -EBUSY. This can happen
if we hotplug a PCI hostdev that belongs to the same IOMMU group
of an existing domain hostdev.

The problem and solution is similar to what we already dealt
with for TPM in commit 4e95cdcb. This patch changes both
DAC and SELinux drivers to disable 'remember' for VFIO hostdevs
in virSecurityDACSetHostdevLabelHelper() and
virSecurityDACSetHostdevLabel(), and 'recall'
in virSecurityDACRestoreHostdevLabel() and
virSecuritySELinuxRestoreHostdevSubsysLabel().
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

There is a case in which we do not want 'remember' to be
set to true in SetOwnership() calls inside the
HostdevLabelHelper() functions of both DAC and SELinux drivers.
Next patch will explain and handle that scenario.

For now, let's make virSecurityDACSetOwnership() and
virSecuritySELinuxSetHostdevLabelHelper() accept a 'remember'
flag, which will be used to set the 'remember' parameter
of their respective SetOwnership() calls. No functional
change is made.
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

The GLib g_lstat() function provides a portable impl for
Win32.
Reviewed-by: NFabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This function is currently not called for any type of storage
source that is not considered 'local' (as defined by
virStorageSourceIsLocalStorage()). Well, NVMe disks are not
'local' from that point of view and therefore we will need to
call this function more frequently.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Replace all occurrences of
  if (VIR_STRDUP(a, b) < 0)
     /* effectively dead code */
with:
  a = g_strdup(b);
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Prefer the GLib version of the macro.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

We mirror the labeling strategy that was used for its sibling
image
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

This will be used for recursing into externalDataStore
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Rename the existing virSecurityDACRestoreImageLabelInt
to virSecurityDACRestoreImageLabelSingle, and extend the new
ImageLabelInt handle externalDataStore
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

This will simplify future patches and make the logic easier to follow
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

The only caller always passes in a non-null parent
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1755803

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

If user has two domains, each have the same disk (configured for
RW) but each runs with different seclabel then we deny start of
the second domain because in order to do that we would need to
relabel the disk but that would cut the first domain off. Even if
we did not do that, qemu would fail to start because it would be
unable to lock the disk image for the second time. So far, this
behaviour is expected. But what is not expected is that we
increase the refcounter in XATTRs and leave it like that.

What happens is that when the second domain starts,
virSecuritySetRememberedLabel() is called, and since there are
XATTRs from the first domain it increments the refcounter and
returns it (refcounter == 2 at this point). Then callers
(virSecurityDACSetOwnership() and
virSecuritySELinuxSetFileconHelper()) realize that refcounter is
greater than 1 and desired seclabel doesn't match the one the
disk image already has and an error is produced. But the
refcounter is never decremented.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1740024Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This effectively reverts d7420430 and adds new code.

Here is the problem: Imagine a file X that is to be shared
between two domains as a disk. Let the first domain (vm1) have
seclabel remembering turned on and the other (vm2) has it turned
off. Assume that both domains will run under the same user, but
the original owner of X is different (i.e. trying to access X
without relabelling leads to EPERM).

Let's start vm1 first. This will cause X to be relabelled and to
gain new attributes:

  trusted.libvirt.security.ref_dac="1"
  trusted.libvirt.security.dac="$originalOwner"

When vm2 is started, X will again be relabelled, but since the
new label is the same as X already has (because of vm1) nothing
changes and vm1 and vm2 can access X just fine. Note that no
XATTR is changed (especially the refcounter keeps its value of 1)
because the vm2 domain has the feature turned off.

Now, vm1 is shut off and vm2 continues running. In seclabel
restore process we would get to X and since its refcounter is 1
we would restore the $originalOwner on it. But this is unsafe to
do because vm2 is still using X (remember the assumption that
$originalOwner and vm2's seclabel are distinct?).

The problem is that refcounter stored in XATTRs doesn't reflect
the actual times a resource is in use. Since I don't see any easy
way around it let's just not store original owner on shared
resources. Shared resource in world of domain disks is:

  - whole backing chain but the top layer,
  - read only disk (we don't require CDROM to be explicitly
    marked as shareable),
  - disk marked as shareable.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

One caller in particular (virSecurityDACSetImageLabelInternal)
will want to have the feature turned on only in some cases.
Introduce @remember member to _virSecurityDACChownItem to track
whether caller wants to do owner remembering or not.
The actual remembering is then enabled if both caller wanted it
and the feature is turned on in the config file.

Technically, we could skip over paths that don't have remember
enabled when creating a list of paths to lock. We won't touch
their XATTRs after all. Well, I rather play it safe and keep them
on the locking list for now.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Both DAC and SELinux drivers support transactions. Each item on
the transaction list consists of various variables and @restore
is one of them. Document it so that as the list of variables grow
it's easier to spot which variable does what.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The way that virSecurityDACRecallLabel is currently written is
that if XATTRs are not supported for given path to the caller
this is not different than if the path is still in use. The value
of 1 is returned which makes secdrivers skip label restore.
This is clearly a bug as we are not restoring labels on say NFS
even though previously we were.

Strictly speaking, changes to virSecurityDACRememberLabel are not
needed, but they are done anyway so that getter and setter behave
in the same fashion.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

We're setting seclabels on unix sockets but never restoring them.
Surprisingly, we are doing so in SELinux driver.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This reverts commit a5e16020.

Getting rid of unistd.h from our headers will require more work than
just fixing the broken mingw build. Revert it until I have a more
complete proposal.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

util/virutil.h bogously included unistd.h. Drop it and replace it by
including it directly where needed.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The default permissions (0600 root:root) are of no use to the qemu
process so we need to change the owner to qemu iff running with
namespaces.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Security labeling of disks consists of labeling of the disk image
itself and it's backing chain. Modify
virSecurityManager[Set|Restore]ImageLabel to take a boolean flag that
will label the full chain rather than the top image itself.

This allows to delete/unify some parts of the code and will also
simplify callers in some cases.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Now that we have seclabel remembering we can safely restore
labels for shared and RO disks. In fact we need to do that to
keep seclabel refcount stored in XATTRs in sync with reality.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This also requires the same DAC label to be used for shared
paths. If a path is already in use by a domain (or domains) then
and the domain we are starting now wants to access the path it
has to have the same DAC label. This might look too restrictive
as the new label can still guarantee access to already running
domains but in reality it is very unlikely and usually an admin
mistake.

This requirement also simplifies seclabel remembering, because we
can store only one seclabel and have a refcounter for how many
times the path is in use. If we were to allow different labels
and store them in some sort of array the algorithm to match
labels to domains would be needlessly complicated.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Because the implementation that will be used for label
remembering/recall is not atomic we have to give callers a chance
to enable or disable it. That is, enable it if and only if
metadata locking is enabled. Otherwise the feature MUST be turned
off.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We are setting label on kernel, initrd, dtb and slic_table files.
But we never restored it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>