[libvirt] [PATCH 3/3] security: Don't remember labels for TPM

[Michal Privoznik]

On 10/10/19 10:15 PM, Cole Robinson wrote:
> On 10/1/19 11:00 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1755803
>>
>> The /dev/tpmN file can be opened only once, as implemented in
>> drivers/char/tpm/tpm-dev.c:tpm_open() from the kernel's tree. Any
>> other attempt to open the file fails. And since we're opening the
>> file ourselves and passing the FD to qemu we will not succeed
>> opening the file again when locking it for seclabel remembering.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>   src/security/security_dac.c     | 18 +++++++++---------
>>   src/security/security_selinux.c | 10 +++++-----
>>   2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
>> index 2733fa664f..347a7a5f63 100644
>> --- a/src/security/security_dac.c
>> +++ b/src/security/security_dac.c
>> @@ -1653,14 +1653,14 @@ 
>> virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
>>       switch (tpm->type) {
>>       case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> -        ret = virSecurityDACSetChardevLabel(mgr, def,
>> -                                            
>> &tpm->data.passthrough.source,
>> -                                            false);
>> +        ret = virSecurityDACSetChardevLabelHelper(mgr, def,
>> +                                                  
>> &tpm->data.passthrough.source,
>> +                                                  false, false);
>>           break;
>>       case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> -        ret = virSecurityDACSetChardevLabel(mgr, def,
>> -                                            &tpm->data.emulator.source,
>> -                                            false);
>> +        ret = virSecurityDACSetChardevLabelHelper(mgr, def,
>> +                                                  
>> &tpm->data.emulator.source,
>> +                                                  false, false);
>>           break;
>>       case VIR_DOMAIN_TPM_TYPE_LAST:
>>           break;
>> @@ -1679,9 +1679,9 @@ 
>> virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
>>       switch (tpm->type) {
>>       case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> -        ret = virSecurityDACRestoreChardevLabel(mgr, def,
>> -                                                
>> &tpm->data.passthrough.source,
>> -                                                false);
>> +        ret = virSecurityDACRestoreChardevLabelHelper(mgr, def,
>> +                                                      
>> &tpm->data.passthrough.source,
>> +                                                      false, false);
>>           break;
>>       case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>>           /* swtpm will have removed the Unix socket upon termination */
>> diff --git a/src/security/security_selinux.c 
>> b/src/security/security_selinux.c
>> index e3be724a2b..0486bdd6b6 100644
>> --- a/src/security/security_selinux.c
>> +++ b/src/security/security_selinux.c
>> @@ -1682,14 +1682,14 @@ 
>> virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr,
>>       switch (tpm->type) {
>>       case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>>           tpmdev = tpm->data.passthrough.source.data.file.path;
>> -        rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, 
>> seclabel->imagelabel, true);
>> +        rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, 
>> seclabel->imagelabel, false);
>>           if (rc < 0)
>>               return -1;
>>           if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) {
>>               rc = virSecuritySELinuxSetFilecon(mgr,
>>                                                 cancel_path,
>> -                                              seclabel->imagelabel, 
>> true);
>> +                                              seclabel->imagelabel, 
>> false);
>>               VIR_FREE(cancel_path);
>>               if (rc < 0) {
>>                   virSecuritySELinuxRestoreTPMFileLabelInt(mgr, def, 
>> tpm);
>> @@ -1701,7 +1701,7 @@ 
>> virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr,
>>           break;
>>       case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>>           tpmdev = tpm->data.emulator.source.data.nix.path;
>> -        rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, 
>> seclabel->imagelabel, true);
>> +        rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, 
>> seclabel->imagelabel, false);
>>           if (rc < 0)
>>               return -1;
>>           break;
> 
> Are the EMULATOR changes actually required? I think it just uses a unix 
> socket and doesn't touch the host kernel tpm subsystem

Well, not per se, because the socket is created on domain startup and 
then unlink()-ed on domain shutdown. That's why it doesn't make sense to 
try remember anything about the socket.

Michal