Re: [PATCH v4 3/6] device/virtio-nsm: Support for Nitro Secure Module device

[Alexander Graf]

On 18.08.24 13:42, Dorjoy Chowdhury wrote:
> Nitro Secure Module (NSM)[1] device is used in AWS Nitro Enclaves for
> stripped down TPM functionality like cryptographic attestation. The
> requests to and responses from NSM device are CBOR[2] encoded.
>
> This commit adds support for NSM device in QEMU. Although related to
> AWS Nitro Enclaves, the virito-nsm device is independent and can be
> used in other machine types as well. The libcbor[3] library has been
> used for the CBOR encoding and decoding functionalities.
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00387.html
> [2] http://cbor.io/
> [3] https://libcbor.readthedocs.io/en/latest/
>
> Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>


[...]


> +static bool add_payload_to_cose(cbor_item_t *cose, VirtIONSM *vnsm)
> +{
> +    cbor_item_t *root = NULL;
> +    cbor_item_t *nested_map;
> +    cbor_item_t *bs = NULL;
> +    size_t locked_cnt;
> +    uint8_t ind[NSM_MAX_PCRS];
> +    size_t payload_map_size = 6;
> +    size_t len;
> +    struct PCRInfo *pcr;
> +    uint8_t zero[64] = {0};
> +    bool r = false;
> +    size_t buf_len = 16384;
> +    uint8_t *buf = g_malloc(buf_len);
> +
> +    if (vnsm->public_key_len > 0) {
> +        payload_map_size++;
> +    }
> +    if (vnsm->user_data_len > 0) {
> +        payload_map_size++;
> +    }
> +    if (vnsm->nonce_len > 0) {
> +        payload_map_size++;
> +    }


Now that you're always emitting user_data and nonce, you should include 
them in payload_map_size unconditionally as well; otherwise your map is 
too small to hold all members.

In addition, a real Nitro Enclave attestation document will return Null 
objects for these fields when they're not set instead of empty strings. 
With the patch below I was able to generate a doc that looks very 
similar to a real one:

diff --git a/hw/virtio/cbor-helpers.c b/hw/virtio/cbor-helpers.c
index 5140020d4e..ffecc97c48 100644
--- a/hw/virtio/cbor-helpers.c
+++ b/hw/virtio/cbor-helpers.c
@@ -140,7 +140,11 @@ bool qemu_cbor_add_bytestring_to_map(cbor_item_t 
*map, const char *key,
     if (!key_cbor) {
         goto cleanup;
     }
-    value_cbor = cbor_build_bytestring(arr, len);
+    if (len) {
+        value_cbor = cbor_build_bytestring(arr, len);
+    } else {
+        value_cbor = cbor_new_null();
+    }
     if (!value_cbor) {
         goto cleanup;
     }
@@ -241,7 +245,11 @@ bool 
qemu_cbor_add_uint8_key_bytestring_to_map(cbor_item_t *map, uint8_t key,
     if (!key_cbor) {
         goto cleanup;
     }
-    value_cbor = cbor_build_bytestring(buf, len);
+    if (len) {
+        value_cbor = cbor_build_bytestring(buf, len);
+    } else {
+        value_cbor = cbor_new_null();
+    }
     if (!value_cbor) {
         goto cleanup;
     }
diff --git a/hw/virtio/virtio-nsm.c b/hw/virtio/virtio-nsm.c
index e91848a2b0..b45d97efe2 100644
--- a/hw/virtio/virtio-nsm.c
+++ b/hw/virtio/virtio-nsm.c
@@ -1126,7 +1126,7 @@ static bool add_payload_to_cose(cbor_item_t *cose, 
VirtIONSM *vnsm)
     cbor_item_t *bs = NULL;
     size_t locked_cnt;
     uint8_t ind[NSM_MAX_PCRS];
-    size_t payload_map_size = 6;
+    size_t payload_map_size = 8;
     size_t len;
     struct PCRInfo *pcr;
     uint8_t zero[64] = {0};
@@ -1137,12 +1137,6 @@ static bool add_payload_to_cose(cbor_item_t 
*cose, VirtIONSM *vnsm)
     if (vnsm->public_key_len > 0) {
         payload_map_size++;
     }
-    if (vnsm->user_data_len > 0) {
-        payload_map_size++;
-    }
-    if (vnsm->nonce_len > 0) {
-        payload_map_size++;
-    }
     root = cbor_new_definite_map(payload_map_size);
     if (!root) {
         goto cleanup;


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