Commit Graph

459 Commits

Author SHA1 Message Date
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
Michal Privoznik
79bd55b302 virSecurityManagerNew: Turn array of booleans into flags
So imagine you want to crate new security manager:

  if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true)));

Hard to parse, right? What about this:

  if (!(mgr = virSecurityManagerNew("selinux", "QEMU",
                                    VIR_SECURITY_MANAGER_DEFAULT_CONFINED |
                                    VIR_SECURITY_MANAGER_PRIVILEGED)));

Now that's better! This is what the commit does.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-07 17:51:28 +02:00
Cédric Bosdonnat
a1bdf04b27 apparmor: differentiate between error and unconfined profiles
profile_status function was not making any difference between error
cases and unconfined profiles. The problem with this approach is that
dominfo was throwing an error on unconfined domains.
2015-10-06 13:47:01 +02:00
Michal Privoznik
00e5b96716 security_selinux: Take @privileged into account
https://bugzilla.redhat.com/show_bug.cgi?id=1124841

If running in session mode it may happen that we fail to set
correct SELinux label, but the image may still be readable to
the qemu process. Take this into account.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-09-15 11:30:14 +02:00
Michal Privoznik
307fb9044c virSecurityManager: Track if running as privileged
We may want to do some decisions in drivers based on fact if we
are running as privileged user or not. Propagate this info there.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-09-15 11:30:14 +02:00
Michal Privoznik
276c409163 security_selinux: Replace SELinuxSCSICallbackData with proper struct
We have plenty of callbacks in the driver. Some of these
callbacks require more than one argument to be passed. For that
we currently have a data type (struct) per each callback. Well,
so far for only one - SELinuxSCSICallbackData. But lets turn it
into more general name so it can be reused in other callbacks too
instead of each one introducing a new, duplicate data type.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-09-15 11:30:13 +02:00
Michal Privoznik
370461d1db virSecuritySELinuxSetSecurityAllLabel: drop useless virFileIsSharedFSType
The check is done in virSecuritySELinuxSetFilecon itself. There's
no need to check it again.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-09-15 11:30:13 +02:00
Guido Günther
ee7d5c312b apparmor: Adjust path to domain monitor socket
f1f68ca33 moved the monitor socket to a per domain directory. Adjust the
path accordingly.
2015-08-29 18:15:49 +02:00
Michal Privoznik
52970dec5b virt-aa-helper: Improve valid_path
So, after some movement in virt-aa-helper, I've noticed the
virt-aa-helper-test failing. I've ran gdb (it took me a while to
realize how to do that) and this showed up immediately:

  Program received signal SIGSEGV, Segmentation fault.
  strlen () at ../sysdeps/x86_64/strlen.S:106
  106     ../sysdeps/x86_64/strlen.S: No such file or directory.
  (gdb) bt
  #0  strlen () at ../sysdeps/x86_64/strlen.S:106
  #1  0x0000555555561a13 in array_starts_with (str=0x5555557ce910 "/tmp/tmp.6nI2Fkv0KL/1.img", arr=0x7fffffffd160, size=-1540438016) at security/virt-aa-helper.c:525
  #2  0x0000555555561d49 in valid_path (path=0x5555557ce910 "/tmp/tmp.6nI2Fkv0KL/1.img", readonly=false) at security/virt-aa-helper.c:617
  #3  0x0000555555562506 in vah_add_path (buf=0x7fffffffd3e0, path=0x5555557cb910 "/tmp/tmp.6nI2Fkv0KL/1.img", perms=0x555555581585 "rw", recursive=false) at security/virt-aa-helper.c:823
  #4  0x0000555555562693 in vah_add_file (buf=0x7fffffffd3e0, path=0x5555557cb910 "/tmp/tmp.6nI2Fkv0KL/1.img", perms=0x555555581585 "rw") at security/virt-aa-helper.c:854
  #5  0x0000555555562918 in add_file_path (disk=0x5555557d4440, path=0x5555557cb910 "/tmp/tmp.6nI2Fkv0KL/1.img", depth=0, opaque=0x7fffffffd3e0) at security/virt-aa-helper.c:931
  #6  0x00007ffff78f18b1 in virDomainDiskDefForeachPath (disk=0x5555557d4440, ignoreOpenFailure=true, iter=0x5555555628a6 <add_file_path>, opaque=0x7fffffffd3e0) at conf/domain_conf.c:23286
  #7  0x0000555555562b5f in get_files (ctl=0x7fffffffd670) at security/virt-aa-helper.c:982
  #8  0x0000555555564100 in vahParseArgv (ctl=0x7fffffffd670, argc=5, argv=0x7fffffffd7e8) at security/virt-aa-helper.c:1277
  #9  0x00005555555643d6 in main (argc=5, argv=0x7fffffffd7e8) at security/virt-aa-helper.c:1332

