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

[Alexander Graf]

On 19.08.24 17:28, Dorjoy Chowdhury wrote:
> Hey Alex,
>
> On Mon, Aug 19, 2024 at 4:13 PM Alexander Graf <graf@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 <dorjoychy111@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?
> Good point. I guess it should be possible if I have the virtio-nsm
> device commit first and then add the machine/nitro-enclave commit with
> full support with the devices. That will of course make the
> machine/nitro-enclave commit larger. What do you think?


As long as nothing compiles the code, it can rely on not yet implemented 
functions. So it's perfectly legit to add all your code in individual 
commits and then at the end add the meson.build change that implements 
the config option. How about the order below?

* Crypto patch for SHA384
* Crypto patch for x509 fingerprint
* NSM device emulation (including libcbor check, introduces 
CONFIG_VIRTIO_NSM)
* EIF format parsing (not compiled yet)
* Nitro Enclaves machine (introduces CONFIG_NITRO_ENCLAVE)
* Nitro Enclaves docs


Alex




Amazon Web Services Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597