On Mon, Aug 19, 2024 at 9:58 PM Alexander Graf <g...@amazon.com> wrote: > > > On 19.08.24 17:28, Dorjoy Chowdhury wrote: > > Hey Alex, > > > > 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? > >> > > 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 >
Sounds good to me. Thanks Alex! Regards, Dorjoy
