[libvirt] AppArmor support for TPM emulator; was:Re: [PATCH 00/12] Add support for TPM emulator

John Ferlan jferlan at redhat.com
Wed May 23 18:03:20 UTC 2018



On 05/23/2018 09:20 AM, Stefan Berger wrote:
> On 05/23/2018 08:07 AM, John Ferlan wrote:
>>
>> On 05/22/2018 04:44 PM, Stefan Berger wrote:
>>> This series of patches adds support for the TPM emulator backend that
>>> is available in QEMU and based on swtpm + libtpms. It allows to attach a
>>> TPM 1.2 or 2 to a QEMU VM. sVirt labels are used for labeling the swtpm
>>> process, its Unix socket, and log file with the same label that the
>>> QEMU process gets. Besides that swtpm is added to the emulator cgroup to
>>> restrict its CPU usage.
>>>
>>> The device XML can be changed from a TPM 1.2 to a TPM 2 and back to a
>>> TPM 1.2. The device state is not removed during those changes but only
>>> when the domain is undefined.
>>>
>>> The swtpm needs persistent storage to store its state. For that I am
>>> using the uuid of the VM as part of the path since the name of the VM
>>> can be changed. Logfiles, PID files, and socket names are based on the
>>> name of the VM, though.
>>>
>>>    Stefan
>>>
>>> v5->v6:
>>>    - Addressed John Ferlan's comments
>>>    - rebased on latest tip
>>>    - Added patch 12.
>>>
>>> v4->v5:
>>>    - Addressed John Ferlan's, Boris Fiuczysnki's and Marc Hartmayer's
>>> comments
>>>    - rebased on latest tip
>>>
>>> v3->v4:
>>>    - Addressed John Ferlan's comments
>>>    - Fixed bugs I found while testing
>>>    - rebased on latest tip
>>>
>>> Stefan Berger (12):
>>>    conf: Add support for external swtpm TPM emulator to domain XML
>>>    qemu: Extend QEMU capabilities with 'tpm-emulator'
>>>    util: Implement virFileChownFiles()
>>>    security: Add DAC and SELinux security for tpm-emulator
>>>    qemu: Extend qemu_conf with tpm-emulator support
>>>    qemu: Extend QEMU with external TPM support
>>>    qemu: Add support for external swtpm TPM emulator
>>>    tests: Add test cases for external swtpm TPM emulator
>>>    security: Label the external swtpm with SELinux labels
>>>    conf: Add support for choosing emulation of a TPM 2
>>>    qemu: Add swtpm to emulator cgroup
>>>    news: Update news with new TPM emulator feature
>>>
>>>   docs/formatdomain.html.in                          |  43 +
>>>   docs/news.xml                                      |   9 +
>>>   docs/schemas/domaincommon.rng                      |  17 +
>>>   libvirt.spec.in                                    |   2 +
>>>   src/conf/domain_audit.c                            |   2 +
>>>   src/conf/domain_conf.c                             |  53 +-
>>>   src/conf/domain_conf.h                             |  12 +
>>>   src/libvirt_private.syms                           |   3 +
>>>   src/qemu/Makefile.inc.am                           |  10 +
>>>   src/qemu/libvirtd_qemu.aug                         |   5 +
>>>   src/qemu/qemu.conf                                 |   8 +
>>>   src/qemu/qemu_capabilities.c                       |   5 +
>>>   src/qemu/qemu_capabilities.h                       |   1 +
>>>   src/qemu/qemu_cgroup.c                             |  36 +
>>>   src/qemu/qemu_cgroup.h                             |   2 +
>>>   src/qemu/qemu_command.c                            |  34 +-
>>>   src/qemu/qemu_conf.c                               |  43 +
>>>   src/qemu/qemu_conf.h                               |   6 +
>>>   src/qemu/qemu_domain.c                             |   3 +
>>>   src/qemu/qemu_extdevice.c                          | 180 ++++
>>>   src/qemu/qemu_extdevice.h                          |  59 ++
>>>   src/qemu/qemu_process.c                            |  16 +
>>>   src/qemu/qemu_security.c                           |  69 ++
>>>   src/qemu/qemu_security.h                           |  11 +
>>>   src/qemu/qemu_tpm.c                                | 946
>>> +++++++++++++++++++++
>>>   src/qemu/qemu_tpm.h                                |  56 ++
>>>   src/qemu/test_libvirtd_qemu.aug.in                 |   2 +
>>>   src/security/security_dac.c                        |   7 +
>>>   src/security/security_driver.h                     |   7 +
>>>   src/security/security_manager.c                    |  36 +
>>>   src/security/security_manager.h                    |   6 +
>>>   src/security/security_selinux.c                    | 172 ++++
>>>   src/security/security_stack.c                      |  40 +
>>>   src/util/virfile.c                                 |  55 ++
>>>   src/util/virfile.h                                 |   3 +
>>>   tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 +
>>>   .../tpm-emulator-tpm2.x86_64-latest.args           |  33 +
>>>   tests/qemuxml2argvdata/tpm-emulator-tpm2.xml       |  30 +
>>>   .../tpm-emulator.x86_64-latest.args                |  33 +
>>>   tests/qemuxml2argvdata/tpm-emulator.xml            |  30 +
>>>   tests/qemuxml2argvtest.c                           |  16 +-
>>>   tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml     |  34 +
>>>   tests/qemuxml2xmloutdata/tpm-emulator.xml          |  34 +
>>>   tests/qemuxml2xmltest.c                            |   1 +
>>>   48 files changed, 2165 insertions(+), 10 deletions(-)
>>>   create mode 100644 src/qemu/qemu_extdevice.c
>>>   create mode 100644 src/qemu/qemu_extdevice.h
>>>   create mode 100644 src/qemu/qemu_tpm.c
>>>   create mode 100644 src/qemu/qemu_tpm.h
>>>   create mode 100644
>>> tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
>>>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>>>   create mode 100644
>>> tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args
>>>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
>>>   create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>>>   create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
>>>
>> This all looks good to me - thanks for the news.xml adjustment. Barring
>> anyone else making a late/additional review - I will look to push the
>> series later on today.
> 
> Thanks.
> 
> As mentioned, I will need to follow up with AppArmor support. What I am
> currently experimenting with is a subprofile of the libvirt profile. The
> problem with it is it's 'suboptimal' in terms of 'unspecific' paths
> containing wild-cards so that this single sub profile can accommodate
> the paths of all domains. This profile is static and not dynamically
> generated.
> 

