On 06/24/2013 08:42 AM, yangdongsheng wrote: We already have a 'STREQ' macro defined for that in internal.h: http://libvirt.org/hacking.html#string_comparision Jan

Before this patch, if relabel is yes or sec_type is dynamic in VM, after VM stopped, resources this VM accessed will be restored to default label on their path, but not the label before VM started. This patch instroduce a struct named _virSecuritySELinuxBackupContext to save the path and the label before libvirt changing them. And labels will be restored to path in VM being stopped. TODO: There is only restoring for imageFile in this patch, files of other types will be implemented later. Signed-off-by: yangdongsheng <yangds.fnst(a)cn.fujitsu.com&gt; --- src/security/security_selinux.c | 229 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 222 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 6fe063e..a780569 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -57,12 +57,16 @@ typedef virSecuritySELinuxData *virSecuritySELinuxDataPtr; typedef struct _virSecuritySELinuxCallbackData virSecuritySELinuxCallbackData; typedef virSecuritySELinuxCallbackData *virSecuritySELinuxCallbackDataPtr; +typedef struct _virSecuritySELinuxBackupContext virSecuritySELinuxBackupContext; +typedef virSecuritySELinuxBackupContext *virSecuritySELinuxBackupContextPtr; + struct _virSecuritySELinuxData { char *domain_context; char *alt_domain_context; char *file_context; char *content_context; virHashTablePtr mcs; + virSecuritySELinuxBackupContextPtr backup_header; bool skipAllLabel; #if HAVE_SELINUX_LABEL_H struct selabel_handle *label_handle; @@ -74,6 +78,21 @@ struct _virSecuritySELinuxCallbackData { virSecurityLabelDefPtr secdef; }; +struct _virSecuritySELinuxBackupContext { + const char *path; + security_context_t *context; + virSecuritySELinuxBackupContextPtr next; +}; + +/* + *Operators on backup context for function virSecuritySELinuxBackupContextHelper. + */ +enum BCONOPERATOR { + ADD = 0, + GET = 1, + DEL = 2 +}; + #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" @@ -992,8 +1011,8 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) /* Set fcon to the appropriate label for path and mode, or return -1. */ static int -getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - const char *newpath, mode_t mode, security_context_t *fcon) +getDefaultContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); @@ -1004,6 +1023,146 @@ getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, #endif } +/*Get the current label on path.*/ +static int +getContext(const char *path, security_context_t *fcon) +{ + return getfilecon(path, fcon); +} + +/* + *Helper for backup context. + *If operator is ADD, helper will get the current label on path + *to build a new _virSecuritySELinuxBackupContext and add it into mgr. + *If operator is GET, helper will search the all backups in mgr + *for the path, if path matches, set fcon_ptr to the related context + *and return 0, if no path matches, set fcon_ptr to NULL and return -1. + *If operator is DEL, helper will delete the backup in mgr whose + *path is equal to path. If no path matches, return 0. + */ +static int +virSecuritySELinuxBackupContextHelper(virSecurityManagerPtr mgr, + const char *path, + security_context_t **fcon_ptr ATTRIBUTE_UNUSED, + enum BCONOPERATOR operator) +{ + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecuritySELinuxBackupContextPtr cur_backup = data->backup_header; + virSecuritySELinuxBackupContextPtr pre_backup = cur_backup; + virSecuritySELinuxBackupContextPtr new_backup_ptr = NULL; + security_context_t *new_backup_fcon_ptr = NULL; + int rc = -1; + + switch (operator) { + case ADD: + //Init a new virSecuritySELinuxBackupContext. + if ((VIR_ALLOC(new_backup_fcon_ptr) < 0)||(VIR_ALLOC(new_backup_ptr)<0)) { + virReportOOMError(); + goto cleanup; + } + new_backup_ptr->next = NULL; + new_backup_ptr->path = path; + + //Set the context of new backup to current label on path. + if (getContext(path, new_backup_fcon_ptr) < 0){ + goto cleanup; + } + else { + new_backup_ptr->context = new_backup_fcon_ptr; + } + //Append the new backup into mgr. + if (cur_backup) { + while(cur_backup->next) { + cur_backup = cur_backup->next; + } + cur_backup->next = new_backup_ptr; + } + else { + //Backup header does not exists. + data->backup_header = new_backup_ptr; + } + rc = 0; + break; + case GET: + case DEL: + //Search the backup whose path is equal to path. + if (cur_backup) { + do + { + if (virStrcmp(cur_backup->path, path) == 0) { + if (operator == GET) { + (*fcon_ptr) = cur_backup->context; + rc = 0; + goto cleanup; + }else { + //operator is DEL. + if (pre_backup == cur_backup) { + //There is only one backup in mgr. + data->backup_header = NULL; + } + else { + //Remove cur_backup from backup list. + pre_backup->next = cur_backup->next; + } + rc = 0; + goto cleanup; + } + } + //Step to next. + pre_backup = cur_backup; + cur_backup = cur_backup->next; + } + while(cur_backup); + } + VIR_INFO("No backup found for %s", path); + rc = -1; + break; + default: + VIR_DEBUG("Operator %d is not supported.", operator); + break; + } +cleanup: + if (rc == 0) { + if (operator == DEL) { + //Free the backup removed from list. + freecon(*cur_backup->context); + VIR_FREE(cur_backup); + } + } + else { + //Free the pointers if function failed when operator is ADD. + if (new_backup_ptr) + VIR_FREE(new_backup_ptr); + if (new_backup_fcon_ptr) + freecon(*new_backup_fcon_ptr); + } + return rc; +} + +static int +virSecuritySELinuxAddBackupContext(virSecurityManagerPtr mgr, + const char *path) +{ + enum BCONOPERATOR operator = ADD; + return virSecuritySELinuxBackupContextHelper(mgr, path, NULL, operator); +} + +static int +virSecuritySELinuxDelBackupContext(virSecurityManagerPtr mgr, + const char *path) +{ + enum BCONOPERATOR operator = DEL; + return virSecuritySELinuxBackupContextHelper(mgr, path, NULL, operator); +} + +static int +virSecuritySELinuxGetBackupContext(virSecurityManagerPtr mgr, + const char *path, + security_context_t **fcon) +{ + enum BCONOPERATOR operator = GET; + return virSecuritySELinuxBackupContextHelper(mgr, path, fcon, operator); +} /* This method shouldn't raise errors, since they'll overwrite * errors that the caller(s) are already dealing with */ @@ -1013,25 +1172,37 @@ virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr, { struct stat buf; security_context_t fcon = NULL; + security_context_t *backup_con_ptr = NULL; int rc = -1; char *newpath = NULL; char ebuf[1024]; VIR_INFO("Restoring SELinux context on '%s'", path); + if ((virSecuritySELinuxGetBackupContext(mgr, path, &backup_con_ptr)) == 0) + { + VIR_INFO("Get a backup context. path: %s, context: %s.", + path, (char*)(*backup_con_ptr)); + rc = virSecuritySELinuxSetFilecon(path, (*backup_con_ptr)); + if (virSecuritySELinuxDelBackupContext(mgr, path) < 0) { + VIR_DEBUG("DEL backup of %s failed.", path); + } + goto cleanup; + } + if (virFileResolveLink(path, &newpath) < 0) { VIR_WARN("cannot resolve symlink %s: %s", path, virStrerror(errno, ebuf, sizeof(ebuf))); - goto err; + goto cleanup; } if (stat(newpath, &buf) != 0) { VIR_WARN("cannot stat %s: %s", newpath, virStrerror(errno, ebuf, sizeof(ebuf))); - goto err; + goto cleanup; } - if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) { + if (getDefaultContext(mgr, newpath, buf.st_mode, &fcon) < 0) { /* Any user created path likely does not have a default label, * which makes this an expected non error */ @@ -1041,7 +1212,7 @@ virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr, rc = virSecuritySELinuxSetFilecon(newpath, fcon); } -err: +cleanup: freecon(fcon); VIR_FREE(newpath); return rc; @@ -1186,6 +1357,23 @@ virSecuritySELinuxRestoreSecurityImageLabel(virSecurityManagerPtr mgr, static int +virSecuritySELinuxSaveSecurityFileLabel(virDomainDiskDefPtr disk, + const char *path, + size_t depth ATTRIBUTE_UNUSED, + void *opaque) +{ + virSecurityDeviceLabelDefPtr disk_seclabel; + virSecuritySELinuxCallbackDataPtr cbdata = opaque; + + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_SELINUX_NAME); + if (disk_seclabel && disk_seclabel->norelabel) + return 0; + + return virSecuritySELinuxAddBackupContext(cbdata->manager, path); +} + +static int virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, const char *path, size_t depth, @@ -1238,6 +1426,30 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, } static int +virSecuritySELinuxSaveSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk) +{ + virSecuritySELinuxCallbackData cbdata; + cbdata.manager = mgr; + cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + + if (cbdata.secdef == NULL) + return -1; + + if (cbdata.secdef->norelabel) + return 0; + + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + return 0; + + return virDomainDiskDefForeachPath(disk, + true, + virSecuritySELinuxSaveSecurityFileLabel, + &cbdata); +} + +static int virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk) @@ -2264,6 +2476,9 @@ virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, def->disks[i]->src, def->disks[i]->dst); continue; } + if (virSecuritySELinuxSaveSecurityImageLabel(mgr, + def, def->disks[i]) < 0) + return -1; if (virSecuritySELinuxSetSecurityImageLabel(mgr, def, def->disks[i]) < 0) return -1; @@ -2363,7 +2578,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr, goto cleanup; } - if (getContext(mgr, "/dev/tap.*", buf.st_mode, &fcon) < 0) { + if (getDefaultContext(mgr, "/dev/tap.*", buf.st_mode, &fcon) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot lookup default selinux label for tap fd %d"), fd); goto cleanup; -- 1.7.10.1

On 06/24/2013 05:50 PM, Daniel P. Berrange wrote: Got it. Thanx :)