So I've taken look at valid_path() because it is obviously
calling array_starts_with() with malformed @size. And here's the
result: there are two variables to hold the size of three arrays
and their value is recalculated before each call of
array_starts_with(). What if we just use three variables,
initialize them and do not touch them afterwards?

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-08-27 15:05:25 +02:00
Peter Kieser
91fdcefa7f virt-aa-helper: add NVRAM store file for read/write
This is a cryptographically signed message in MIME format.

Some UEFI firmwares may want to use a non-volatile memory to store some
variables.
If AppArmor is enabled, and NVRAM store file is set currently
virt-aa-helper does
not add the NVRAM store file to the template. Add this file for
read/write when
this functionality is defined in domain XML.

Signed-off-by: Peter Kieser <peter@kieser.ca>
2015-08-26 16:25:44 +02:00
Guido Günther
4d4c90dfd5 selinux: fix compile errors
Remove unused variable, tag unused parameter and adjust return type.

introduced by 3f48345f7e

CC     security/libvirt_security_manager_la-security_selinux.lo
security/security_selinux.c: In function 'virSecuritySELinuxDomainSetDirLabel':
security/security_selinux.c:2520:5: error: return makes pointer from integer without a cast [-Werror]
security/security_selinux.c:2514:9: error: unused variable 'ret' [-Werror=unused-variable]
security/security_selinux.c:2509:59: error: unused parameter 'mgr' [-Werror=unused-parameter]
2015-08-24 14:15:12 +02:00
intrigeri
2f01cfdf05 virt-aa-helper: allow access to /usr/share/ovmf/
We forbid access to /usr/share/, but (at least on Debian-based systems)
the Open Virtual Machine Firmware files needed for booting UEFI virtual
machines in QEMU live in /usr/share/ovmf/. Therefore, we need to add
that directory to the list of read only paths.

A similar patch was suggested by Jamie Strandboge <jamie@canonical.com>
on https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1483071.
2015-08-24 13:00:39 +02:00
Guido Günther
d25a5e087a virt-aa-helper: Simplify restriction logic
First check overrides, then read only files then restricted access
itself.

This allows us to mark files for read only access whose parents were
already restricted for read write.

Based on a proposal by Martin Kletzander
2015-08-24 13:00:39 +02:00
Guido Günther
26c5fa3a9b virt-aa-helper: document --probing and --dry-run 2015-08-24 13:00:39 +02:00
Martin Kletzander
f4c60dfbf2 security_dac: Add SetDirLabel support
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
3f48345f7e security_selinux: Add SetDirLabel support
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
99cf04e32d security_stack: Add SetDirLabel support
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
f65a2a12f4 security: Add virSecurityDomainSetDirLabel
That function can be used for setting security labels on arbitrary
directories.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
7b6953bc22 security_dac: Label non-listening sockets
SELinux security driver already does that, but DAC driver somehow missed
the memo.  Let's fix it so it works the same way.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
4ac6ce38d3 security_selinux: Use proper structure to access socket data
In virSecuritySELinuxSetSecurityChardevLabel() we are labelling unix
socket path, but accessing another structure of the union.  This does
not pose a problem currently as both paths are at the same offset, but
this should be fixed for the future.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Cédric Bosdonnat
24f3c2f7e0 virt-aa-helper: add DomainGuest to mockup caps
With commit 3f9868a virt-aa-helper stopped working due to missing
DomainGuest in the caps.

The test with -c without arch also needs to be
removed since the new capabilities code uses the host arch when none is
provided.
2015-07-10 11:30:36 +02:00
Cédric Bosdonnat
61dab0f74e virt-aa-helper: rename ctl->hvm to ctl->os
ctl->hvm contains os.type string value, change the name to reflect it.
2015-07-10 11:30:36 +02:00
Cédric Bosdonnat
a55a5e7cfe Get more libvirt errors from virt-aa-helper
Initializing libvirt log in virt-aa-helper and getting it to output
libvirt log to stderr. This will help debugging problems happening in
libvirt functions called from within virt-aa-helper
2015-07-10 11:30:36 +02:00
Cédric Bosdonnat
e44bcae9f0 virt-aa-helper: fix rules for paths with trailing slash
Rules generated for a path like '/' were having '//' which isn't
correct for apparmor. Make virt-aa-helper smarter to avoid these.
2015-07-10 11:30:36 +02:00
Serge Hallyn
56ba2f99a5 virt-aa-helper: add unix channels for nserials as well
Commit 03d7462d added it for channels, but it is also needed for serials.  Add
it for serials, parallels, and consoles as well.

This solves https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1015154

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
2015-07-08 13:49:58 +02:00