Re: [PATCH v3 4/5] machine/nitro-enclave: Add built-in Nitro Secure Module device

[Daniel P . Berrangé]

On Mon, Aug 12, 2024 at 03:51:22PM +0200, Alexander Graf wrote:
> 
> On 10.08.24 18:45, Dorjoy Chowdhury wrote:
> > AWS Nitro Enclaves have built-in Nitro Secure Module (NSM) device which
> > is used for stripped down TPM functionality like attestation. This commit
> > adds the built-in NSM device in the nitro-enclave machine type.
> > 
> > In Nitro Enclaves, all the PCRs start in a known zero state and the first
> > 16 PCRs are locked from boot and reserved. The PCR0, PCR1, PCR2 and PCR8
> > contain the SHA384 hashes related to the EIF file used to boot the
> > VM for validation.
> > 
> > A new optional nitro-enclave machine option 'id' has been added which will
> > be the enclave identifier reflected in the module-id of the NSM device.
> > Otherwise, the device will have a default id set.
> > 
> > Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> > ---
> >   hw/core/eif.c                   | 205 +++++++++++++++++++++++++++++++-
> >   hw/core/eif.h                   |   5 +-
> >   hw/core/meson.build             |   4 +-
> >   hw/i386/Kconfig                 |   1 +
> >   hw/i386/nitro_enclave.c         |  85 ++++++++++++-
> >   include/hw/i386/nitro_enclave.h |  19 +++
> >   6 files changed, 310 insertions(+), 9 deletions(-)
> > 
> 
> [...]
> 
> 
> > diff --git a/hw/core/meson.build b/hw/core/meson.build
> > index f32d1ad943..7e7a14ee00 100644
> > --- a/hw/core/meson.build
> > +++ b/hw/core/meson.build
> > @@ -12,6 +12,8 @@ hwcore_ss.add(files(
> >     'qdev-clock.c',
> >   ))
> > 
> > +libcbor = dependency('libcbor', version: '>=0.8.0')
> > +
> >   common_ss.add(files('cpu-common.c'))
> >   common_ss.add(files('machine-smp.c'))
> >   system_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
> > @@ -24,7 +26,7 @@ system_ss.add(when: 'CONFIG_REGISTER', if_true: 
> > files('register.c'))
> >   system_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
> >   system_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
> >   system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
> > -system_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: [files('eif.c'), 
> > zlib])
> > +system_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: [files('eif.c'), 
> > zlib, libcbor, gnutls])
> 
> 
> I think this is missing a dependency check somewhere:
> 
> ../hw/core/eif.c:16:10: fatal error: gnutls/gnutls.h: No such file or
> directory
>    16 | #include <gnutls/gnutls.h>
>       |          ^~~~~~~~~~~~~~~~~
> 
> It's also the first time anything accesses gnutls directly instead of
> through the QEMU crypto framework. Is there a particular reason you can not
> use qcrypto?

None of the existing qcrypto APIs provide the functionality needed in
this patch. The right answer though is to introduce a new API in
qcrypto to avoid directly coupling this Nitro code to GNUTLS.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|