Commit Graph

483 Commits

Author SHA1 Message Date
Michal Privoznik
3027bacf95 virSecuritySELinuxSetFileconHelper: Fix build with broken selinux.h
There are still some systems out there that have broken
setfilecon*() prototypes. Instead of taking 'const char *tcon' it
is taking 'char *tcon'. The function should just set the context,
not modify it.

We had been bitten with this problem before which resulted in
292d3f2d and subsequently b109c09765. However, with one my latest
commits (4674fc6afd) I've changed the type of @tcon variable to
'const char *' which results in build failure on the systems from
above.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 19:23:49 +01:00
Michal Privoznik
4674fc6afd security_selinux: Implement transaction APIs
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:50:00 +01:00
Michal Privoznik
67232478db security_dac: Implement transaction APIs
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:50:00 +01:00
Michal Privoznik
95576b4df0 security driver: Introduce transaction APIs
With our new qemu namespace code in place, the relabelling of
devices is done not as good is it could: a child process is
spawned, it enters the mount namespace of the qemu process and
then runs desired API of the security driver.

Problem with this approach is that internal state transition of
the security driver done in the child process is not reflected in
the parent process. While currently it wouldn't matter that much,
it is fairly easy to forget about that. We should take the extra
step now while this limitation is still fresh in our minds.

Three new APIs are introduced here:
  virSecurityManagerTransactionStart()
  virSecurityManagerTransactionCommit()
  virSecurityManagerTransactionAbort()

The Start() is going to be used to let security driver know that
we are starting a new transaction. During a transaction no
security labels are actually touched, but rather recorded and
only at Commit() phase they are actually updated. Should
something go wrong Abort() aborts the transaction freeing up all
memory allocated by transaction.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:49:59 +01:00
Michal Privoznik
39779eb195 security_dac: Resolve virSecurityDACSetOwnershipInternal const correctness
The code at the very bottom of the DAC secdriver that calls
chown() should be fine with read-only data. If something needs to
be prepared it should have been done beforehand.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:49:59 +01:00
Michal Privoznik
65fb0b79f7 security_selinux: s/virSecuritySELinuxSecurity/virSecuritySELinux/
It doesn't make much sense to have two different prefix for
functions within the same driver.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-09 09:17:42 +01:00
Michal Privoznik
ce937d3710 security: Drop virSecurityManagerSetHugepages
Since its introduction in 2012 this internal API did nothing.
Moreover we have the same API that does exactly the same:
virSecurityManagerDomainSetPathLabel.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2016-12-08 15:45:52 +01:00
Christian Ehrhardt
dffdac06c0 virt-aa-helper: fix parsing security labels by introducing VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL
When virt-aa-helper parses xml content it can fail on security labels.

It fails by requiring to parse active domain content on seclabels that
are not yet filled in.

Testcase with virt-aa-helper on a minimal xml:
 $ cat << EOF > /tmp/test.xml
<domain type='kvm'>
    <name>test-seclabel</name>
    <uuid>12345678-9abc-def1-2345-6789abcdef00</uuid>
    <memory unit='KiB'>1</memory>
    <os><type arch='x86_64'>hvm</type></os>
    <seclabel type='dynamic' model='apparmor' relabel='yes'/>
    <seclabel type='dynamic' model='dac' relabel='yes'/>
</domain>
EOF
 $ /usr/lib/libvirt/virt-aa-helper -d -r -p 0 \
   -u libvirt-12345678-9abc-def1-2345-6789abcdef00 < /tmp/test.xml

Current Result:
 virt-aa-helper: error: could not parse XML
 virt-aa-helper: error: could not get VM definition
Expected Result is a valid apparmor profile

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Guido Günther <agx@sigxcpu.org>
2016-11-30 08:15:57 +01:00
Eric Farman
81a206f52b security: Include vhost-scsi in security labels
Ensure that the vhost-scsi wwpn information is passed to the
different security policies.

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
2016-11-24 12:16:26 -05:00
Eric Farman
fc0e627bac Introduce framework for a hostdev SCSI_host subsystem type
We already have a "scsi" hostdev subsys type, which refers to a single
LUN that is passed through to a guest.  But what of things where
multiple LUNs are passed through via a single SCSI HBA, such as with
the vhost-scsi target?  Create a new hostdev subsys type that will
carry this.

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
2016-11-24 12:15:26 -05:00
Eric Farman
85b0721095 Cleanup switch statements on the hostdev subsystem type
As was suggested in an earlier review comment[1], we can
catch some additional code points by cleaning up how we use the
hostdev subsystem type in some switch statements.

