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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sat, Aug 10, 2024 at 10:45:01PM +0600, 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/eif.c b/hw/core/eif.c
> index 5558879a96..d2c65668ef 100644
> --- a/hw/core/eif.c
> +++ b/hw/core/eif.c
> @@ -12,6 +12,9 @@
>  #include "qemu/bswap.h"
>  #include "qapi/error.h"
>  #include <zlib.h> /* for crc32 */
> +#include <cbor.h>
> +#include <gnutls/gnutls.h>
> +#include <gnutls/x509.h>
>  
>  #include "hw/core/eif.h"
>  

> @@ -269,6 +284,125 @@ static bool read_eif_ramdisk(FILE *eif, FILE *initrd, uint64_t size,
>      return false;
>  }
>  
> +static bool get_fingerprint_sha384_from_cert(uint8_t *cert, size_t size,
> +                                             uint8_t *sha384, Error **errp)
> +{
> +    gnutls_x509_crt_t crt;
> +    size_t hash_size = 48;
> +    gnutls_datum_t datum = {.data = cert, .size = size};
> +
> +    gnutls_global_init();
> +    gnutls_x509_crt_init(&crt);
> +
> +    if (gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM) != 0) {
> +        error_setg(errp, "Failed to import certificate");
> +        goto cleanup;
> +    }
> +
> +    if (gnutls_x509_crt_get_fingerprint(crt, GNUTLS_DIG_SHA384, sha384,
> +                                        &hash_size) != 0) {
> +        error_setg(errp, "Failed to compute SHA384 fingerprint");
> +        goto cleanup;
> +    }
> +
> +    return true;
> +
> + cleanup:
> +    gnutls_x509_crt_deinit(crt);
> +    gnutls_global_deinit();
> +    return false;
> +}

I'd suggest this go into  qcrypto/x509-utils.c & include/qcrypto/x509-utils.h,
as:

    int qcrypto_get_x509_cert_fingerprint(uint8_t *cert,
                                          size_t size,
					  QCryptoHashAlgorith hash,
					  Error **errp);

there's no need to be calling gnutls_global_init() / deinit() either.


> @@ -299,7 +433,9 @@ static long get_file_size(FILE *f, Error **errp)
>   */
>  bool read_eif_file(const char *eif_path, const char *machine_initrd,
>                     char **kernel_path, char **initrd_path, char **cmdline,
> -                   Error **errp)
> +                   uint8_t *image_sha384, uint8_t *bootstrap_sha384,
> +                   uint8_t *app_sha384, uint8_t *fingerprint_sha384,
> +                   bool *signature_found, Error **errp)
>  {
>      FILE *f = NULL;
>      FILE *machine_initrd_f = NULL;
> @@ -308,9 +444,33 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
>      uint32_t crc = 0;
>      EifHeader eif_header;
>      bool seen_sections[EIF_SECTION_MAX] = {false};
> -
> +    /* kernel + ramdisks + cmdline sha384 hash */
> +    GChecksum *image_hasher = NULL;
> +    /* kernel + boot ramdisk + cmdline sha384 hash */
> +    GChecksum *bootstrap_hasher = NULL;
> +    /* application ramdisk(s) hash */
> +    GChecksum *app_hasher = NULL;
> +    size_t digest_len;
> +
> +    *signature_found = false;
>      *kernel_path = *initrd_path = *cmdline = NULL;
>  
> +    image_hasher = g_checksum_new(G_CHECKSUM_SHA384);
> +    if (image_hasher == NULL) {
> +        error_setg(errp, "Failed to initialize sha384 hash for image");
> +        goto cleanup;
> +    }
> +    bootstrap_hasher = g_checksum_new(G_CHECKSUM_SHA384);
> +    if (bootstrap_hasher == NULL) {
> +        error_setg(errp, "Failed to initialize sha384 hash for bootstrap");
> +        goto cleanup;
> +    }
> +    app_hasher = g_checksum_new(G_CHECKSUM_SHA384);
> +    if (app_hasher == NULL) {
> +        error_setg(errp, "Failed to initialize sha384 hash for app");
> +        goto cleanup;
> +    }

Don't use GChecksum APIs please, use the qcrypto hash APIs instead,
as we need all code to be using the designated QEMU crypto backend.


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 :|