[libvirt] [PATCH 02/12] Refactor setup & cleanup of security labels in security driver
Daniel P. Berrange
berrange at redhat.com
Wed Jan 20 15:14:59 UTC 2010
The current security driver architecture has the following
split of logic
* domainGenSecurityLabel
Allocate the unique label for the domain about to be started
* domainGetSecurityLabel
Retrieve the current live security label for a process
* domainSetSecurityLabel
Apply the previously allocated label to the current process
Setup all disk image / device labelling
* domainRestoreSecurityLabel
Restore the original disk image / device labelling.
Release the unique label for the domain
The 'domainSetSecurityLabel' method is special because it runs
in the context of the child process between the fork + exec.
This is require in order to set the process label. It is not
required in order to label disks/devices though. Having the
disk labelling code run in the child process limits what it
can do.
In particularly libvirtd would like to remember the current
disk image label, and only change shared image labels for the
first VM to start. This requires use & update of global state
in the libvirtd daemon, and thus cannot run in the child
process context.
The solution is to split domainSetSecurityLabel into two parts,
one applies process label, and the other handles disk image
labelling. At the same time domainRestoreSecurityLabel is
similarly split, just so that it matches the style. Thus the
previous 4 methods are replaced by the following 6 new methods
* domainGenSecurityLabel
Allocate the unique label for the domain about to be started
No actual change here.
* domainReleaseSecurityLabel
Release the unique label for the domain
* domainGetSecurityProcessLabel
Retrieve the current live security label for a process
Merely renamed for clarity.
* domainSetSecurityProcessLabel
Apply the previously allocated label to the current process
* domainRestoreSecurityAllLabel
Restore the original disk image / device labelling.
* domainSetSecurityAllLabel
Setup all disk image / device labelling
The SELinux and AppArmour drivers are then updated to comply with
this new spec. Notice that the AppArmour driver was actually a
little different. It was creating its profile for the disk image
and device labels in the 'domainGenSecurityLabel' method, where as
the SELinux driver did it in 'domainSetSecurityLabel'. With the
new method split, we can have consistency, with both drivers doing
that in the domainSetSecurityAllLabel method.
NB, the AppArmour changes here haven't been compiled so may not
build.
---
src/qemu/qemu_driver.c | 21 ++++++++---
src/security/security_apparmor.c | 66 ++++++++++++++++++++++-----------
src/security/security_driver.h | 30 +++++++++------
src/security/security_selinux.c | 75 +++++++++++++++++++++++++-------------
4 files changed, 127 insertions(+), 65 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 446f6ff..b67abf1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2443,8 +2443,14 @@ static int qemudDomainSetSecurityLabel(virConnectPtr conn, struct qemud_driver *
int rc = 0;
if (driver->securityDriver &&
- driver->securityDriver->domainSetSecurityLabel &&
- driver->securityDriver->domainSetSecurityLabel(conn, driver->securityDriver, vm) < 0)
+ driver->securityDriver->domainSetSecurityAllLabel &&
+ driver->securityDriver->domainSetSecurityAllLabel(conn, vm) < 0)
+ rc = -1;
+
+ if (rc == 0 &&
+ driver->securityDriver &&
+ driver->securityDriver->domainSetSecurityProcessLabel &&
+ driver->securityDriver->domainSetSecurityProcessLabel(conn, driver->securityDriver, vm) < 0)
rc = -1;
return rc;
@@ -3056,8 +3062,11 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
/* Reset Security Labels */
if (driver->securityDriver &&
- driver->securityDriver->domainRestoreSecurityLabel)
- driver->securityDriver->domainRestoreSecurityLabel(conn, vm);
+ driver->securityDriver->domainRestoreSecurityAllLabel)
+ driver->securityDriver->domainRestoreSecurityAllLabel(conn, vm);
+ if (driver->securityDriver &&
+ driver->securityDriver->domainReleaseSecurityLabel)
+ driver->securityDriver->domainReleaseSecurityLabel(conn, vm);
/* Clear out dynamically assigned labels */
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
@@ -4633,8 +4642,8 @@ static int qemudDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr sec
* QEMU monitor hasn't seen SIGHUP/ERR on poll().
*/
if (virDomainObjIsActive(vm)) {
- if (driver->securityDriver && driver->securityDriver->domainGetSecurityLabel) {
- if (driver->securityDriver->domainGetSecurityLabel(dom->conn, vm, seclabel) == -1) {
+ if (driver->securityDriver && driver->securityDriver->domainGetSecurityProcessLabel) {
+ if (driver->securityDriver->domainGetSecurityProcessLabel(dom->conn, vm, seclabel) == -1) {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Failed to get security label"));
goto cleanup;
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 0f9ff95..f288645 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -341,16 +341,6 @@ AppArmorGenSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
if ((profile_name = get_profile_name(conn, vm)) == NULL)
return rc;
- /* if the profile is not already loaded, then load one */
- if (profile_loaded(profile_name) < 0) {
- if (load_profile(conn, profile_name, vm, NULL) < 0) {
- virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("cannot generate AppArmor profile "
- "\'%s\'"), profile_name);
- goto clean;
- }
- }
-
vm->def->seclabel.label = strndup(profile_name, strlen(profile_name));
if (!vm->def->seclabel.label) {
virReportOOMError(NULL);
@@ -375,7 +365,6 @@ AppArmorGenSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
goto clean;
err:
- remove_profile(profile_name);
VIR_FREE(vm->def->seclabel.label);
VIR_FREE(vm->def->seclabel.imagelabel);
VIR_FREE(vm->def->seclabel.model);
@@ -386,12 +375,33 @@ AppArmorGenSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
return rc;
}
+static int
+AppArmorSetSecurityAllLabel(virConnectPtr conn, virDomainObjPtr vm)
+{
+ int rc = -1;
+
+ if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC)
+ return 0;
+
+ /* if the profile is not already loaded, then load one */
+ if (profile_loaded(vm->def->seclabel.label) < 0) {
+ if (load_profile(conn, vm->def->seclabel.label, vm, NULL) < 0) {
+ virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("cannot generate AppArmor profile "
+ "\'%s\'"), vm->def->seclabel.label);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
/* Seen with 'virsh dominfo <vm>'. This function only called if the VM is
* running.
*/
static int
-AppArmorGetSecurityLabel(virConnectPtr conn,
- virDomainObjPtr vm, virSecurityLabelPtr sec)
+AppArmorGetSecurityProcessLabel(virConnectPtr conn,
+ virDomainObjPtr vm, virSecurityLabelPtr sec)
{
int rc = -1;
char *profile_name = NULL;
@@ -423,7 +433,20 @@ AppArmorGetSecurityLabel(virConnectPtr conn,
* more details. Currently called via qemudShutdownVMDaemon.
*/
static int
-AppArmorRestoreSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
+AppArmorReleaseSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
+{
+ const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+
+ VIR_FREE(secdef->model);
+ VIR_FREE(secdef->label);
+ VIR_FREE(secdef->imagelabel);
+
+ return 0;
+}
+
+
+static int
+AppArmorRestoreSecurityAllLabel(virConnectPtr conn, virDomainObjPtr vm)
{
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
int rc = 0;
@@ -434,9 +457,6 @@ AppArmorRestoreSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
_("could not remove profile for \'%s\'"),
secdef->label);
}
- VIR_FREE(secdef->model);
- VIR_FREE(secdef->label);
- VIR_FREE(secdef->imagelabel);
}
return rc;
}
@@ -445,8 +465,8 @@ AppArmorRestoreSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
* LOCAL_STATE_DIR/log/libvirt/qemu/<vm name>.log
*/
static int
-AppArmorSetSecurityLabel(virConnectPtr conn,
- virSecurityDriverPtr drv, virDomainObjPtr vm)
+AppArmorSetSecurityProcessLabel(virConnectPtr conn,
+ virSecurityDriverPtr drv, virDomainObjPtr vm)
{
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
int rc = -1;
@@ -620,9 +640,11 @@ virSecurityDriver virAppArmorSecurityDriver = {
.domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel,
.domainGenSecurityLabel = AppArmorGenSecurityLabel,
.domainReserveSecurityLabel = AppArmorReserveSecurityLabel,
- .domainGetSecurityLabel = AppArmorGetSecurityLabel,
- .domainRestoreSecurityLabel = AppArmorRestoreSecurityLabel,
- .domainSetSecurityLabel = AppArmorSetSecurityLabel,
+ .domainReleaseSecurityLabel = AppArmorReleaseSecurityLabel,
+ .domainGetSecurityProcessLabel = AppArmorGetSecurityProcessLabel,
+ .domainSetSecurityProcessLabel = AppArmorSetSecurityProcessLabel,
+ .domainRestoreSecurityAllLabel = AppArmorRestoreSecurityAllLabel,
+ .domainSetSecurityAllLabel = AppArmorSetSecurityAllLabel,
.domainSetSecurityHostdevLabel = AppArmorSetSecurityHostdevLabel,
.domainRestoreSecurityHostdevLabel = AppArmorRestoreSecurityHostdevLabel,
};
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 97c9002..5d2446d 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -52,15 +52,19 @@ typedef int (*virSecurityDomainRestoreSavedStateLabel) (virConnectPtr conn,
typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn,
virDomainObjPtr sec);
typedef int (*virSecurityDomainReserveLabel) (virConnectPtr conn,
- virDomainObjPtr sec);
-typedef int (*virSecurityDomainGetLabel) (virConnectPtr conn,
- virDomainObjPtr vm,
- virSecurityLabelPtr sec);
-typedef int (*virSecurityDomainRestoreLabel) (virConnectPtr conn,
- virDomainObjPtr vm);
-typedef int (*virSecurityDomainSetLabel) (virConnectPtr conn,
- virSecurityDriverPtr drv,
- virDomainObjPtr vm);
+ virDomainObjPtr sec);
+typedef int (*virSecurityDomainReleaseLabel) (virConnectPtr conn,
+ virDomainObjPtr sec);
+typedef int (*virSecurityDomainSetAllLabel) (virConnectPtr conn,
+ virDomainObjPtr sec);
+typedef int (*virSecurityDomainRestoreAllLabel) (virConnectPtr conn,
+ virDomainObjPtr vm);
+typedef int (*virSecurityDomainGetProcessLabel) (virConnectPtr conn,
+ virDomainObjPtr vm,
+ virSecurityLabelPtr sec);
+typedef int (*virSecurityDomainSetProcessLabel) (virConnectPtr conn,
+ virSecurityDriverPtr drv,
+ virDomainObjPtr vm);
typedef int (*virSecurityDomainSecurityVerify) (virConnectPtr conn,
virDomainDefPtr def);
@@ -73,9 +77,11 @@ struct _virSecurityDriver {
virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
virSecurityDomainGenLabel domainGenSecurityLabel;
virSecurityDomainReserveLabel domainReserveSecurityLabel;
- virSecurityDomainGetLabel domainGetSecurityLabel;
- virSecurityDomainSetLabel domainSetSecurityLabel;
- virSecurityDomainRestoreLabel domainRestoreSecurityLabel;
+ virSecurityDomainReleaseLabel domainReleaseSecurityLabel;
+ virSecurityDomainGetProcessLabel domainGetSecurityProcessLabel;
+ virSecurityDomainSetProcessLabel domainSetSecurityProcessLabel;
+ virSecurityDomainSetAllLabel domainSetSecurityAllLabel;
+ virSecurityDomainRestoreAllLabel domainRestoreSecurityAllLabel;
virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel;
virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel;
virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c48f4c8..847a522 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -277,9 +277,9 @@ SELinuxSecurityDriverOpen(virConnectPtr conn, virSecurityDriverPtr drv)
}
static int
-SELinuxGetSecurityLabel(virConnectPtr conn,
- virDomainObjPtr vm,
- virSecurityLabelPtr sec)
+SELinuxGetSecurityProcessLabel(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virSecurityLabelPtr sec)
{
security_context_t ctx;
@@ -615,8 +615,8 @@ done:
}
static int
-SELinuxRestoreSecurityLabel(virConnectPtr conn,
- virDomainObjPtr vm)
+SELinuxRestoreSecurityAllLabel(virConnectPtr conn,
+ virDomainObjPtr vm)
{
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
int i;
@@ -636,6 +636,19 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn,
vm->def->disks[i]) < 0)
rc = -1;
}
+
+ return rc;
+}
+
+static int
+SELinuxReleaseSecurityLabel(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm)
+{
+ const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+
+ if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC)
+ return 0;
+
context_t con = context_new(secdef->label);
if (con) {
mcsRemove(context_range_get(con));
@@ -646,7 +659,7 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn,
VIR_FREE(secdef->label);
VIR_FREE(secdef->imagelabel);
- return rc;
+ return 0;
}
@@ -693,13 +706,12 @@ SELinuxSecurityVerify(virConnectPtr conn, virDomainDefPtr def)
}
static int
-SELinuxSetSecurityLabel(virConnectPtr conn,
- virSecurityDriverPtr drv,
- virDomainObjPtr vm)
+SELinuxSetSecurityProcessLabel(virConnectPtr conn,
+ virSecurityDriverPtr drv,
+ virDomainObjPtr vm)
{
/* TODO: verify DOI */
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
- int i;
if (vm->def->seclabel.label == NULL)
return 0;
@@ -722,18 +734,29 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
return -1;
}
- if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
- for (i = 0 ; i < vm->def->ndisks ; i++) {
- /* XXX fixme - we need to recursively label the entriy tree :-( */
- if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR)
- continue;
- if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0)
- return -1;
- }
- for (i = 0 ; i < vm->def->nhostdevs ; i++) {
- if (SELinuxSetSecurityHostdevLabel(conn, vm, vm->def->hostdevs[i]) < 0)
- return -1;
- }
+ return 0;
+}
+
+static int
+SELinuxSetSecurityAllLabel(virConnectPtr conn,
+ virDomainObjPtr vm)
+{
+ const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+ int i;
+
+ if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC)
+ return 0;
+
+ for (i = 0 ; i < vm->def->ndisks ; i++) {
+ /* XXX fixme - we need to recursively label the entriy tree :-( */
+ if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR)
+ continue;
+ if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0)
+ return -1;
+ }
+ for (i = 0 ; i < vm->def->nhostdevs ; i++) {
+ if (SELinuxSetSecurityHostdevLabel(conn, vm, vm->def->hostdevs[i]) < 0)
+ return -1;
}
return 0;
@@ -748,9 +771,11 @@ virSecurityDriver virSELinuxSecurityDriver = {
.domainRestoreSecurityImageLabel = SELinuxRestoreSecurityImageLabel,
.domainGenSecurityLabel = SELinuxGenSecurityLabel,
.domainReserveSecurityLabel = SELinuxReserveSecurityLabel,
- .domainGetSecurityLabel = SELinuxGetSecurityLabel,
- .domainRestoreSecurityLabel = SELinuxRestoreSecurityLabel,
- .domainSetSecurityLabel = SELinuxSetSecurityLabel,
+ .domainReleaseSecurityLabel = SELinuxReleaseSecurityLabel,
+ .domainGetSecurityProcessLabel = SELinuxGetSecurityProcessLabel,
+ .domainSetSecurityProcessLabel = SELinuxSetSecurityProcessLabel,
+ .domainRestoreSecurityAllLabel = SELinuxRestoreSecurityAllLabel,
+ .domainSetSecurityAllLabel = SELinuxSetSecurityAllLabel,
.domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel,
.domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel,
.domainSetSavedStateLabel = SELinuxSetSavedStateLabel,
--
1.6.5.2
More information about the libvir-list
mailing list