10:50 a.m.
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> 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>
>> ---
>> 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