Re: [libvirt] [PATCH v4 09/11] security: Label the external swtpm with SELinux labels

On 05/15/2018 11:45 AM, Stefan Berger wrote: > On 05/15/2018 11:38 AM, Marc Hartmayer wrote: >> On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger >> <stefanb(a)linux.vnet.ibm.com&gt; wrote: >>> In this patch we label the swtpm process with SELinux labels. We >>> give it the >>> same label as the QEMU process has. We label its state directory and >>> files >>> as well. We restore the old security labels once the swtpm has >>> terminated. >>> >>> The file and process labels now look as follows: >>> >>> Directory: /var/lib/libvirt/swtpm >>> >>> [root@localhost swtpm]# ls -lZ >>> total 4 >>> rwx------. 2 tss  tss system_u:object_r:svirt_image_t:s0:c254,c932 >>> 4096 Apr  5 16:46 testvm >>> >>> [root@localhost testvm]# ls -lZ >>> total 8 >>> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 >>> 3648 Apr  5 16:46 tpm-00.permall >>> >>> The log in /var/log/swtpm/libvirt/qemu is labeled as follows: >>> >>> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 >>> 2237 Apr  5 16:46 vtpm.log >>> >>> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | >>> grep swtpm | grep ctrl | grep -v grep >>> system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0  0.0 28172 >>> 3892 ?        Ss   16:57   0:00 /usr/bin/swtpm socket --daemon >>> --ctrl >>> type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 >>> --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log >>> file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log >>> >>> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | >>> grep qemu | grep tpm | grep -v grep >>> system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0  0.0 3096704 >>> 48500 ?    Sl   16:57   3:28 /bin/qemu-system-x86_64 [..] >>> >>> Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com&gt; >>> --- >>>   src/libvirt_private.syms        |   2 + >>>   src/qemu/qemu_tpm.c             |  24 +++++- >>>   src/security/security_driver.h  |   7 ++ >>>   src/security/security_manager.c |  36 +++++++++ >>>   src/security/security_manager.h |   6 ++ >>>   src/security/security_selinux.c | 164 >>> ++++++++++++++++++++++++++++++++++++++++ >>>   src/security/security_stack.c   |  40 ++++++++++ >>>   7 files changed, 278 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >>> index 75b8932..2ce67e7 100644 >>> --- a/src/libvirt_private.syms >>> +++ b/src/libvirt_private.syms >>> @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; >>>   virSecurityManagerRestoreInputLabel; >>>   virSecurityManagerRestoreMemoryLabel; >>>   virSecurityManagerRestoreSavedStateLabel; >>> +virSecurityManagerRestoreTPMLabels; >>>   virSecurityManagerSetAllLabel; >>>   virSecurityManagerSetChardevLabel; >>>   virSecurityManagerSetChildProcessLabel; >>> @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; >>>   virSecurityManagerSetSavedStateLabel; >>>   virSecurityManagerSetSocketLabel; >>>   virSecurityManagerSetTapFDLabel; >>> +virSecurityManagerSetTPMLabels; >>>   virSecurityManagerStackAddNested; >>>   virSecurityManagerTransactionAbort; >>>   virSecurityManagerTransactionCommit; >>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c >>> index 024d24d..62f0146 100644 >>> --- a/src/qemu/qemu_tpm.c >>> +++ b/src/qemu/qemu_tpm.c >>> @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, >>> >>>       virCommandSetErrorBuffer(cmd, &errbuf); >>> >>> -    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { >>> +    if (virSecurityManagerSetTPMLabels(driver->securityManager, >>> +                                       def) < 0) >>> +        goto cleanup; >>> + >>> +    if >>> (virSecurityManagerSetChildProcessLabel(driver->securityManager, >>> +                                               def, cmd) < 0) >>> +        goto cleanup; >>> + >>> +    if (virSecurityManagerPreFork(driver->securityManager) < 0) >>> +        goto cleanup; >>> + >>> +    /* make sure we run this with the appropriate user */ >>> +    virCommandSetUID(cmd, cfg->swtpm_user); >>> +    virCommandSetGID(cmd, cfg->swtpm_group); >>> + >>> +    ret = virCommandRun(cmd, &exitstatus); >>> + >>> +    virSecurityManagerPostFork(driver->securityManager); >>> + >>> +    if (ret < 0 || exitstatus != 0) { >>>           VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " >>>                       "stderr: %s"), exitstatus, errbuf); >>>           virReportError(VIR_ERR_INTERNAL_ERROR, >> Don't we have to set ret to -1 here (for the case where ret == 0 && >> exitstatus != 0)? > > Very true. ret is initialized with '-1'. So we are good.

   Stefan