Re: [PATCH v5 7/8] machine/nitro-enclave: New machine type for AWS Nitro Enclaves

[Dorjoy Chowdhury <dorjoychy111@gmail.com>]

Hi Daniel,

On Wed, Aug 28, 2024 at 9:39 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Aug 22, 2024 at 09:08:48PM +0600, Dorjoy Chowdhury wrote:
> > AWS nitro enclaves[1] is an Amazon EC2[2] feature that allows creating
> > isolated execution environments, called enclaves, from Amazon EC2
> > instances which are used for processing highly sensitive data. Enclaves
> > have no persistent storage and no external networking. The enclave VMs
> > are based on the Firecracker microvm with a vhost-vsock device for
> > communication with the parent EC2 instance that spawned it and a Nitro
> > Secure Module (NSM) device for cryptographic attestation. The parent
> > instance VM always has CID 3 while the enclave VM gets a dynamic CID.
> >
> > An EIF (Enclave Image Format)[3] file is used to boot an AWS nitro enclave
> > virtual machine. This commit adds support for AWS nitro enclave emulation
> > using a new machine type option '-M nitro-enclave'. This new machine type
> > is based on the 'microvm' machine type, similar to how real nitro enclave
> > VMs are based on Firecracker microvm. For nitro-enclave to boot from an
> > EIF file, the kernel and ramdisk(s) are extracted into a temporary kernel
> > and a temporary initrd file which are then hooked into the regular x86
> > boot mechanism along with the extracted cmdline. The EIF file path should
> > be provided using the '-kernel' QEMU option.
> >
> > In QEMU, the vsock emulation for nitro enclave is added using vhost-user-
> > vsock as opposed to vhost-vsock. vhost-vsock doesn't support sibling VM
> > communication which is needed for nitro enclaves. So for the vsock
> > communication to CID 3 to work, another process that does the vsock
> > emulation in  userspace must be run, for example, vhost-device-vsock[4]
> > from rust-vmm, with necessary vsock communication support in another
> > guest VM with CID 3. Using vhost-user-vsock also enables the possibility
> > to implement some proxying support in the vhost-user-vsock daemon that
> > will forward all the packets to the host machine instead of CID 3 so
> > that users of nitro-enclave can run the necessary applications in their
> > host machine instead of running another whole VM with CID 3. The following
> > mandatory nitro-enclave machine option has been added related to the
> > vhost-user-vsock device.
> >   - 'vsock': The chardev id from the '-chardev' option for the
> > vhost-user-vsock device.
> >
> > AWS Nitro Enclaves have built-in Nitro Secure Module (NSM) device which
> > has been added using the virtio-nsm device added in a previous commit.
> > 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. The following optional nitro-enclave machine options
> > have been added related to the NSM device.
> >   - '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.
> >
> > [1] https://docs.aws.amazon.com/enclaves/latest/user/nitro-enclave.html
> > [2] https://aws.amazon.com/ec2/
> > [3] https://github.com/aws/aws-nitro-enclaves-image-format
> > [4] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-vsock
> >
> > Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> > ---
> >  MAINTAINERS                              |   9 +
> >  backends/hostmem-memfd.c                 |   2 -
> >  configs/devices/i386-softmmu/default.mak |   1 +
> >  hw/core/machine.c                        |  71 ++---
> >  hw/core/meson.build                      |   3 +
> >  hw/i386/Kconfig                          |   6 +
> >  hw/i386/meson.build                      |   3 +
> >  hw/i386/microvm.c                        |   6 +-
> >  hw/i386/nitro_enclave.c                  | 355 +++++++++++++++++++++++
> >  include/hw/boards.h                      |   2 +
> >  include/hw/i386/microvm.h                |   2 +
> >  include/hw/i386/nitro_enclave.h          |  62 ++++
> >  include/sysemu/hostmem.h                 |   2 +
> >  13 files changed, 488 insertions(+), 36 deletions(-)
> >  create mode 100644 hw/i386/nitro_enclave.c
> >  create mode 100644 include/hw/i386/nitro_enclave.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index da4f698137..aa7846107e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1877,6 +1877,15 @@ F: hw/i386/microvm.c
> >  F: include/hw/i386/microvm.h
> >  F: pc-bios/bios-microvm.bin
> >
> > +nitro-enclave
> > +M: Alexander Graf <graf@amazon.com>
> > +M: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> > +S: Maintained
> > +F: hw/core/eif.c
> > +F: hw/core/eif.h
>
> The eif.c/h files were added in the prevuous patch, so upto this line
> should be added in the previous patch.
>

Yeah, it makes sense to include it in the previous patch. Thanks!

> > +F: hw/i386/nitro_enclave.c
> > +F: include/hw/i386/nitro_enclave.h
>
> These two lines can remain in this patch
>
> >  Machine core
> >  M: Eduardo Habkost <eduardo@habkost.net>
> >  M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>
>
> > diff --git a/hw/core/meson.build b/hw/core/meson.build
> > index a3d9bab9f4..5437a94490 100644
> > --- a/hw/core/meson.build
> > +++ b/hw/core/meson.build
> > @@ -24,6 +24,9 @@ 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'))
> > +if libcbor.found() and gnutls.found()
> > +  system_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: [files('eif.c'), zlib, libcbor, gnutls])
> > +endif
> >
> >  system_ss.add(files(
> >    'cpu-sysemu.c',
>
>
> This change to meson.build should be in the previous patch, since
> that's the one that introduces eif.c.
>

'CONFIG_NITRO_ENCLAVE' is introduced in this patch, so the
meson.change above should stay in this patch, right?

Regards,
Dorjoy