[1] End of https://www.redhat.com/archives/libvir-list/2016-September/msg00399.html

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
2016-11-11 16:58:56 -05:00
John Ferlan
8f67b9ecd2 conf: Use virDomainChrSourceDefPtr for _virDomainSmartcardDef 'passthru'
Use a pointer and the virDomainChrSourceDefNew() function in order to
allocate the structure for _virDomainSmartcardDef.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2016-10-24 06:44:23 -04:00
John Ferlan
77a12987a4 Introduce virDomainChrSourceDefNew for virDomainChrDefPtr
Change the virDomainChrDef to use a pointer to 'source' and allocate
that pointer during virDomainChrDefNew.

This has tremendous "fallout" in the rest of the code which mainly
has to change source.$field to source->$field.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2016-10-21 14:03:36 -04:00
Michal Privoznik
c41b989112 virDomainDefParse{File,String}: Introduce @parseOpaque argument
We want to pass the proper opaque pointer instead of NULL to
virDomainDefParse and subsequently virDomainDefParseNode too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2016-09-26 16:50:12 +02:00
Julio Faracco
fe94ee5db5 security: Fixing wrong label in virt-aa-helper.c.
There is an issue with a wrong label inside vah_add_path().
The compilation fails with the error:
make[3]: Entering directory '/tmp/libvirt/src'
  CC       security/virt_aa_helper-virt-aa-helper.o
security/virt-aa-helper.c: In function 'vah_add_path':
security/virt-aa-helper.c:769:9: error: label 'clean' used but not defined
         goto clean;

This patch moves 'clean' label to 'cleanup' label.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
2016-09-07 18:06:31 -04:00
Rufo Dogav
b3c1bcb9fd Avoid segfault in virt-aa-helper when handling read-only filesystems
This patch fixes a segfault in virt-aa-helper caused by attempting to
modify a static string literal. It is triggered when a domain has a
<filesystem> with type='mount' configured read-only and libvirt is
using the AppArmor security driver for sVirt confinement. An "R" is
passed into the function and converted to 'r'.
2016-09-07 16:29:02 -04:00
Guido Günther
0d55e0b24c virt-aa-helper: Make help output match option name 2016-07-22 07:55:59 -04:00
Cédric Bosdonnat
cedd2ab282 virt-aa-helper: better write denials handling
Better fix replacing c726af2d: introducing an 'R' permission to
add read rule, but no explicit deny write rule.
2016-07-19 16:21:36 +02:00
Julio Faracco
fc624d86ea security: compilation error due to wrong parameter for vah_add_path().
The commit da665fbd introduced virStorageSourcePtr inside the structure
_virDomainFSDef. This is causing an error when libvirt is being compiled.