I see that Jano had some comments... I also was reminded today that you
still have libvirt.git commit access - so once you feel comfortable with
addressing those comments, I guess you have the capability to push and
won't need me for that!

I'll defer to those with AppArmor experience for the rest!

John

> diff --git a/examples/apparmor/usr.sbin.libvirtd
> b/examples/apparmor/usr.sbin.libvirtd
> index 3102cab382..dcab4feb6c 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -126,4 +126,15 @@
> 
>     /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
>    }
> +  /usr/bin/swtpm Cx -> usr_bin_swtpm,
> +  profile usr_bin_swtpm flags=(complain) {
> +    #include <abstractions/base>
> +
> +    /usr/bin/swtpm rm,
> +
> +    /run/libvirt/qemu/swtpm/*-swtpm.pid rw,
> +    /run/libvirt/qemu/swtpm/*-swtpm.sock w,
> +    /var/log/swtpm/libvirt/qemu/*-swtpm.log w,
> +
> /var/lib/libvirt/swtpm/[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*/{tpm1.2,tpm2}/{tpm,tpm2}-00.permall
> rw,
> +  }
>  }
> 
> 
> A better solution would be to extend the QEMU domain profile with these
> additional paths, which, as a side-effect, would give a QEMU instance
> access to these paths as well. Basically QEMU and swtpm would share that
> profile. To good thing is we can use specific paths (no wild cards) for
> the files that the swtpm needs to access.
> 
> Yet a stricter solution would be to dynamically create a profile
> specifically for the swtpm that contains only the necessary paths.
> Though I think the code in src/security/security_apparmor.c is not
> prepared for that and it may end up being a bigger undertaking.
> 
> Anyone who would be willing to give an opinion on the latter two cases ?
> I am cc'ing Christian Ehrhard who seems to have worked on AppArmor
> related code.
> 
>    Stefan
> 
>>
>> John
>>
> 




More information about the libvir-list mailing list