Re: [libvirt] [PATCH v3 07/11] Add SELinux and DAC labeling support for TPM passthrough

On 04/01/2013 05:06 PM, Corey Bryant wrote: > > > On 03/21/2013 11:42 AM, Stefan Berger wrote: >> Signed-off-by: Stefan Berger&lt;stefanb(a)linux.vnet.ibm.com&gt; >> >> --- >> src/security/security_dac.c | 53 ++++++++++++++++++++++ >> src/security/security_selinux.c | 96 >> ++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 149 insertions(+) >> >> Index: libvirt/src/security/security_selinux.c >> =================================================================== >> --- libvirt.orig/src/security/security_selinux.c >> +++ libvirt/src/security/security_selinux.c >> @@ -45,6 +45,7 @@ >> #include "virrandom.h" >> #include "virutil.h" >> #include "virconf.h" >> +#include "virtpm.h" >> >> #define VIR_FROM_THIS VIR_FROM_SECURITY >> >> @@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData { >> #define SECURITY_SELINUX_VOID_DOI "0" >> #define SECURITY_SELINUX_NAME "selinux" >> >> +static int >> +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr >> mgr, >> + virDomainDefPtr def, >> + virDomainTPMDefPtr tpm); >> + >> + >> /* >> * Returns 0 on success, 1 if already reserved, or -1 on fatal error >> */ >> @@ -1062,6 +1069,84 @@ err: >> return rc; >> } >> >> + >> +static int >> +virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, >> + virDomainDefPtr def, >> + virDomainTPMDefPtr tpm) >> +{ >> + int rc; >> + virSecurityLabelDefPtr seclabel; >> + char *cancel_path; >> + >> + seclabel = virDomainDefGetSecurityLabelDef(def, >> SECURITY_SELINUX_NAME); >> + if (seclabel == NULL) >> + return -1; >> + >> + switch (tpm->type) { >> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: >> + rc = virSecuritySELinuxSetFilecon( >> + tpm->data.passthrough.source.data.file.path, >> + seclabel->imagelabel); >> + if (rc < 0) >> + return -1; >> + >> + if ((cancel_path = virTPMFindCancelPath()) != NULL) { >> + rc = virSecuritySELinuxSetFilecon(cancel_path, >> + seclabel->imagelabel); >> + VIR_FREE(cancel_path); >> + if (rc < 0) { >> + virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, >> + tpm); >> + return -1; >> + } >> + } else { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("Cannot determine TPM command cancel >> path")); >> + return -1; > > This makes me wonder if cancel-path should be specifiable at the > libvirt level rather than just using the default sysfs entry. If I've > read the code correctly I don't think it can currently be specified. > However QEMU is capable of taking a cancel-path string in case it is > different from the default sysfs path. > > I am not sure whether to allow users to specify the path and misconfigure it and to have QEMU write a letter into the wrong file. I wonder whether we could have libvirt determine the path and display it in the XML as read-only, though. Stefan