make[3]: Entering directory `/media/julio/8d65c59c-6ade-4740-9cdc-38016a4cb8ae
/home/julio/Desktop/virt/libvirt/src'
  CC       security/virt_aa_helper-virt-aa-helper.o
security/virt-aa-helper.c: In function 'get_files':
security/virt-aa-helper.c:1087:13: error: passing argument 2 of 'vah_add_path'
from incompatible pointer type [-Werror]
             if (vah_add_path(&buf, fs->src, "rw", true) != 0)
             ^
security/virt-aa-helper.c:732:1: note: expected 'const char *' but argument is
of type 'virStorageSourcePtr'
 vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool
recursive)
 ^
cc1: all warnings being treated as errors

Adding the attribute "path" from virStorageSourcePtr fixes this issue.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
2016-07-19 09:02:51 +02:00
Daniel P. Berrange
b9c8af4d71 selinux: convert to typesafe virConf accessors
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-07-12 09:58:22 +01:00
Pavel Hrdina
acc83afe33 vnc: add support for listen type 'socket'
VNC graphics already supports sockets but only via 'socket' attribute.
This patch coverts that attribute into listen type 'socket'.

For backward compatibility we need to handle listen type 'socket' and 'socket'
attribute properly to support old XMLs and new XMLs.  If both are provided they
have to match, if only one of them is provided we need to be able to parse that
configuration too.

To not break migration back to old libvirt if the socket is provided by user we
need to generate migratable XML without the listen element and use only 'socket'
attribute.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2016-06-09 14:42:48 +02:00
Pavel Hrdina
b6465e1aa4 graphics: introduce new listen type 'socket'
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2016-06-09 14:24:00 +02:00
Peter Krempa
b394af162a conf: Add infrastructure for adding configuration validation
Until now we weren't able to add checks that would reject configuration
once accepted by the parser. This patch adds a new callback and
infrastructure to add such checks. In this patch all the places where
rejecting a now-invalid configuration wouldn't be a good idea are marked
with a new parser flag.
2016-06-07 13:02:20 +02:00
Ján Tomko
5da23bbedf security: label the slic_table
Add support for the slic_table to the security drivers.
2016-05-25 17:15:21 +02:00
Pavel Hrdina
3902f634bf virt-aa-helper: remove replace_string and use virStringReplace instead
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2016-05-13 17:42:57 +02:00
Simon Arlott
ab9569e546 virt-aa-helper: disallow VNC socket read permissions
The VM does not need read permission for its own VNC socket to create(),
bind(), accept() connections or to receive(), send(), etc. on connections.

https://bugzilla.redhat.com/show_bug.cgi?id=1312573
2016-04-20 09:58:47 -04:00
Martin Kletzander
32f3f0835e security: Rename DomainSetDirLabel to DomainSetPathLabel
It already labels abritrary paths, so it's just the naming that was
wrong.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2016-04-18 20:34:30 +02:00
Cole Robinson
e7db227810 util: Add virGettextInitialize, convert the code
Take setlocale/gettext error handling pattern from tools/virsh-*
and use it for all standalone binaries via a new shared
virGettextInitialize routine. The virsh* pattern differed slightly
from other callers. All users now consistently:

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror
2016-04-14 13:22:40 -04:00
Pavel Hrdina
d713a6b120 build: add GCC 6.0 -Wlogical-op workaround
fdstream.c: In function 'virFDStreamWrite':
fdstream.c:390:29: error: logical 'or' of equal expressions [-Werror=logical-op]
        if (errno == EAGAIN || errno == EWOULDBLOCK) {
                            ^~

Fedora rawhide now uses gcc 6.0 and there is a bug with -Wlogical-op
producing false warnings.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602

Use GCC pragma push/pop and ignore -Wlogical-op for GCC that supports
push/pop pragma and also has this bug.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2016-04-11 12:09:14 +02:00
Guido Günther
dfbc9a8382 apparmor: QEMU monitor socket moved
The directory name changed in a89f05ba8d.

This unbreaks launching QEMU/KVM VMs with apparmor enabled. It also adds
the directory for the qemu guest-agent socket which is not known when
parsing the domain XML.
2016-04-02 12:49:28 +02:00
Laurent Bigonville
0b6e5ddd89 security_selinux: Fix typo in error message 2016-02-19 17:15:31 +00:00
Peter Krempa
41c987b72d Fix build after recent patches
Few build breaking mistakes in less-popular parts of our code.
2016-02-04 16:34:28 +01:00
Martin Kletzander
1794a0103a qemu: Don't crash when create fails early
Since commit 7140807917 we are generating
socket path later than before -- when starting a domain.  That makes one
particular inconsistent state of a chardev, which was not possible
before, currently valid.  However, SELinux security driver forgot to
guard the main restoring function by a check for NULL-paths.  So make it
no-op for NULL paths, as in the DAC driver.

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2016-02-03 11:01:42 +01:00
Jiri Denemark
8f0a15727f security: Do not restore labels on device tree binary
A device tree binary file specified by /domain/os/dtb element is a
read-only resource similar to kernel and initrd files. We shouldn't
restore its label when destroying a domain to avoid breaking other
domains configure with the same device tree.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2016-01-15 16:34:37 +01:00
Jiri Denemark
68acc701bd security: Do not restore kernel and initrd labels
Kernel/initrd files are essentially read-only shareable images and thus
should be handled in the same way. We already use the appropriate label
for kernel/initrd files when starting a domain, but when a domain gets
destroyed we would remove the labels which would make other running
domains using the same files very unhappy.

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2016-01-15 10:55:58 +01:00
Cédric Bosdonnat
c726af2d5a virt-aa-helper: don't deny writes to readonly mounts
There is no need to deny writes on a readonly mount: write still
won't be accepted, even if the user remounts the folder as RW in
the guest as qemu sets the 9p mount as ro.

This deny rule was leading to problems for example with readonly /:
The qemu process had to write to a bunch of files in / like logs,
sockets, etc. This deny rule was also preventing auditing of these
denials, making it harder to debug.
2016-01-14 15:42:05 +01:00
Ján Tomko
077bdba5c2 security_stack: remove extra Security from function names
Many of the functions follow the pattern:
virSecurity.*Security.*Label

Remove the second 'Security' from the names, it should be
obvious that the virSecurity* functions deal with security
labels even without it.
2015-12-15 16:06:08 +01:00
Ján Tomko
ba9285b3a3 security_selinux: remove extra Security from function names
Many of the functions follow the pattern:
virSecurity.*Security.*Label

Remove the second 'Security' from the names, it should be obvious
that the virSecurity* functions deal with security labels even
without it.
2015-12-15 16:06:08 +01:00
Ján Tomko
be33e96533 security_dac: remove extra Security from function names
Many of the functions follow the pattern:
virSecurity.*Security.*Label

Remove the second 'Security' from the names, it should be obvious
that the virSecurity* functions deal with security labels even
without it.
2015-12-15 16:06:08 +01:00
Ján Tomko
bfc29df3e0 security_selinux: fix indentation 2015-12-09 10:44:26 +01:00
Ján Tomko
63cc969a84 security_dac: check if virSecurityDACGetIds returns negative
Use the customary check '< 0' instead of checking for non-zero.

No functional change.
2015-12-09 10:44:26 +01:00
Ján Tomko
d5aba1a4d9 security: label the evdev for input device passthrough
Add functions for setting and restoring the label of input devices
to DAC and SELinux drivers.

https://bugzilla.redhat.com/show_bug.cgi?id=1231114
2015-11-30 12:57:43 +01:00
Jiri Denemark
c6172260a5 security: Cleanup DAC driver
Fixes several style issues and removes "DEF" (what is it supposed to
mean anyway?) from debug messages.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2015-11-11 17:02:53 +01:00
Ishmanpreet Kaur Khera
32cee5b2f0 Avoid using !STREQ and !STRNEQ
We have macros for both positive and negative string matching.
Therefore there is no need to use !STREQ or !STRNEQ. At the same
time as we are dropping this, new syntax-check rule is
introduced to make sure we won't introduce it again.

Signed-off-by: Ishmanpreet Kaur Khera <khera.ishman@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-21 15:03:35 +02:00
Michal Privoznik
6222a6fee3 security_dac: Introduce remember/recall APIs
Even though the APIs are not implemented yet, they create a
skeleton that can be filled in later.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
ec04c18bc5 security_dac: Limit usage of virSecurityDACSetOwnershipInternal
This function should really be called only when we want to change
ownership of a file (or disk source). Lets switch to calling a
wrapper function which will eventually record the current owner
of the file and call virSecurityDACSetOwnershipInternal
subsequently.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
fdf44d5b47 virSecurityDACRestoreSecurityFileLabel: Pass virSecurityDACDataPtr
This is pure code adjustment. The structure is going to be needed
later as it will hold a reference that will be used to talk to
virtlockd. However, so far this is no functional change just code
preparation.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
607f34319d virSecurityDACSetOwnership: Pass virSecurityDACDataPtr
This is pure code adjustment. The structure is going to be needed
later as it will hold a reference that will be used to talk to
virtlockd. However, so far this is no functional change just code
preparation.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
a0f43d820d virSecurityDACSetOwnershipInternal: Don't chown so often
It's better if we stat() file that we are about to chown() at
first and check if there's something we need to change. Not that
it would make much difference, but for the upcoming patches we
need to be doing stat() anyway. Moreover, if we do things this
way, we can drop @chown_errno variable which will become
redundant.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
d37d8f78c0 security_dac: Fix TODO marks
Correctly mark the places where we need to remember and recall
file ownership. We don't want to mislead any potential developer.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00