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

-- 
2.14.3

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

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.

John

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

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.

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
>

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

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

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

On 05/23/2018 02:03 PM, John Ferlan wrote:
>
> 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!

Thanks. Between v3 and v4 it looks like some hunks got lost related to 
the TPM 2 enablement. I readded them and added an error message in case 
TPM 2 is not supported by swtpm_setup/swptm. I am keeping your 
Reviewed-by's - no 'abuse' intended :-)

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


     Stefan


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

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