11:22 a.m.
Enabling a security driver in qemu.conf is currently all or nothing.
The option to disable security on a per VM basis can be a useful debugging
tool or work around for frustrated users.
Patches 1-3 and 5-6 are prep and cleanup work. Patch 4 fixes an
easily triggerable segfault when defining a domain in qemu. Patch 7
is the actual feature.
Cole Robinson (7):
tests: Add qemuxml2xml tests for <seclabel> handling
security: Use virDomainSeclabelDefClear
security: Add virSecurityIsSpecifiedDriver
qemu: Fix segfault if defining a domain without <seclabel>
domain: Handle seclabel model with an enum
domain: Always validate seclabel model
security: Allow disabling security on a per VM basis
cfg.mk | 1 +
docs/schemas/domain.rng | 13 ++-
src/conf/domain_conf.c | 69 ++++++++++-----
src/conf/domain_conf.h | 15 +++-
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c | 4 +-
src/security/security_apparmor.c | 31 ++-----
src/security/security_driver.c | 25 ++++++
src/security/security_driver.h | 3 +
src/security/security_manager.c | 90 +++++++++++++-------
src/security/security_selinux.c | 50 +++---------
tests/domainschematest | 2 +-
.../qemuxml2xml-balloon-device-auto-out.xml | 25 ++++++
.../qemuxml2xml-channel-virtio-auto-out.xml | 54 ++++++++++++
.../qemuxml2xml-console-compat-auto-out.xml | 31 +++++++
.../qemuxml2xml-console-virtio-out.xml | 29 ++++++
.../qemuxml2xml-disk-scsi-device-auto-out.xml | 31 +++++++
.../qemuxml2xml-seclabel-dynamic-in.xml | 24 +++++
.../qemuxml2xml-seclabel-dynamic-out.xml | 21 +++++
.../qemuxml2xml-seclabel-model-none-in.xml | 21 +++++
.../qemuxml2xml-seclabel-model-none-out.xml | 21 +++++
.../qemuxml2xml-seclabel-static-in.xml | 24 +++++
.../qemuxml2xml-seclabel-static-out.xml | 23 +++++
.../qemuxml2xmlout-balloon-device-auto.xml | 25 ------
.../qemuxml2xmlout-channel-virtio-auto.xml | 54 ------------
.../qemuxml2xmlout-console-compat-auto.xml | 31 -------
.../qemuxml2xmlout-console-virtio.xml | 29 ------
.../qemuxml2xmlout-disk-scsi-device-auto.xml | 31 -------
tests/qemuxml2xmltest.c | 26 ++++--
29 files changed, 501 insertions(+), 304 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
--
1.7.3.2
11:22 a.m.
New subject: [libvirt] [PATCH 1/7] tests: Add qemuxml2xml tests for <seclabel> handling
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
docs/schemas/domain.rng | 13 ++++-
tests/domainschematest | 2 +-
.../qemuxml2xml-balloon-device-auto-out.xml | 25 +++++++++
.../qemuxml2xml-channel-virtio-auto-out.xml | 54 ++++++++++++++++++++
.../qemuxml2xml-console-compat-auto-out.xml | 31 +++++++++++
.../qemuxml2xml-console-virtio-out.xml | 29 +++++++++++
.../qemuxml2xml-disk-scsi-device-auto-out.xml | 31 +++++++++++
.../qemuxml2xml-seclabel-dynamic-in.xml | 24 +++++++++
.../qemuxml2xml-seclabel-dynamic-out.xml | 20 +++++++
.../qemuxml2xml-seclabel-static-in.xml | 24 +++++++++
.../qemuxml2xml-seclabel-static-out.xml | 23 ++++++++
.../qemuxml2xmlout-balloon-device-auto.xml | 25 ---------
.../qemuxml2xmlout-channel-virtio-auto.xml | 54 --------------------
.../qemuxml2xmlout-console-compat-auto.xml | 31 -----------
.../qemuxml2xmlout-console-virtio.xml | 29 -----------
.../qemuxml2xmlout-disk-scsi-device-auto.xml | 31 -----------
tests/qemuxml2xmltest.c | 25 +++++++---
17 files changed, 290 insertions(+), 181 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index a524e4b..9c1e9bb 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -61,9 +61,16 @@
<value>static</value>
</choice>
</attribute>
- <element name="label">
- <text/>
- </element>
+ <optional>
+ <element name="label">
+ <text/>
+ </element>
+ </optional>
+ <optional>
+ <element name="imagelabel">
+ <text/>
+ </element>
+ </optional>
</element>
</define>
<define name="hvs">
diff --git a/tests/domainschematest b/tests/domainschematest
index 7557cef..085ef93 100755
--- a/tests/domainschematest
+++ b/tests/domainschematest
@@ -4,7 +4,7 @@
. $srcdir/test-lib.sh
. $abs_srcdir/schematestutils.sh
-DIRS="domainschemadata qemuxml2argvdata sexpr2xmldata xmconfigdata xml2sexprdata
qemuxml2xmloutdata"
+DIRS="domainschemadata qemuxml2argvdata sexpr2xmldata xmconfigdata xml2sexprdata
qemuxml2xmldata"
SCHEMA="domain.rng"
check_schema "$DIRS" "$SCHEMA"
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
new file mode 100644
index 0000000..ed91e37
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
@@ -0,0 +1,25 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
new file mode 100644
index 0000000..7990374
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
@@ -0,0 +1,54 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu cpuset='1-4,8-20,525'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <controller type='virtio-serial' index='0' ports='16'
vectors='4'/>
+ <controller type='virtio-serial' index='1'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0a' function='0x0'/>
+ </controller>
+ <controller type='virtio-serial' index='2'/>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='0'/>
+ </channel>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.foo'/>
+ <address type='virtio-serial' controller='1' bus='0'
port='0'/>
+ </channel>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.bar'/>
+ <address type='virtio-serial' controller='1' bus='0'
port='3'/>
+ </channel>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.wizz'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.ooh'/>
+ <address type='virtio-serial' controller='1' bus='0'
port='4'/>
+ </channel>
+ <channel type='pty'>
+ <target type='virtio' name='org.linux-kvm.port.lla'/>
+ <address type='virtio-serial' controller='2' bus='0'
port='0'/>
+ </channel>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
new file mode 100644
index 0000000..9591c87
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <serial type='pty'>
+ <target port='0'/>
+ </serial>
+ <console type='pty'>
+ <target type='serial' port='0'/>
+ </console>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
new file mode 100644
index 0000000..431dd34
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu cpuset='1-4,8-20,525'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <controller type='virtio-serial' index='0'/>
+ <console type='pty'>
+ <target type='virtio' port='0'/>
+ </console>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
new file mode 100644
index 0000000..a250940
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <source file='/tmp/scsidisk.img'/>
+ <target dev='sda' bus='scsi'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <controller type='scsi' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
new file mode 100644
index 0000000..5cbac90
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
@@ -0,0 +1,24 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='dynamic' model='selinux'>
+ <label>foolabel</label>
+ <imagelabel>fooimagelabel</imagelabel>
+ </seclabel>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
new file mode 100644
index 0000000..8b344d7
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
@@ -0,0 +1,20 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
new file mode 100644
index 0000000..0837a68
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
@@ -0,0 +1,24 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='static' model='apparmor'>
+ <label>foolabel</label>
+ <imagelabel>fooimagelabel</imagelabel>
+ </seclabel>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
new file mode 100644
index 0000000..9954175
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='static' model='apparmor'>
+ <label>foolabel</label>
+ </seclabel>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
deleted file mode 100644
index ed91e37..0000000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
+++ /dev/null
@@ -1,25 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory>219200</memory>
- <currentMemory>219200</currentMemory>
- <vcpu>1</vcpu>
- <os>
- <type arch='i686' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu</emulator>
- <disk type='block' device='disk'>
- <source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda' bus='ide'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <controller type='ide' index='0'/>
- <memballoon model='virtio'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
deleted file mode 100644
index 7990374..0000000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
+++ /dev/null
@@ -1,54 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory>219200</memory>
- <currentMemory>219200</currentMemory>
- <vcpu cpuset='1-4,8-20,525'>1</vcpu>
- <os>
- <type arch='i686' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu</emulator>
- <disk type='block' device='disk'>
- <source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda' bus='ide'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <controller type='ide' index='0'/>
- <controller type='virtio-serial' index='0' ports='16'
vectors='4'/>
- <controller type='virtio-serial' index='1'>
- <address type='pci' domain='0x0000' bus='0x00'
slot='0x0a' function='0x0'/>
- </controller>
- <controller type='virtio-serial' index='2'/>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.0'/>
- <address type='virtio-serial' controller='0' bus='0'
port='0'/>
- </channel>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.foo'/>
- <address type='virtio-serial' controller='1' bus='0'
port='0'/>
- </channel>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.bar'/>
- <address type='virtio-serial' controller='1' bus='0'
port='3'/>
- </channel>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.wizz'/>
- <address type='virtio-serial' controller='0' bus='0'
port='1'/>
- </channel>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.ooh'/>
- <address type='virtio-serial' controller='1' bus='0'
port='4'/>
- </channel>
- <channel type='pty'>
- <target type='virtio' name='org.linux-kvm.port.lla'/>
- <address type='virtio-serial' controller='2' bus='0'
port='0'/>
- </channel>
- <memballoon model='virtio'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
deleted file mode 100644
index 9591c87..0000000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
+++ /dev/null
@@ -1,31 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory>219200</memory>
- <currentMemory>219200</currentMemory>
- <vcpu>1</vcpu>
- <os>
- <type arch='i686' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu</emulator>
- <disk type='block' device='disk'>
- <source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda' bus='ide'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <controller type='ide' index='0'/>
- <serial type='pty'>
- <target port='0'/>
- </serial>
- <console type='pty'>
- <target type='serial' port='0'/>
- </console>
- <memballoon model='virtio'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
deleted file mode 100644
index 431dd34..0000000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
+++ /dev/null
@@ -1,29 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory>219200</memory>
- <currentMemory>219200</currentMemory>
- <vcpu cpuset='1-4,8-20,525'>1</vcpu>
- <os>
- <type arch='i686' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu</emulator>
- <disk type='block' device='disk'>
- <source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda' bus='ide'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <controller type='ide' index='0'/>
- <controller type='virtio-serial' index='0'/>
- <console type='pty'>
- <target type='virtio' port='0'/>
- </console>
- <memballoon model='virtio'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
deleted file mode 100644
index a250940..0000000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
+++ /dev/null
@@ -1,31 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory>219200</memory>
- <currentMemory>219200</currentMemory>
- <vcpu>1</vcpu>
- <os>
- <type arch='i686' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu</emulator>
- <disk type='block' device='disk'>
- <source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda' bus='ide'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <disk type='file' device='disk'>
- <source file='/tmp/scsidisk.img'/>
- <target dev='sda' bus='scsi'/>
- <address type='drive' controller='0' bus='0'
unit='0'/>
- </disk>
- <controller type='ide' index='0'/>
- <controller type='scsi' index='0'/>
- <memballoon model='virtio'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 326a1f1..2af7494 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -15,9 +15,14 @@
# include "qemu/qemu_conf.h"
# include "testutilsqemu.h"
+# define XML2ARGV_FMT "%s/qemuxml2argvdata/qemuxml2argv-%s.xml"
+# define XML2XMLIN_FMT "%s/qemuxml2xmldata/qemuxml2xml-%s-in.xml"
+# define XML2XMLOUT_FMT "%s/qemuxml2xmldata/qemuxml2xml-%s-out.xml"
+
static char *progname;
static char *abs_srcdir;
static struct qemud_driver driver;
+static char *input_folder_fmt;
# define MAX_FILE 4096
@@ -58,7 +63,7 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char
*outxml) {
struct testInfo {
const char *name;
- int different;
+ bool different;
};
static int testCompareXMLToXMLHelper(const void *data) {
@@ -67,10 +72,8 @@ static int testCompareXMLToXMLHelper(const void *data) {
char xml_out[PATH_MAX];
int ret;
- snprintf(xml_in, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml",
- abs_srcdir, info->name);
- snprintf(xml_out, PATH_MAX, "%s/qemuxml2xmloutdata/qemuxml2xmlout-%s.xml",
- abs_srcdir, info->name);
+ snprintf(xml_in, PATH_MAX, input_folder_fmt, abs_srcdir, info->name);
+ snprintf(xml_out, PATH_MAX, XML2XMLOUT_FMT, abs_srcdir, info->name);
if (info->different) {
ret = testCompareXMLToXMLFiles(xml_in, xml_out);
@@ -111,16 +114,18 @@ mymain(int argc, char **argv)
} while (0)
# define DO_TEST(name) \
- DO_TEST_FULL(name, 0)
+ DO_TEST_FULL(name, false)
# define DO_TEST_DIFFERENT(name) \
- DO_TEST_FULL(name, 1)
+ DO_TEST_FULL(name, true)
/* Unset or set all envvars here that are copied in qemudBuildCommandLine
* using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
* values for these envvars */
setenv("PATH", "/bin", 1);
+ input_folder_fmt = (char *) XML2ARGV_FMT;
+
DO_TEST("minimal");
DO_TEST("boot-cdrom");
DO_TEST("boot-network");
@@ -190,6 +195,12 @@ mymain(int argc, char **argv)
DO_TEST_DIFFERENT("disk-scsi-device-auto");
DO_TEST_DIFFERENT("console-virtio");
+
+ /* All tests after this point use input and output from qemuxml2xmldata*/
+ input_folder_fmt = (char *) XML2XMLIN_FMT;
+ DO_TEST_DIFFERENT("seclabel-dynamic");
+ DO_TEST_DIFFERENT("seclabel-static");
+
virCapabilitiesFree(driver.caps);
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
--
1.7.3.2
12:39 p.m.
New subject: [libvirt] [PATCH 1/7] tests: Add qemuxml2xml tests for <seclabel> handling
On 01/12/2011 10:22 AM, Cole Robinson wrote:
+++ b/tests/domainschematest
@@ -4,7 +4,7 @@
. $srcdir/test-lib.sh
. $abs_srcdir/schematestutils.sh
-DIRS="domainschemadata qemuxml2argvdata sexpr2xmldata xmconfigdata xml2sexprdata
qemuxml2xmloutdata"
+DIRS="domainschemadata qemuxml2argvdata sexpr2xmldata xmconfigdata xml2sexprdata
qemuxml2xmldata"
At first, I though this should be listing _both_ qemuxml2xml{out,}data,
rather than just one or the other, but now I see that you've moved the
only five files that were in that directory so that all qemu*.xml files
are in a single location, and adjusted the test to match. Nice cleanup,
but incomplete - you also need to tweak tests/Makefile.am to drop
reference to the deleted qemuxml2xmloutdata directory, so that 'make
dist' won't complain.
This would have been a little more obvious to me if you had used git
diff rename compression (git config diff.renames true), rather than
showing the moved files as deletions and creations.
In fact, separating into two patches (file movement consolidation, vs.
new tests), might be nice, if you're up to the task, but I won't insist.
@@ -67,10 +72,8 @@ static int testCompareXMLToXMLHelper(const void
*data) {
char xml_out[PATH_MAX];
int ret;
- snprintf(xml_in, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml",
- abs_srcdir, info->name);
- snprintf(xml_out, PATH_MAX,
"%s/qemuxml2xmloutdata/qemuxml2xmlout-%s.xml",
- abs_srcdir, info->name);
+ snprintf(xml_in, PATH_MAX, input_folder_fmt, abs_srcdir, info->name);
+ snprintf(xml_out, PATH_MAX, XML2XMLOUT_FMT, abs_srcdir, info->name);
Hmm, this silently ignores overflow if run in a deeply nested hierarchy.
Someday, we should switch these to virAsprintf (and as a bonus, it
avoids PATH_MAX stack allocation). But that's independent of this patch.
Conditional ACK, once you fix the tests/Makefile.am EXTRA_DIST nit.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 a.m.
New subject: [libvirt] [PATCH 1/7] tests: Add qemuxml2xml tests for <seclabel> handling
On Wed, Jan 12, 2011 at 12:22:57PM -0500, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
docs/schemas/domain.rng | 13 ++++-
tests/domainschematest | 2 +-
.../qemuxml2xml-balloon-device-auto-out.xml | 25 +++++++++
.../qemuxml2xml-channel-virtio-auto-out.xml | 54 ++++++++++++++++++++
.../qemuxml2xml-console-compat-auto-out.xml | 31 +++++++++++
.../qemuxml2xml-console-virtio-out.xml | 29 +++++++++++
.../qemuxml2xml-disk-scsi-device-auto-out.xml | 31 +++++++++++
.../qemuxml2xml-seclabel-dynamic-in.xml | 24 +++++++++
.../qemuxml2xml-seclabel-dynamic-out.xml | 20 +++++++
.../qemuxml2xml-seclabel-static-in.xml | 24 +++++++++
.../qemuxml2xml-seclabel-static-out.xml | 23 ++++++++
.../qemuxml2xmlout-balloon-device-auto.xml | 25 ---------
.../qemuxml2xmlout-channel-virtio-auto.xml | 54 --------------------
.../qemuxml2xmlout-console-compat-auto.xml | 31 -----------
.../qemuxml2xmlout-console-virtio.xml | 29 -----------
.../qemuxml2xmlout-disk-scsi-device-auto.xml | 31 -----------
tests/qemuxml2xmltest.c | 25 +++++++---
17 files changed, 290 insertions(+), 181 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-balloon-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-channel-virtio-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-compat-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-console-virtio-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-disk-scsi-device-auto-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-static-out.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml
delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml
ACK with the caveat Eric mentioned already.
Daniel
11:22 a.m.
New subject: [libvirt] [PATCH 2/7] security: Use virDomainSeclabelDefClear
Renamed from virSecurityLabelDefFree.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
cfg.mk | 1 +
src/conf/domain_conf.c | 17 +++++++++--------
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c | 4 +---
src/security/security_apparmor.c | 11 ++---------
src/security/security_selinux.c | 9 ++-------
7 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 03186b3..2c6f595 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -98,6 +98,7 @@ useless_free_options = \
--name=virDomainInputDefFree \
--name=virDomainNetDefFree \
--name=virDomainObjFree \
+ --name=virDomainSeclabelDefClear \
--name=virDomainSnapshotDefFree \
--name=virDomainSnapshotObjFree \
--name=virDomainSoundDefFree \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b4df38c..8f6ef55 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -754,13 +754,14 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
VIR_FREE(def);
}
-void virSecurityLabelDefFree(virDomainDefPtr def);
-
-void virSecurityLabelDefFree(virDomainDefPtr def)
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
{
- VIR_FREE(def->seclabel.model);
- VIR_FREE(def->seclabel.label);
- VIR_FREE(def->seclabel.imagelabel);
+ if (!seclabel)
+ return;
+
+ VIR_FREE(seclabel->model);
+ VIR_FREE(seclabel->label);
+ VIR_FREE(seclabel->imagelabel);
}
static void
@@ -855,7 +856,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainMemballoonDefFree(def->memballoon);
- virSecurityLabelDefFree(def);
+ virDomainSeclabelDefClear(&def->seclabel);
virCPUDefFree(def->cpu);
@@ -4272,7 +4273,7 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
return 0;
error:
- virSecurityLabelDefFree(def);
+ virDomainSeclabelDefClear(&def->seclabel);
return -1;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a459a22..b5cf433 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1082,6 +1082,7 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel);
int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e9b8cb7..191ae6a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -279,6 +279,7 @@ virDomainRemoveInactive;
virDomainSaveConfig;
virDomainSaveStatus;
virDomainSaveXML;
+virDomainSeclabelDefClear;
virDomainSnapshotAssignDef;
virDomainSnapshotDefFormat;
virDomainSnapshotDefFree;
@@ -313,7 +314,6 @@ virDomainWatchdogActionTypeToString;
virDomainWatchdogModelTypeFromString;
virDomainWatchdogModelTypeToString;
-
# domain_event.h
virDomainEventCallbackListAdd;
virDomainEventCallbackListAddID;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9eb9cd5..3745cce 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3043,9 +3043,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
/* Clear out dynamically assigned labels */
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
- VIR_FREE(vm->def->seclabel.model);
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
}
virDomainDefClearDeviceAliases(vm->def);
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index d82ba73..42f812c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -475,9 +475,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
goto clean;
err:
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
- VIR_FREE(vm->def->seclabel.model);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
clean:
VIR_FREE(profile_name);
@@ -547,12 +545,7 @@ static int
AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainObjPtr vm)
{
- const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
-
- VIR_FREE(secdef->model);
- VIR_FREE(secdef->label);
- VIR_FREE(secdef->imagelabel);
-
+ virDomainSeclabelDefClear(&vm->def->seclabel);
return 0;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index d06afde..f11e209 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -216,9 +216,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
rc = 0;
goto done;
err:
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
- VIR_FREE(vm->def->seclabel.model);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
done:
VIR_FREE(scontext);
return rc;
@@ -830,10 +828,7 @@ SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
context_free(con);
}
- VIR_FREE(secdef->model);
- VIR_FREE(secdef->label);
- VIR_FREE(secdef->imagelabel);
-
+ virDomainSeclabelDefClear(secdef);
return 0;
}
--
1.7.3.2
2:28 p.m.
New subject: [libvirt] [PATCH 2/7] security: Use virDomainSeclabelDefClear
On 01/12/2011 10:22 AM, Cole Robinson wrote:
Renamed from virSecurityLabelDefFree.
I like the rename, but...
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
cfg.mk | 1 +
src/conf/domain_conf.c | 17 +++++++++--------
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c | 4 +---
src/security/security_apparmor.c | 11 ++---------
src/security/security_selinux.c | 9 ++-------
7 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 03186b3..2c6f595 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -98,6 +98,7 @@ useless_free_options = \
--name=virDomainInputDefFree \
--name=virDomainNetDefFree \
--name=virDomainObjFree \
+ --name=virDomainSeclabelDefClear \
This is wrong. It is _not_ a free-like function, because it does not
free the passed in pointer. You probably want to have two functions:
/* clear guts only, must be given valid pointer */
void virDomainSeclabelDefClear(ptr) ATTRIBUTE_NONNULL(1);
/* clear entire allocated pointer, free-like */
void virDomainSeclabelDefFree(ptr) {
if (!ptr) return;
virDomainSeclabelDefClear(ptr);
VIR_FREE(ptr);
}
and list only virDomainSeclabelDefFree in cfg.mk.
-void virSecurityLabelDefFree(virDomainDefPtr def)
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
{
- VIR_FREE(def->seclabel.model);
- VIR_FREE(def->seclabel.label);
- VIR_FREE(def->seclabel.imagelabel);
+ if (!seclabel)
+ return;
which means this early exit should only be part of the Free variant, not
the Clear variant.
+
+ VIR_FREE(seclabel->model);
+ VIR_FREE(seclabel->label);
+ VIR_FREE(seclabel->imagelabel);
}
static void
@@ -855,7 +856,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainMemballoonDefFree(def->memballoon);
- virSecurityLabelDefFree(def);
+ virDomainSeclabelDefClear(&def->seclabel);
But hunks like this are okay (when a SeclabelDef occurs as an inline
struct member, rather than an indirect SeclabelDefPtr pointer).
+++ b/src/libvirt_private.syms
@@ -279,6 +279,7 @@ virDomainRemoveInactive;
virDomainSaveConfig;
virDomainSaveStatus;
virDomainSaveXML;
+virDomainSeclabelDefClear;
virDomainSnapshotAssignDef;
virDomainSnapshotDefFormat;
virDomainSnapshotDefFree;
@@ -313,7 +314,6 @@ virDomainWatchdogActionTypeToString;
virDomainWatchdogModelTypeFromString;
virDomainWatchdogModelTypeToString;
-
# domain_event.h
Why the whitespace change?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:15 a.m.
New subject: [libvirt] [PATCH 2/7] security: Use virDomainSeclabelDefClear
On Wed, Jan 12, 2011 at 12:22:58PM -0500, Cole Robinson wrote:
Renamed from virSecurityLabelDefFree.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
cfg.mk | 1 +
src/conf/domain_conf.c | 17 +++++++++--------
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c | 4 +---
src/security/security_apparmor.c | 11 ++---------
src/security/security_selinux.c | 9 ++-------
7 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 03186b3..2c6f595 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -98,6 +98,7 @@ useless_free_options = \
--name=virDomainInputDefFree \
--name=virDomainNetDefFree \
--name=virDomainObjFree \
+ --name=virDomainSeclabelDefClear \
This isn't needed
--name=virDomainSnapshotDefFree \
--name=virDomainSnapshotObjFree \
--name=virDomainSoundDefFree \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b4df38c..8f6ef55 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -754,13 +754,14 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
VIR_FREE(def);
}
-void virSecurityLabelDefFree(virDomainDefPtr def);
-
-void virSecurityLabelDefFree(virDomainDefPtr def)
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
{
- VIR_FREE(def->seclabel.model);
- VIR_FREE(def->seclabel.label);
- VIR_FREE(def->seclabel.imagelabel);
+ if (!seclabel)
+ return;
Nor is this.
+ VIR_FREE(seclabel->model);
+ VIR_FREE(seclabel->label);
+ VIR_FREE(seclabel->imagelabel);
}
static void
@@ -855,7 +856,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainMemballoonDefFree(def->memballoon);
- virSecurityLabelDefFree(def);
+ virDomainSeclabelDefClear(&def->seclabel);
virCPUDefFree(def->cpu);
@@ -4272,7 +4273,7 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
return 0;
error:
- virSecurityLabelDefFree(def);
+ virDomainSeclabelDefClear(&def->seclabel);
return -1;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a459a22..b5cf433 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1082,6 +1082,7 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel);
int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e9b8cb7..191ae6a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -279,6 +279,7 @@ virDomainRemoveInactive;
virDomainSaveConfig;
virDomainSaveStatus;
virDomainSaveXML;
+virDomainSeclabelDefClear;
virDomainSnapshotAssignDef;
virDomainSnapshotDefFormat;
virDomainSnapshotDefFree;
@@ -313,7 +314,6 @@ virDomainWatchdogActionTypeToString;
virDomainWatchdogModelTypeFromString;
virDomainWatchdogModelTypeToString;
-
# domain_event.h
virDomainEventCallbackListAdd;
virDomainEventCallbackListAddID;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9eb9cd5..3745cce 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3043,9 +3043,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
/* Clear out dynamically assigned labels */
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
- VIR_FREE(vm->def->seclabel.model);
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
}
virDomainDefClearDeviceAliases(vm->def);
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index d82ba73..42f812c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -475,9 +475,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
goto clean;
err:
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
- VIR_FREE(vm->def->seclabel.model);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
clean:
VIR_FREE(profile_name);
@@ -547,12 +545,7 @@ static int
AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainObjPtr vm)
{
- const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
-
- VIR_FREE(secdef->model);
- VIR_FREE(secdef->label);
- VIR_FREE(secdef->imagelabel);
-
+ virDomainSeclabelDefClear(&vm->def->seclabel);
return 0;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index d06afde..f11e209 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -216,9 +216,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
rc = 0;
goto done;
err:
- VIR_FREE(vm->def->seclabel.label);
- VIR_FREE(vm->def->seclabel.imagelabel);
- VIR_FREE(vm->def->seclabel.model);
+ virDomainSeclabelDefClear(&vm->def->seclabel);
done:
VIR_FREE(scontext);
return rc;
@@ -830,10 +828,7 @@ SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
context_free(con);
}
- VIR_FREE(secdef->model);
- VIR_FREE(secdef->label);
- VIR_FREE(secdef->imagelabel);
-
+ virDomainSeclabelDefClear(secdef);
return 0;
}
ACK to the rest of the patch
Daniel
11:22 a.m.
New subject: [libvirt] [PATCH 3/7] security: Add virSecurityIsSpecifiedDriver
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_apparmor.c | 7 +------
src/security/security_driver.c | 19 +++++++++++++++++++
src/security/security_driver.h | 3 +++
src/security/security_selinux.c | 29 +++++------------------------
4 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 42f812c..00e5a01 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -581,12 +581,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
if ((profile_name = get_profile_name(vm)) == NULL)
return rc;
- if (STRNEQ(virSecurityManagerGetModel(mgr), secdef->model)) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("security label driver mismatch: "
- "\'%s\' model configured for domain, but
"
- "hypervisor driver is \'%s\'."),
- secdef->model, virSecurityManagerGetModel(mgr));
+ if (!virSecurityIsSpecifiedDriver(mgr, vm->def)) {
if (use_apparmor() > 0)
goto clean;
}
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index fd2c01a..5711aee 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -75,3 +75,22 @@ virSecurityDriverPtr virSecurityDriverLookup(const char *name)
return drv;
}
+
+bool
+virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
+ virDomainDefPtr def)
+{
+ bool ret = true;
+
+ if (!STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
+ virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label driver mismatch: "
+ "'%s' model configured for domain, but
"
+ "hypervisor driver is '%s'."),
+ def->seclabel.model,
+ virSecurityManagerGetModel(mgr));
+ ret = false;
+ }
+
+ return ret;
+}
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index e5a8d41..a0b15f4 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -117,5 +117,8 @@ struct _virSecurityDriver {
};
virSecurityDriverPtr virSecurityDriverLookup(const char *name);
+bool virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
+ virDomainDefPtr def)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
#endif /* __VIR_SECURITY_H__ */
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f11e209..f3b76f9 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -866,12 +866,8 @@ SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def)
{
const virSecurityLabelDefPtr secdef = &def->seclabel;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("security label driver mismatch: "
- "'%s' model configured for domain, but
"
- "hypervisor driver is '%s'."),
- secdef->model, virSecurityManagerGetModel(mgr));
+
+ if (!virSecurityIsSpecifiedDriver(mgr, def)) {
return -1;
}
@@ -895,12 +891,7 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr,
if (vm->def->seclabel.label == NULL)
return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("security label driver mismatch: "
- "'%s' model configured for domain, but
"
- "hypervisor driver is '%s'."),
- secdef->model, virSecurityManagerGetModel(mgr));
+ if (!virSecurityIsSpecifiedDriver(mgr, vm->def)) {
if (security_getenforce() == 1)
return -1;
}
@@ -930,12 +921,7 @@ SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr,
if (vm->def->seclabel.label == NULL)
return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("security label driver mismatch: "
- "'%s' model configured for domain, but
"
- "hypervisor driver is '%s'."),
- secdef->model, virSecurityManagerGetModel(mgr));
+ if (!virSecurityIsSpecifiedDriver(mgr, vm->def)) {
goto done;
}
@@ -997,12 +983,7 @@ SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr,
if (vm->def->seclabel.label == NULL)
return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("security label driver mismatch: "
- "'%s' model configured for domain, but
"
- "hypervisor driver is '%s'."),
- secdef->model, virSecurityManagerGetModel(mgr));
+ if (!virSecurityIsSpecifiedDriver(mgr, vm->def)) {
if (security_getenforce() == 1)
return -1;
}
--
1.7.3.2
2:31 p.m.
New subject: [libvirt] [PATCH 3/7] security: Add virSecurityIsSpecifiedDriver
On 01/12/2011 10:22 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_apparmor.c | 7 +------
src/security/security_driver.c | 19 +++++++++++++++++++
src/security/security_driver.h | 3 +++
src/security/security_selinux.c | 29 +++++------------------------
4 files changed, 28 insertions(+), 30 deletions(-)
The summary line of the diffstat doesn't show it very well, but I like
this refactorization.
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:16 a.m.
New subject: [libvirt] [PATCH 3/7] security: Add virSecurityIsSpecifiedDriver
On Wed, Jan 12, 2011 at 12:22:59PM -0500, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_apparmor.c | 7 +------
src/security/security_driver.c | 19 +++++++++++++++++++
src/security/security_driver.h | 3 +++
src/security/security_selinux.c | 29 +++++------------------------
4 files changed, 28 insertions(+), 30 deletions(-)
ACK
Daniel
11:23 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Fix segfault if defining a domain without <seclabel>
If the selinux driver is the default, it will lead to a null dereference.
This change should still yeild the intended result, since a null model
basically implies 'use the default'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 5711aee..7d2e0de 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -82,7 +82,8 @@ virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
{
bool ret = true;
- if (!STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
+ if (def->seclabel.model &&
+ !STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
_("security label driver mismatch: "
"'%s' model configured for domain, but
"
--
1.7.3.2
2:32 p.m.
New subject: [libvirt] [PATCH 4/7] qemu: Fix segfault if defining a domain without <seclabel>
On 01/12/2011 10:23 AM, Cole Robinson wrote:
If the selinux driver is the default, it will lead to a null
dereference.
This change should still yeild the intended result, since a null model
basically implies 'use the default'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 5711aee..7d2e0de 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -82,7 +82,8 @@ virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
{
bool ret = true;
- if (!STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
+ if (def->seclabel.model &&
+ !STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
ACK; and this probably also fixes segfaults for other drivers given your
3/7 cleanup.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:20 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Fix segfault if defining a domain without <seclabel>
On Wed, Jan 12, 2011 at 12:23:00PM -0500, Cole Robinson wrote:
If the selinux driver is the default, it will lead to a null
dereference.
This change should still yeild the intended result, since a null model
basically implies 'use the default'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 5711aee..7d2e0de 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -82,7 +82,8 @@ virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
{
bool ret = true;
- if (!STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
+ if (def->seclabel.model &&
+ !STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
_("security label driver mismatch: "
"'%s' model configured for domain, but
"
ACK it doesn't hurt, but it should have also been checked at the
higher level in the Verify function
Daniel
11:23 a.m.
New subject: [libvirt] [PATCH 5/7] domain: Handle seclabel model with an enum
This allows us to explicitly handle the 'default' seclabel case, as
well as provide easier model validation.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++--------
src/conf/domain_conf.h | 14 ++++++++++++--
src/security/security_apparmor.c | 9 +++------
src/security/security_driver.c | 15 ++++++++++-----
src/security/security_selinux.c | 8 ++------
5 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f6ef55..077a396 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -313,6 +313,12 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
"dynamic",
"static")
+VIR_ENUM_IMPL(virDomainSeclabelModel, VIR_DOMAIN_SECLABEL_MODEL_LAST,
+ "default",
+ "selinux",
+ "apparmor",
+ "none")
+
VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
"vepa",
"private",
@@ -759,7 +765,7 @@ void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
if (!seclabel)
return;
- VIR_FREE(seclabel->model);
+ seclabel->model = VIR_DOMAIN_SECLABEL_MODEL_DEFAULT;
VIR_FREE(seclabel->label);
VIR_FREE(seclabel->imagelabel);
}
@@ -4244,7 +4250,15 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
"%s", _("missing security model"));
goto error;
}
- def->seclabel.model = p;
+
+ def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
+ if (def->seclabel.model < 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("unknown security model '%s'"), p);
+ VIR_FREE(p);
+ goto error;
+ }
+ VIR_FREE(p);
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
@@ -7336,18 +7350,26 @@ char *virDomainDefFormat(virDomainDefPtr def,
virBufferAddLit(&buf, " </devices>\n");
- if (def->seclabel.model) {
- const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type);
+ if (def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) {
+ const char *sectype, *secmodel;
+
+ sectype = virDomainSeclabelTypeToString(def->seclabel.type);
if (!sectype)
goto cleanup;
+
+ secmodel = virDomainSeclabelModelTypeToString(def->seclabel.model);
+ if (!secmodel)
+ goto cleanup;
+
+ virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'",
+ sectype, secmodel);
+
if (!def->seclabel.label ||
(def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
(flags & VIR_DOMAIN_XML_INACTIVE))) {
- virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'/>\n",
- sectype, def->seclabel.model);
+ virBufferAddLit(&buf, "/>\n");
} else {
- virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'>\n",
- sectype, def->seclabel.model);
+ virBufferAddLit(&buf, ">\n");
virBufferEscapeString(&buf, "
<label>%s</label>\n",
def->seclabel.label);
if (def->seclabel.imagelabel &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b5cf433..81409f8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -782,14 +782,23 @@ enum virDomainSeclabelType {
VIR_DOMAIN_SECLABEL_LAST,
};
+enum virDomainSeclabelModel {
+ VIR_DOMAIN_SECLABEL_MODEL_DEFAULT,
+ VIR_DOMAIN_SECLABEL_MODEL_SELINUX,
+ VIR_DOMAIN_SECLABEL_MODEL_APPARMOR,
+ VIR_DOMAIN_SECLABEL_MODEL_NONE,
+
+ VIR_DOMAIN_SECLABEL_MODEL_LAST,
+};
+
/* Security configuration for domain */
typedef struct _virSecurityLabelDef virSecurityLabelDef;
typedef virSecurityLabelDef *virSecurityLabelDefPtr;
struct _virSecurityLabelDef {
- char *model; /* name of security model */
char *label; /* security label string */
char *imagelabel; /* security image label string */
- int type;
+ int type; /* dynamic vs. static */
+ int model; /* name of security model */
};
enum virDomainTimerNameType {
@@ -1287,6 +1296,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode)
/* from libvirt.h */
VIR_ENUM_DECL(virDomainState)
VIR_ENUM_DECL(virDomainSeclabel)
+VIR_ENUM_DECL(virDomainSeclabelModel)
VIR_ENUM_DECL(virDomainClockOffset)
VIR_ENUM_DECL(virDomainNetdevMacvtap)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 00e5a01..7a6fe5c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -441,7 +441,8 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return 0;
if ((vm->def->seclabel.label) ||
- (vm->def->seclabel.model) || (vm->def->seclabel.imagelabel)) {
+ (vm->def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) ||
+ (vm->def->seclabel.imagelabel)) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
_("security label already defined for VM"));
@@ -465,11 +466,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
goto err;
}
- vm->def->seclabel.model = strdup(SECURITY_APPARMOR_NAME);
- if (!vm->def->seclabel.model) {
- virReportOOMError();
- goto err;
- }
+ vm->def->seclabel.model = VIR_DOMAIN_SECLABEL_MODEL_APPARMOR;
rc = 0;
goto clean;
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 7d2e0de..2cd4c0e 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -80,18 +80,23 @@ bool
virSecurityIsSpecifiedDriver(virSecurityManagerPtr mgr,
virDomainDefPtr def)
{
- bool ret = true;
+ bool ret = false;
+ const char *model = virDomainSeclabelModelTypeToString(def->seclabel.model);
+ if (!model)
+ goto err;
- if (def->seclabel.model &&
- !STREQ(virSecurityManagerGetModel(mgr), def->seclabel.model)) {
+ if (def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT &&
+ !STREQ(virSecurityManagerGetModel(mgr), model)) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
_("security label driver mismatch: "
"'%s' model configured for domain, but
"
"hypervisor driver is '%s'."),
- def->seclabel.model,
+ model,
virSecurityManagerGetModel(mgr));
- ret = false;
+ goto err;
}
+ ret = true;
+err:
return ret;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f3b76f9..2266c21 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -173,7 +173,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return 0;
if (vm->def->seclabel.label ||
- vm->def->seclabel.model ||
+ vm->def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT ||
vm->def->seclabel.imagelabel) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("security label already defined for
VM"));
@@ -206,12 +206,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
_("cannot generate selinux context for %s"),
mcs);
goto err;
}
- vm->def->seclabel.model = strdup(SECURITY_SELINUX_NAME);
- if (!vm->def->seclabel.model) {
- virReportOOMError();
- goto err;
- }
+ vm->def->seclabel.model = VIR_DOMAIN_SECLABEL_MODEL_SELINUX;
rc = 0;
goto done;
--
1.7.3.2
2:48 p.m.
New subject: [libvirt] [PATCH 5/7] domain: Handle seclabel model with an enum
On 01/12/2011 10:23 AM, Cole Robinson wrote:
This allows us to explicitly handle the 'default' seclabel
case, as
well as provide easier model validation.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
I'm not so sure about this one. I didn't see any coding bugs in the
actual patch, but design-wise:
src/conf/domain_conf.c | 38
++++++++++++++++++++++++++++++--------
src/conf/domain_conf.h | 14 ++++++++++++--
src/security/security_apparmor.c | 9 +++------
src/security/security_driver.c | 15 ++++++++++-----
src/security/security_selinux.c | 8 ++------
5 files changed, 57 insertions(+), 27 deletions(-)
1. it's already a net gain in lines of code
2. to me, it really seems like it will only be useful if we go with
patch 7/7 (which Daniel already NACK'd).
3. ...
+VIR_ENUM_IMPL(virDomainSeclabelModel,
VIR_DOMAIN_SECLABEL_MODEL_LAST,
+ "default",
+ "selinux",
+ "apparmor",
+ "none")
This means the FromString knows how to parse "default",
@@ -4244,7 +4250,15 @@ virSecurityLabelDefParseXML(const
virDomainDefPtr def,
"%s", _("missing security
model"));
goto error;
}
- def->seclabel.model = p;
+
+ def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
+ if (def->seclabel.model < 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("unknown security model '%s'"),
p);
+ VIR_FREE(p);
+ goto error;
+ }
while this doesn't reject such a parse,
@@ -7336,18 +7350,26 @@ char *virDomainDefFormat(virDomainDefPtr
def,
virBufferAddLit(&buf, " </devices>\n");
- if (def->seclabel.model) {
- const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type);
+ if (def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) {
yet this intentionally avoids outputting model='default',
and overall, you didn't modify the domain.rng or docs to mention the new
attribute value. I'm assuming you didn't intend to extend the xml to
allow model='default', which means you either want:
+enum virDomainSeclabelModel {
+ VIR_DOMAIN_SECLABEL_MODEL_DEFAULT,
this to be = -1,
+ VIR_DOMAIN_SECLABEL_MODEL_SELINUX,
and this 0, and just omit "default" from the VIR_ENUM_IMPL; or that you
have to correct the parseXML function to reject an explicit
model='default' parse. Since non-zero defaults have their own set of
interesting issues (you'd have to track down every place in code that
allocates a virDomainSeclabelDef and ensure that it gets initialized
correctly), I'd lean to the latter option, but both options add
complexity to this patch.
So unless we can come up with another reason why this is worth
including, I'm inclined to just omit this one.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:25 a.m.
New subject: [libvirt] [PATCH 5/7] domain: Handle seclabel model with an enum
On Wed, Jan 12, 2011 at 01:48:24PM -0700, Eric Blake wrote:
On 01/12/2011 10:23 AM, Cole Robinson wrote:
> This allows us to explicitly handle the 'default' seclabel case, as
> well as provide easier model validation.
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
I'm not so sure about this one. I didn't see any coding bugs in the
actual patch, but design-wise:
> src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++--------
> src/conf/domain_conf.h | 14 ++++++++++++--
> src/security/security_apparmor.c | 9 +++------
> src/security/security_driver.c | 15 ++++++++++-----
> src/security/security_selinux.c | 8 ++------
> 5 files changed, 57 insertions(+), 27 deletions(-)
1. it's already a net gain in lines of code
2. to me, it really seems like it will only be useful if we go with
patch 7/7 (which Daniel already NACK'd).
Even though the code is a bit longer, I think it is good
practice to use an ENUM for handling this.
> +VIR_ENUM_IMPL(virDomainSeclabelModel,
VIR_DOMAIN_SECLABEL_MODEL_LAST,
> + "default",
> + "selinux",
> + "apparmor",
> + "none")
This means the FromString knows how to parse "default",
> @@ -4244,7 +4250,15 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
> "%s", _("missing security
model"));
> goto error;
> }
> - def->seclabel.model = p;
> +
> + def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
> + if (def->seclabel.model < 0) {
> + virDomainReportError(VIR_ERR_XML_ERROR,
> + _("unknown security model
'%s'"), p);
> + VIR_FREE(p);
> + goto error;
> + }
while this doesn't reject such a parse,
> @@ -7336,18 +7350,26 @@ char *virDomainDefFormat(virDomainDefPtr def,
>
> virBufferAddLit(&buf, " </devices>\n");
>
> - if (def->seclabel.model) {
> - const char *sectype =
virDomainSeclabelTypeToString(def->seclabel.type);
> + if (def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) {
yet this intentionally avoids outputting model='default',
Overall, this kind of state of affairs is OK. I'm fairly
sure we've other areas where the 'default' is silently
accepted in parse, but not output when formatting for
sake of brevity.
and overall, you didn't modify the domain.rng or docs to mention
the new
attribute value. I'm assuming you didn't intend to extend the xml to
allow model='default', which means you either want:
>
> +enum virDomainSeclabelModel {
> + VIR_DOMAIN_SECLABEL_MODEL_DEFAULT,
this to be = -1,
> + VIR_DOMAIN_SECLABEL_MODEL_SELINUX,
and this 0, and just omit "default" from the VIR_ENUM_IMPL; or that you
No, we really do want the default value to be 0. It is important
for defaults to be zero, so that when you VIR_ALLOC you get the
correct defaults automatically.
Regards,
Daniel
11:23 a.m.
New subject: [libvirt] [PATCH 5/7] domain: Handle seclabel model with an enum
On Wed, Jan 12, 2011 at 12:23:01PM -0500, Cole Robinson wrote:
This allows us to explicitly handle the 'default' seclabel
case, as
well as provide easier model validation.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++--------
src/conf/domain_conf.h | 14 ++++++++++++--
src/security/security_apparmor.c | 9 +++------
src/security/security_driver.c | 15 ++++++++++-----
src/security/security_selinux.c | 8 ++------
5 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f6ef55..077a396 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -313,6 +313,12 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
"dynamic",
"static")
+VIR_ENUM_IMPL(virDomainSeclabelModel, VIR_DOMAIN_SECLABEL_MODEL_LAST,
+ "default",
+ "selinux",
+ "apparmor",
+ "none")
If we remove 'none' from the enum, this is ok.
+
VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
"vepa",
"private",
@@ -759,7 +765,7 @@ void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
if (!seclabel)
return;
- VIR_FREE(seclabel->model);
+ seclabel->model = VIR_DOMAIN_SECLABEL_MODEL_DEFAULT;
VIR_FREE(seclabel->label);
VIR_FREE(seclabel->imagelabel);
}
@@ -4244,7 +4250,15 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
"%s", _("missing security
model"));
goto error;
}
- def->seclabel.model = p;
+
+ def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
+ if (def->seclabel.model < 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("unknown security model '%s'"),
p);
+ VIR_FREE(p);
+ goto error;
+ }
+ VIR_FREE(p);
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
@@ -7336,18 +7350,26 @@ char *virDomainDefFormat(virDomainDefPtr def,
virBufferAddLit(&buf, " </devices>\n");
- if (def->seclabel.model) {
- const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type);
+ if (def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) {
+ const char *sectype, *secmodel;
+
+ sectype = virDomainSeclabelTypeToString(def->seclabel.type);
if (!sectype)
goto cleanup;
+
+ secmodel = virDomainSeclabelModelTypeToString(def->seclabel.model);
+ if (!secmodel)
+ goto cleanup;
+
+ virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'",
+ sectype, secmodel);
+
if (!def->seclabel.label ||
(def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
(flags & VIR_DOMAIN_XML_INACTIVE))) {
- virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'/>\n",
- sectype, def->seclabel.model);
+ virBufferAddLit(&buf, "/>\n");
} else {
- virBufferVSprintf(&buf, " <seclabel type='%s'
model='%s'>\n",
- sectype, def->seclabel.model);
+ virBufferAddLit(&buf, ">\n");
virBufferEscapeString(&buf, "
<label>%s</label>\n",
def->seclabel.label);
if (def->seclabel.imagelabel &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b5cf433..81409f8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -782,14 +782,23 @@ enum virDomainSeclabelType {
VIR_DOMAIN_SECLABEL_LAST,
};
+enum virDomainSeclabelModel {
+ VIR_DOMAIN_SECLABEL_MODEL_DEFAULT,
+ VIR_DOMAIN_SECLABEL_MODEL_SELINUX,
+ VIR_DOMAIN_SECLABEL_MODEL_APPARMOR,
+ VIR_DOMAIN_SECLABEL_MODEL_NONE,
+
+ VIR_DOMAIN_SECLABEL_MODEL_LAST,
+};
Remove NONE here too.
ACK, if the 'none' / NONE bits are removed.
Daniel
11:23 a.m.
New subject: [libvirt] [PATCH 6/7] domain: Always validate seclabel model
This will help facilitate disabling seclabel for an individual VM. One
functional change is that the user can now hardcode type='dynamic', but
there was no good reason to deny it anyways.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/conf/domain_conf.c | 34 ++++++++++----------
src/security/security_apparmor.c | 6 ++--
src/security/security_selinux.c | 6 ++--
.../qemuxml2xml-seclabel-dynamic-out.xml | 1 +
4 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 077a396..e5b89a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4238,28 +4238,28 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
goto error;
}
+ p = virXPathStringLimit("string(./seclabel/@model)",
+ VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
+ if (p == NULL) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing security model"));
+ goto error;
+ }
+
+ def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
+ if (def->seclabel.model < 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("unknown security model '%s'"), p);
+ VIR_FREE(p);
+ goto error;
+ }
+ VIR_FREE(p);
+
/* Only parse details, if using static labels, or
* if the 'live' VM XML is requested
*/
if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC ||
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
- p = virXPathStringLimit("string(./seclabel/@model)",
- VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
- if (p == NULL) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security model"));
- goto error;
- }
-
- def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
- if (def->seclabel.model < 0) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- _("unknown security model '%s'"), p);
- VIR_FREE(p);
- goto error;
- }
- VIR_FREE(p);
-
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 7a6fe5c..9a49e29 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -431,7 +431,7 @@ AppArmorSecurityManagerGetDOI(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED)
* called on shutdown.
*/
static int
-AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+AppArmorGenSecurityLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
int rc = -1;
@@ -440,8 +440,8 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC)
return 0;
- if ((vm->def->seclabel.label) ||
- (vm->def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT) ||
+ if ((!virSecurityIsSpecifiedDriver(mgr, vm->def)) ||
+ (vm->def->seclabel.label) ||
(vm->def->seclabel.imagelabel)) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2266c21..a03e7d2 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -160,7 +160,7 @@ SELinuxInitialize(void)
}
static int
-SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+SELinuxGenSecurityLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
int rc = -1;
@@ -172,8 +172,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC)
return 0;
- if (vm->def->seclabel.label ||
- vm->def->seclabel.model != VIR_DOMAIN_SECLABEL_MODEL_DEFAULT ||
+ if (!virSecurityIsSpecifiedDriver(mgr, vm->def) ||
+ vm->def->seclabel.label ||
vm->def->seclabel.imagelabel) {
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("security label already defined for
VM"));
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
index 8b344d7..8d3f837 100644
--- a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-dynamic-out.xml
@@ -17,4 +17,5 @@
<controller type='ide' index='0'/>
<memballoon model='virtio'/>
</devices>
+ <seclabel type='dynamic' model='selinux'/>
</domain>
--
1.7.3.2
3:41 p.m.
New subject: [libvirt] [PATCH 6/7] domain: Always validate seclabel model
On 01/12/2011 10:23 AM, Cole Robinson wrote:
This will help facilitate disabling seclabel for an individual VM.
One
functional change is that the user can now hardcode type='dynamic', but
there was no good reason to deny it anyways.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/conf/domain_conf.c | 34 ++++++++++----------
src/security/security_apparmor.c | 6 ++--
src/security/security_selinux.c | 6 ++--
.../qemuxml2xml-seclabel-dynamic-out.xml | 1 +
4 files changed, 24 insertions(+), 23 deletions(-)
Hmm, the domain.rng states that attribute model is <text/> rather than
limiting it to a <choice> between selinux/apparmor (as currently
supported) or even <choice> selinux/apparmor/none (per your enum in
patch 5/7, as used in patch 7/7).
That might be an independently useful thing to clean up, to tighten the
.rng to match the possible valid values. And maybe 5/7 has a use after
all (but with cleanups to avoid issues with model='default' and to omit
model='none').
Given your commit message, I see what you are getting at - the current
xml parsing does not reject <seclabel type='dynamic' model='bogus'>
for
a defined but inactive domain. At which point this code motion makes
sense, to always validate model to match the list of allowed enum
values. But it could use domain.rng tightening, and
docs/formatdomain.html.in doesn't even mention seclabel, so we'd
probably want that in first. And then there's the question of whether
to go with patch 5/7 as a prereq to this, or whether you should rewrite
the code motion in terms of the state things were in before 5/7.
I guess this means it is worth a v2, if you think it is still worth
keeping this patch in the series.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:31 a.m.
New subject: [libvirt] [PATCH 6/7] domain: Always validate seclabel model
On Wed, Jan 12, 2011 at 12:23:02PM -0500, Cole Robinson wrote:
This will help facilitate disabling seclabel for an individual VM.
One
functional change is that the user can now hardcode type='dynamic', but
there was no good reason to deny it anyways.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/conf/domain_conf.c | 34 ++++++++++----------
src/security/security_apparmor.c | 6 ++--
src/security/security_selinux.c | 6 ++--
.../qemuxml2xml-seclabel-dynamic-out.xml | 1 +
4 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 077a396..e5b89a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4238,28 +4238,28 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
goto error;
}
+ p = virXPathStringLimit("string(./seclabel/@model)",
+ VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
+ if (p == NULL) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing security model"));
+ goto error;
+ }
+
+ def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
+ if (def->seclabel.model < 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("unknown security model '%s'"), p);
+ VIR_FREE(p);
+ goto error;
+ }
+ VIR_FREE(p);
+
/* Only parse details, if using static labels, or
* if the 'live' VM XML is requested
*/
if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC ||
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
- p = virXPathStringLimit("string(./seclabel/@model)",
- VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
- if (p == NULL) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security
model"));
- goto error;
- }
-
- def->seclabel.model = virDomainSeclabelModelTypeFromString(p);
- if (def->seclabel.model < 0) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- _("unknown security model '%s'"),
p);
- VIR_FREE(p);
- goto error;
- }
- VIR_FREE(p);
-
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
This changes semantics. If the seclabel type is dynamic, then we
want to ignore any kind of 'model' at all, because the model should
automatically become whatever is current active driver. This ensures
that if you change security drivers, then all dynamic VMs will
automatically use the new driver and not be stuck with the model
of the old driver.
Since, we're not supporting per-VM disabled models, I don't think
we need this patch anyway.
Regards,
Daniel
11:23 a.m.
New subject: [libvirt] [PATCH 7/7] security: Allow disabling security on a per VM basis
Make the SecurityManager explicitly handle the case when seclabel
model='none'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_manager.c | 90 +++++++++++++-------
.../qemuxml2xml-seclabel-model-none-in.xml | 21 +++++
.../qemuxml2xml-seclabel-model-none-out.xml | 21 +++++
tests/qemuxml2xmltest.c | 1 +
4 files changed, 101 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 66cffb5..9f98886 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -122,6 +122,16 @@ void virSecurityManagerFree(virSecurityManagerPtr mgr)
VIR_FREE(mgr);
}
+static virSecurityDriverPtr
+virSecurityManagerGetDriver(virSecurityManagerPtr mgr,
+ virDomainDefPtr def)
+{
+ if (def->seclabel.model == VIR_DOMAIN_SECLABEL_MODEL_NONE)
+ return virSecurityDriverLookup("none");
+
+ return mgr->drv;
+}
+
const char *
virSecurityManagerGetDOI(virSecurityManagerPtr mgr)
{
@@ -151,8 +161,9 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
- if (mgr->drv->domainRestoreSecurityImageLabel)
- return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainRestoreSecurityImageLabel)
+ return drv->domainRestoreSecurityImageLabel(mgr, vm, disk);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -161,8 +172,9 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainSetSecuritySocketLabel)
- return mgr->drv->domainSetSecuritySocketLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSecuritySocketLabel)
+ return drv->domainSetSecuritySocketLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -171,8 +183,9 @@ int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr,
int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainClearSecuritySocketLabel)
- return mgr->drv->domainClearSecuritySocketLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainClearSecuritySocketLabel)
+ return drv->domainClearSecuritySocketLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -182,8 +195,9 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
- if (mgr->drv->domainSetSecurityImageLabel)
- return mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSecurityImageLabel)
+ return drv->domainSetSecurityImageLabel(mgr, vm, disk);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -193,8 +207,9 @@ int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
virDomainHostdevDefPtr dev)
{
- if (mgr->drv->domainRestoreSecurityHostdevLabel)
- return mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainRestoreSecurityHostdevLabel)
+ return drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -204,8 +219,9 @@ int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
virDomainHostdevDefPtr dev)
{
- if (mgr->drv->domainSetSecurityHostdevLabel)
- return mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSecurityHostdevLabel)
+ return drv->domainSetSecurityHostdevLabel(mgr, vm, dev);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -215,8 +231,9 @@ int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
const char *savefile)
{
- if (mgr->drv->domainSetSavedStateLabel)
- return mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSavedStateLabel)
+ return drv->domainSetSavedStateLabel(mgr, vm, savefile);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -226,8 +243,9 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr
mgr,
virDomainObjPtr vm,
const char *savefile)
{
- if (mgr->drv->domainRestoreSavedStateLabel)
- return mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainRestoreSavedStateLabel)
+ return drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -236,8 +254,9 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr
mgr,
int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainGenSecurityLabel)
- return mgr->drv->domainGenSecurityLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainGenSecurityLabel)
+ return drv->domainGenSecurityLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -246,8 +265,9 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainReserveSecurityLabel)
- return mgr->drv->domainReserveSecurityLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainReserveSecurityLabel)
+ return drv->domainReserveSecurityLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -256,8 +276,9 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr,
int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainReleaseSecurityLabel)
- return mgr->drv->domainReleaseSecurityLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainReleaseSecurityLabel)
+ return drv->domainReleaseSecurityLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -267,8 +288,9 @@ int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
const char *stdin_path)
{
- if (mgr->drv->domainSetSecurityAllLabel)
- return mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSecurityAllLabel)
+ return drv->domainSetSecurityAllLabel(mgr, vm, stdin_path);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -278,8 +300,9 @@ int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
int migrated)
{
- if (mgr->drv->domainRestoreSecurityAllLabel)
- return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainRestoreSecurityAllLabel)
+ return drv->domainRestoreSecurityAllLabel(mgr, vm, migrated);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -289,8 +312,9 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm,
virSecurityLabelPtr sec)
{
- if (mgr->drv->domainGetSecurityProcessLabel)
- return mgr->drv->domainGetSecurityProcessLabel(mgr, vm, sec);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainGetSecurityProcessLabel)
+ return drv->domainGetSecurityProcessLabel(mgr, vm, sec);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -299,8 +323,9 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr,
virDomainObjPtr vm)
{
- if (mgr->drv->domainSetSecurityProcessLabel)
- return mgr->drv->domainSetSecurityProcessLabel(mgr, vm);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, vm->def);
+ if (drv->domainSetSecurityProcessLabel)
+ return drv->domainSetSecurityProcessLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
@@ -309,8 +334,9 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr,
int virSecurityManagerVerify(virSecurityManagerPtr mgr,
virDomainDefPtr def)
{
- if (mgr->drv->domainSecurityVerify)
- return mgr->drv->domainSecurityVerify(mgr, def);
+ virSecurityDriverPtr drv = virSecurityManagerGetDriver(mgr, def);
+ if (drv->domainSecurityVerify)
+ return drv->domainSecurityVerify(mgr, def);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
new file mode 100644
index 0000000..2b3d40b
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
@@ -0,0 +1,21 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='dynamic' model='none'/>
+</domain>
diff --git a/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
new file mode 100644
index 0000000..2b3d40b
--- /dev/null
+++ b/tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
@@ -0,0 +1,21 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='dynamic' model='none'/>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2af7494..8c08ee6 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -200,6 +200,7 @@ mymain(int argc, char **argv)
input_folder_fmt = (char *) XML2XMLIN_FMT;
DO_TEST_DIFFERENT("seclabel-dynamic");
DO_TEST_DIFFERENT("seclabel-static");
+ DO_TEST_DIFFERENT("seclabel-model-none");
virCapabilitiesFree(driver.caps);
--
1.7.3.2
3:43 p.m.
New subject: [libvirt] [PATCH 7/7] security: Allow disabling security on a per VM basis
On 01/12/2011 10:23 AM, Cole Robinson wrote:
Make the SecurityManager explicitly handle the case when seclabel
model='none'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_manager.c | 90 +++++++++++++-------
.../qemuxml2xml-seclabel-model-none-in.xml | 21 +++++
.../qemuxml2xml-seclabel-model-none-out.xml | 21 +++++
tests/qemuxml2xmltest.c | 1 +
4 files changed, 101 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
I agree with Daniel's NACK to this patch - when security is enabled
globally, allowing just one rogue domain can invalidate all others. And
when security is not enabled, <seclabel> is an illusion not aided by an
XML marking.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:33 a.m.
New subject: [libvirt] [PATCH 7/7] security: Allow disabling security on a per VM basis
On Wed, Jan 12, 2011 at 12:23:03PM -0500, Cole Robinson wrote:
Make the SecurityManager explicitly handle the case when seclabel
model='none'.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/security/security_manager.c | 90 +++++++++++++-------
.../qemuxml2xml-seclabel-model-none-in.xml | 21 +++++
.../qemuxml2xml-seclabel-model-none-out.xml | 21 +++++
tests/qemuxml2xmltest.c | 1 +
4 files changed, 101 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-in.xml
create mode 100644 tests/qemuxml2xmldata/qemuxml2xml-seclabel-model-none-out.xml
As previously discussed, we should't take this patch
Daniel
12:10 p.m.
On Wed, Jan 12, 2011 at 12:22:56PM -0500, Cole Robinson wrote:
Enabling a security driver in qemu.conf is currently all or nothing.
The option to disable security on a per VM basis can be a useful debugging
tool or work around for frustrated users.
Patches 1-3 and 5-6 are prep and cleanup work. Patch 4 fixes an
easily triggerable segfault when defining a domain in qemu. Patch 7
is the actual feature.
Hmm, I can understand the motivation for wanting to allow
users to disable security per VM. From the POV of a security
person it is a bad idea to allow this capability to be used
by default since running one single unconfined VM compromises
the entire security model. As a host admin, we need to be
able to enforce that every single VM launched is always running
with security model active, and not allow libvirt admins to
override that decision at all.
Thus a default libvirt install would have to forbid any attempt
to run a VM with a secmodel=none, and reject with an error.
It would have to require a host level configuration change to
allow running VMs without a secmodel. Unfortunately once you
require this you might as well just be changing the existing
config param in qemu.conf for libvirtd as a whole.
If a user is having trouble, and needs to debug then I think
it is best to just 'setenforce 0' and do the debugging.
NB, some of your patches in this series are useful regardless,
but I don't think we should allow a tunable to turn off
security per VM.
Regards,
Daniel
5062
days inactive
5063
days old
23 comments
3 participants
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake