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

[Corey Bryant]

On 04/02/2013 07:15 AM, Stefan Berger wrote:
> 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<stefanb at linux.vnet.ibm.com>
>>>
>>> ---
>>>   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
>

After discussing with Stefan some more, I think just using the default 
path is enough.  I don't know why the sysfs path would not be the 
default anyway.  And as far as I know we've decided not to support fd 
passing for vTPM, at least at this point, so that is not a concern.

-- 
Regards,
Corey Bryant