On Mon, Aug 19, 2024 at 4:13 PM Alexander Graf <g...@amazon.com> wrote:
>
> Hey Dorjoy,
>
> On 18.08.24 13:42, 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.
> >
> > Some optional nitro-enclave machine options have been added:
> > - 'id': Enclave identifier, reflected in the module-id of the NSM
> > device. If not provided, a default id will be set.
> > - 'parent-role': Parent instance IAM role ARN, reflected in PCR3
> > of the NSM device.
> > - 'parent-id': Parent instance identifier, reflected in PCR4 of the
> > NSM device.
> >
> > Signed-off-by: Dorjoy Chowdhury <dorjoychy...@gmail.com>
> > ---
> > crypto/meson.build | 2 +-
> > crypto/x509-utils.c | 73 +++++++++++
>
>
> Can you please put this new API into its own patch file?
>
>
> > hw/core/eif.c | 225 +++++++++++++++++++++++++++++---
> > hw/core/eif.h | 5 +-
>
>
> These changes to eif.c should ideally already be part of the patch that
> introduces eif.c (patch 1), no? In fact, do you think you can make the
> whole eif logic its own patch file?
>
>
> > hw/core/meson.build | 4 +-
> > hw/i386/Kconfig | 1 +
> > hw/i386/nitro_enclave.c | 141 +++++++++++++++++++-
> > include/crypto/x509-utils.h | 22 ++++
> > include/hw/i386/nitro_enclave.h | 26 ++++
> > 9 files changed, 479 insertions(+), 20 deletions(-)
> > create mode 100644 crypto/x509-utils.c
> > create mode 100644 include/crypto/x509-utils.h
> >
> > diff --git a/crypto/meson.build b/crypto/meson.build
> > index c46f9c22a7..09633194ed 100644
> > --- a/crypto/meson.build
> > +++ b/crypto/meson.build
> > @@ -62,7 +62,7 @@ endif
> > if gcrypt.found()
> > util_ss.add(gcrypt, files('random-gcrypt.c'))
> > elif gnutls.found()
> > - util_ss.add(gnutls, files('random-gnutls.c'))
> > + util_ss.add(gnutls, files('random-gnutls.c', 'x509-utils.c'))
>
>
> What if we don't have gnutls. Will everything still compile or do we
> need to add any dependencies?
>
>
[...]
> >
> > diff --git a/hw/core/meson.build b/hw/core/meson.build
> > index f32d1ad943..8dc4552e35 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.7.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])
>
>
> Ah, you add the gnutls dependency here. Great! However, this means we
> now make gnutls (and libcbor) a mandatory dependency for the default
> configuration. Does configure know about that? I believe before gnutls
> was optional, right?
>
I see gnutls is not a required dependency in the root meson.build. I
am not sure what we should do here.
Hey Daniel, do you have any suggestions about how this dependency
should be included?
Regards,
Dorjoy
