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

[Cole Robinson <crobinso@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>   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

- Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list