[RFC v1 0/3] Initial support for SPDM

[RFC v1 0/3] Initial support for SPDM

[Alistair Francis]

The Security Protocol and Data Model (SPDM) Specification defines
messages, data objects, and sequences for performing message exchanges
over a variety of transport and physical media.
 - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf

This series is a very initial start on adding SPDM support to QEMU.

This series uses libspdm [1] which is a reference implementation of
SPDM.

The series starts by adding support for building and linking with
libspdm. It then progresses to handling SPDM requests in the NVMe device
over the PCIe DOE protocol.

This is a very early attempt. The code quality is not super high, the C
code hasn't been tested at all. This is really just an RFC to see if
QEMU will accept linking with libspdm.

So, the main question is, how do people feel about linking with libspdm
to support SPDM?

1: https://github.com/DMTF/libspdm

Alistair Francis (3):
  subprojects: libspdm: Initial support
  hw: nvme: ctrl: Initial support for DOE
  hw: nvme: ctrl: Process SPDM requests

 meson.build                   | 78 +++++++++++++++++++++++++++++++++++
 hw/nvme/nvme.h                |  4 ++
 include/hw/pci/pcie_doe.h     |  1 +
 hw/nvme/ctrl.c                | 35 ++++++++++++++++
 .gitmodules                   |  3 ++
 meson_options.txt             |  3 ++
 scripts/meson-buildoptions.sh |  3 ++
 subprojects/.gitignore        |  1 +
 subprojects/libspdm.wrap      |  5 +++
 9 files changed, 133 insertions(+)
 create mode 100644 subprojects/libspdm.wrap

-- 
2.41.0

[RFC v1 1/3] subprojects: libspdm: Initial support

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 meson.build                   | 78 +++++++++++++++++++++++++++++++++++
 .gitmodules                   |  3 ++
 meson_options.txt             |  3 ++
 scripts/meson-buildoptions.sh |  3 ++
 subprojects/.gitignore        |  1 +
 subprojects/libspdm.wrap      |  5 +++
 6 files changed, 93 insertions(+)
 create mode 100644 subprojects/libspdm.wrap

diff --git a/meson.build b/meson.build
index 98e68ef0b1..3ac91defbc 100644
--- a/meson.build
+++ b/meson.build
@@ -1864,6 +1864,15 @@ elif get_option('vduse_blk_export').disabled()
     have_vduse_blk_export = false
 endif
 
+have_libspdm = (targetos == 'linux')
+if get_option('libspdm').enabled()
+    if targetos != 'linux'
+        error('libspdm requires linux')
+    endif
+elif get_option('libspdm').disabled()
+    have_libspdm = false
+endif
+
 # libbpf
 libbpf = dependency('libbpf', required: get_option('bpf'), method: 'pkg-config')
 if libbpf.found() and not cc.links('''
@@ -2141,6 +2150,7 @@ config_host_data.set('CONFIG_VHOST_VDPA', have_vhost_vdpa)
 config_host_data.set('CONFIG_VMNET', vmnet.found())
 config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
 config_host_data.set('CONFIG_VDUSE_BLK_EXPORT', have_vduse_blk_export)
+config_host_data.set('CONFIG_LIBSPDM', have_libspdm)
 config_host_data.set('CONFIG_PNG', png.found())
 config_host_data.set('CONFIG_VNC', vnc.found())
 config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
@@ -3172,6 +3182,7 @@ blockdev_ss = ss.source_set()
 block_ss = ss.source_set()
 chardev_ss = ss.source_set()
 common_ss = ss.source_set()
+libspdm_ss = ss.source_set()
 crypto_ss = ss.source_set()
 hwcore_ss = ss.source_set()
 io_ss = ss.source_set()
@@ -3321,6 +3332,56 @@ if have_libvduse
   libvduse = libvduse_proj.get_variable('libvduse_dep')
 endif
 
+libspdm = not_found
+if have_libspdm
+  cmake = import('cmake')
+  libspdm_opt_var = cmake.subproject_options()
+
+  libspdm_opt_var.add_cmake_defines({'ARCH': 'x64'})
+  libspdm_opt_var.add_cmake_defines({'TOOLCHAIN': 'NONE'})
+  libspdm_opt_var.add_cmake_defines({'TARGET': 'Release'})
+  libspdm_opt_var.add_cmake_defines({'CRYPTO': 'openssl'})
+  libspdm_opt_var.add_cmake_defines({'ENABLE_BINARY_BUILD': '1'})
+  libspdm_opt_var.add_cmake_defines({'COMPILED_LIBCRYPTO_PATH': '/usr/lib'})
+  libspdm_opt_var.add_cmake_defines({'COMPILED_LIBSSL_PATH': '/usr/lib'})
+  libspdm_opt_var.add_cmake_defines({'DISABLE_TESTS': '1'})
+
+  libspdm_proj = cmake.subproject('libspdm', options: libspdm_opt_var)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_common_lib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('memlib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('malloclib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_crypt_lib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_responder_lib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('cryptlib_openssl')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_device_secret_lib_null')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('platform_lib_null')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_secured_message_lib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_crypt_lib')
+  libspdm_ss.add(libspdm_lib)
+
+  libspdm_lib = libspdm_proj.dependency('spdm_crypt_ext_lib')
+  libspdm_ss.add(libspdm_lib)
+endif
+
 # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
 # that is filled in by qapi/.
 subdir('qapi')
@@ -3593,6 +3654,19 @@ libcrypto = static_library('crypto', crypto_ss.sources() + genh,
 crypto = declare_dependency(link_whole: libcrypto,
                             dependencies: [authz, qom])
 
+if have_libspdm
+  libspdm_ss = libspdm_ss.apply(config_host, strict: false)
+  libspdm = static_library('libspdm', libspdm_ss.sources() + genh,
+                             dependencies: [libspdm_ss.dependencies()],
+                             name_suffix: 'fa',
+                             build_by_default: false)
+
+  openssl = dependency('openssl', version : '>=3.0.9')
+  libspdm = declare_dependency(link_whole: libspdm, dependencies: [openssl])
+
+  common_user_inc += 'subprojects/libspdm/include'
+endif
+
 io_ss = io_ss.apply(config_host, strict: false)
 libio = static_library('io', io_ss.sources() + genh,
                        dependencies: [io_ss.dependencies()],
@@ -3668,6 +3742,10 @@ if emulator_modules.length() > 0
   alias_target('modules', emulator_modules)
 endif
 
+if have_libspdm
+  system_ss.add(libspdm)
+endif
+
 system_ss.add(authz, blockdev, chardev, crypto, io, qmp)
 common_ss.add(qom, qemuutil)
 
diff --git a/.gitmodules b/.gitmodules
index 73cae4cd4d..1bf93427ad 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -43,3 +43,6 @@
 [submodule "tests/lcitool/libvirt-ci"]
 	path = tests/lcitool/libvirt-ci
 	url = https://gitlab.com/libvirt/libvirt-ci.git
+[submodule "subprojects/libspdm"]
+	path = subprojects/libspdm
+	url = https://github.com/DMTF/libspdm.git
diff --git a/meson_options.txt b/meson_options.txt
index aaea5ddd77..b61a1f4515 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -292,6 +292,9 @@ option('libvduse', type: 'feature', value: 'auto',
 option('vduse_blk_export', type: 'feature', value: 'auto',
        description: 'VDUSE block export support')
 
+option('libspdm', type: 'feature', value: 'auto',
+       description: 'build libspdm Library')
+
 option('capstone', type: 'feature', value: 'auto',
        description: 'Whether and how to find the capstone library')
 option('fdt', type: 'combo', value: 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 9da3fe299b..8a9d3ad01d 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -126,6 +126,7 @@ meson_options_help() {
   printf "%s\n" '  libudev         Use libudev to enumerate host devices'
   printf "%s\n" '  libusb          libusb support for USB passthrough'
   printf "%s\n" '  libvduse        build VDUSE Library'
+  printf "%s\n" '  libspdm         build libspdm Library'
   printf "%s\n" '  linux-aio       Linux AIO support'
   printf "%s\n" '  linux-io-uring  Linux io_uring support'
   printf "%s\n" '  live-block-migration'
@@ -353,6 +354,8 @@ _meson_option_parse() {
     --disable-libusb) printf "%s" -Dlibusb=disabled ;;
     --enable-libvduse) printf "%s" -Dlibvduse=enabled ;;
     --disable-libvduse) printf "%s" -Dlibvduse=disabled ;;
+    --enable-libspdm) printf "%s" -Dlibspdm=enabled ;;
+    --disable-libspdm) printf "%s" -Dlibspdm=disabled ;;
     --enable-linux-aio) printf "%s" -Dlinux_aio=enabled ;;
     --disable-linux-aio) printf "%s" -Dlinux_aio=disabled ;;
     --enable-linux-io-uring) printf "%s" -Dlinux_io_uring=enabled ;;
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index adca0266be..5d0ac1be1a 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -5,4 +5,5 @@
 /dtc
 /keycodemapdb
 /libvfio-user
+/libspdm
 /slirp
diff --git a/subprojects/libspdm.wrap b/subprojects/libspdm.wrap
new file mode 100644
index 0000000000..ca3607fbaa
--- /dev/null
+++ b/subprojects/libspdm.wrap
@@ -0,0 +1,5 @@
+[wrap-git]
+url = https://github.com/DMTF/libspdm.git
+revision = 55f5141436d2f6debf4b7cd57641cc78d8e4c6fc
+depth = 1
+clone-recursive = true
-- 
2.41.0

[RFC v1 2/3] hw: nvme: ctrl: Initial support for DOE

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/nvme/nvme.h            |  4 ++++
 include/hw/pci/pcie_doe.h |  1 +
 hw/nvme/ctrl.c            | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 209e8f5b4c..e0918516e3 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -20,6 +20,7 @@
 
 #include "qemu/uuid.h"
 #include "hw/pci/pci_device.h"
+#include "hw/pci/pcie_doe.h"
 #include "hw/block/block.h"
 
 #include "block/nvme.h"
@@ -597,6 +598,9 @@ typedef struct NvmeCtrl {
         uint16_t    vqrfap;
         uint16_t    virfap;
     } next_pri_ctrl_cap;    /* These override pri_ctrl_cap after reset */
+
+    /* DOE */
+    DOECap doe_spdm;
 } NvmeCtrl;
 
 typedef enum NvmeResetType {
diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
index 87dc17dcef..18e9492977 100644
--- a/include/hw/pci/pcie_doe.h
+++ b/include/hw/pci/pcie_doe.h
@@ -46,6 +46,7 @@ REG32(PCI_DOE_CAP_STATUS, 0)
 
 /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */
 #define PCI_SIG_DOE_DISCOVERY       0x00
+#define PCI_SIG_DOE_SPDM            0x01
 
 #define PCI_DOE_DW_SIZE_MAX         (1 << 18)
 #define PCI_DOE_PROTOCOL_NUM_MAX    256
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f2e5a2fa73..ec3d5d3c29 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -202,6 +202,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/hostmem.h"
 #include "hw/pci/msix.h"
+#include "hw/pci/pcie_doe.h"
 #include "hw/pci/pcie_sriov.h"
 #include "migration/vmstate.h"
 
@@ -8088,6 +8089,13 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
     return 0;
 }
 
+#ifdef CONFIG_LIBSPDM
+static bool nvme_doe_spdm_rsp(DOECap *doe_cap)
+{
+    return false;
+}
+#endif
+
 static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
 {
     ERRP_GUARD();
@@ -8317,6 +8325,13 @@ void nvme_attach_ns(NvmeCtrl *n, NvmeNamespace *ns)
                             BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
 }
 
+#ifdef CONFIG_LIBSPDM
+static DOEProtocol doe_spdm_prot[] = {
+    { PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_SPDM, nvme_doe_spdm_rsp },
+    { }
+};
+#endif
+
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
     NvmeCtrl *n = NVME(pci_dev);
@@ -8359,6 +8374,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 
         nvme_attach_ns(n, ns);
     }
+
+#ifdef CONFIG_LIBSPDM
+    /* DOE Initailization */
+    pcie_doe_init(pci_dev, &n->doe_spdm, 0x190, doe_spdm_prot, true, 0);
+#endif
 }
 
 static void nvme_exit(PCIDevice *pci_dev)
-- 
2.41.0

[RFC v1 3/3] hw: nvme: ctrl: Process SPDM requests

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/nvme/ctrl.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index ec3d5d3c29..a299dc7175 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -206,6 +206,11 @@
 #include "hw/pci/pcie_sriov.h"
 #include "migration/vmstate.h"
 
+#ifdef CONFIG_LIBSPDM
+#include "library/spdm_common_lib.h"
+#include "library/spdm_responder_lib.h"
+#endif
+
 #include "nvme.h"
 #include "dif.h"
 #include "trace.h"
@@ -8092,6 +8097,16 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
 #ifdef CONFIG_LIBSPDM
 static bool nvme_doe_spdm_rsp(DOECap *doe_cap)
 {
+    void *context = (void *)malloc(libspdm_get_context_size());
+    uint32_t *session_id;
+    bool is_app_message;
+
+    libspdm_init_context(context);
+
+    libspdm_process_request(context, &session_id, &is_app_message,
+                            doe_cap->write_mbox_len,
+                            doe_cap->write_mbox);
+
     return false;
 }
 #endif
-- 
2.41.0

Re: [RFC v1 0/3] Initial support for SPDM

[Jonathan Cameron via]

On Tue,  8 Aug 2023 11:51:21 -0400
Alistair Francis <alistair23@gmail.com> wrote:

> The Security Protocol and Data Model (SPDM) Specification defines
> messages, data objects, and sequences for performing message exchanges
> over a variety of transport and physical media.
>  - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf
> 
> This series is a very initial start on adding SPDM support to QEMU.
> 
> This series uses libspdm [1] which is a reference implementation of
> SPDM.
> 
> The series starts by adding support for building and linking with
> libspdm. It then progresses to handling SPDM requests in the NVMe device
> over the PCIe DOE protocol.
> 
> This is a very early attempt. The code quality is not super high, the C
> code hasn't been tested at all. This is really just an RFC to see if
> QEMU will accept linking with libspdm.
> 
> So, the main question is, how do people feel about linking with libspdm
> to support SPDM?
> 
> 1: https://github.com/DMTF/libspdm

Hi Alastair,

For reference / background we've had SPDM support for CXL emulated devices
in our staging tree for quite a while - it's not upstream because of
exactly this question (+ no one had upstreaming this as a high priority
as out of tree was fine for developing the kernel stack - it's well
isolated in the code so there was little cost in rebasing this feature)
- and because libspdm is packaged by almost no one. There were problems
building it with external crypto libraries etc.

Looks like you have had a lot more success than I ever did in getting that
to work. I tried a few times in the past and ended up sticking with
the Avery design folks approach of a socket connection to spdm-emu
Given you cc'd them I'm guessing you came across that work which is what
we used for testing the kernel code Lukas is working on currently.

https://lore.kernel.org/qemu-devel/20210629132520.00000d1f@Huawei.com/

https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8

So I think we have 2 choices here.
1) Do what you have done and build the library as you are doing.
 - Can fix a version - so don't care if they change the ABI etc other
   than needing to move the version QEMU uses forwards when we need
   to for new features.
 - Cert management is going to add a lot of complexity into QEMU.
   I've not looked at how much infrastructure we can reuse from elsewhere.
   Maybe this is a solved problem.

2) Keep the SPDM emulation external.
 - I'm not sure the socket protocol used by spdm-emu is fixed in stone
   as people tend to change both ends.
 - Cert management and protocol options etc are already available.

Personally I prefer the control option 1 gives us, even if it's a lot more
code.  Option 2 also rather limits our ability to do anything with
the encrypted data as well. It's fine if the aim is just verification
of simple flows, but if we need to inject particular measurements etc
it doesn't really work.

Jonathan



> 
> Alistair Francis (3):
>   subprojects: libspdm: Initial support
>   hw: nvme: ctrl: Initial support for DOE
>   hw: nvme: ctrl: Process SPDM requests
> 
>  meson.build                   | 78 +++++++++++++++++++++++++++++++++++
>  hw/nvme/nvme.h                |  4 ++
>  include/hw/pci/pcie_doe.h     |  1 +
>  hw/nvme/ctrl.c                | 35 ++++++++++++++++
>  .gitmodules                   |  3 ++
>  meson_options.txt             |  3 ++
>  scripts/meson-buildoptions.sh |  3 ++
>  subprojects/.gitignore        |  1 +
>  subprojects/libspdm.wrap      |  5 +++
>  9 files changed, 133 insertions(+)
>  create mode 100644 subprojects/libspdm.wrap
> 

Re: [RFC v1 0/3] Initial support for SPDM

[Alistair Francis]

On Wed, Aug 9, 2023 at 8:11 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Tue,  8 Aug 2023 11:51:21 -0400
> Alistair Francis <alistair23@gmail.com> wrote:
>
> > The Security Protocol and Data Model (SPDM) Specification defines
> > messages, data objects, and sequences for performing message exchanges
> > over a variety of transport and physical media.
> >  - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf
> >
> > This series is a very initial start on adding SPDM support to QEMU.
> >
> > This series uses libspdm [1] which is a reference implementation of
> > SPDM.
> >
> > The series starts by adding support for building and linking with
> > libspdm. It then progresses to handling SPDM requests in the NVMe device
> > over the PCIe DOE protocol.
> >
> > This is a very early attempt. The code quality is not super high, the C
> > code hasn't been tested at all. This is really just an RFC to see if
> > QEMU will accept linking with libspdm.
> >
> > So, the main question is, how do people feel about linking with libspdm
> > to support SPDM?
> >
> > 1: https://github.com/DMTF/libspdm
>
> Hi Alastair,
>
> For reference / background we've had SPDM support for CXL emulated devices
> in our staging tree for quite a while - it's not upstream because of
> exactly this question (+ no one had upstreaming this as a high priority
> as out of tree was fine for developing the kernel stack - it's well
> isolated in the code so there was little cost in rebasing this feature)
> - and because libspdm is packaged by almost no one. There were problems
> building it with external crypto libraries etc.

Thanks for pointing that out! I didn't realise there was existing QEMU
work. I'm glad others are looking into it

Building with libspdm is difficult. Even this series does have weird
issues with openssl's crypto library. I have been working towards
packaging libspdm into buildroot, which has helped cleanup libspdm to
be more user friendly. libspdm 3.0 for example is now a lot easier to
build/package, but I expect to continue to improve things.

There will need to be more improvements to libspdm before this series
could be merged.

>
> Looks like you have had a lot more success than I ever did in getting that
> to work. I tried a few times in the past and ended up sticking with
> the Avery design folks approach of a socket connection to spdm-emu
> Given you cc'd them I'm guessing you came across that work which is what
> we used for testing the kernel code Lukas is working on currently.
>
> https://lore.kernel.org/qemu-devel/20210629132520.00000d1f@Huawei.com/
>
> https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8

Thanks for that!

In the past the QEMU community has refused to accept upstream changes
that expose QEMU internals via sockets. So I suspect linking with
libspdm will be a more upstreamable approach.

On top of that requiring user to run an external socket application is clunky.

>
> So I think we have 2 choices here.
> 1) Do what you have done and build the library as you are doing.
>  - Can fix a version - so don't care if they change the ABI etc other
>    than needing to move the version QEMU uses forwards when we need
>    to for new features.

I agree

>  - Cert management is going to add a lot of complexity into QEMU.
>    I've not looked at how much infrastructure we can reuse from elsewhere.
>    Maybe this is a solved problem.

Could we not just point to a cert when running QEMU?

>
> 2) Keep the SPDM emulation external.
>  - I'm not sure the socket protocol used by spdm-emu is fixed in stone
>    as people tend to change both ends.
>  - Cert management and protocol options etc are already available.

I suspect this will never get upstream though. My aim is to get
something upstream, so this is probably a no go (unless someone jumps
in and says this is ok).

>
> Personally I prefer the control option 1 gives us, even if it's a lot more
> code.  Option 2 also rather limits our ability to do anything with
> the encrypted data as well. It's fine if the aim is just verification
> of simple flows, but if we need to inject particular measurements etc
> it doesn't really work.

I like option 1 as well :)

I don't envisage QEMU supporting extremely complex flows. I was more
aiming for a certificate and some measurement data and maybe a
manifest. My hope was basic SPDM support, not a complete test suite.

Alistair

>
> Jonathan
>
>
>
> >
> > Alistair Francis (3):
> >   subprojects: libspdm: Initial support
> >   hw: nvme: ctrl: Initial support for DOE
> >   hw: nvme: ctrl: Process SPDM requests
> >
> >  meson.build                   | 78 +++++++++++++++++++++++++++++++++++
> >  hw/nvme/nvme.h                |  4 ++
> >  include/hw/pci/pcie_doe.h     |  1 +
> >  hw/nvme/ctrl.c                | 35 ++++++++++++++++
> >  .gitmodules                   |  3 ++
> >  meson_options.txt             |  3 ++
> >  scripts/meson-buildoptions.sh |  3 ++
> >  subprojects/.gitignore        |  1 +
> >  subprojects/libspdm.wrap      |  5 +++
> >  9 files changed, 133 insertions(+)
> >  create mode 100644 subprojects/libspdm.wrap
> >
>

Re: [RFC v1 0/3] Initial support for SPDM

[Jonathan Cameron via]

On Wed, 9 Aug 2023 12:45:35 -0400
Alistair Francis <alistair23@gmail.com> wrote:

> On Wed, Aug 9, 2023 at 8:11 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Tue,  8 Aug 2023 11:51:21 -0400
> > Alistair Francis <alistair23@gmail.com> wrote:
> >  
> > > The Security Protocol and Data Model (SPDM) Specification defines
> > > messages, data objects, and sequences for performing message exchanges
> > > over a variety of transport and physical media.
> > >  - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf
> > >
> > > This series is a very initial start on adding SPDM support to QEMU.
> > >
> > > This series uses libspdm [1] which is a reference implementation of
> > > SPDM.
> > >
> > > The series starts by adding support for building and linking with
> > > libspdm. It then progresses to handling SPDM requests in the NVMe device
> > > over the PCIe DOE protocol.
> > >
> > > This is a very early attempt. The code quality is not super high, the C
> > > code hasn't been tested at all. This is really just an RFC to see if
> > > QEMU will accept linking with libspdm.
> > >
> > > So, the main question is, how do people feel about linking with libspdm
> > > to support SPDM?
> > >
> > > 1: https://github.com/DMTF/libspdm  
> >
> > Hi Alastair,
> >
> > For reference / background we've had SPDM support for CXL emulated devices
> > in our staging tree for quite a while - it's not upstream because of
> > exactly this question (+ no one had upstreaming this as a high priority
> > as out of tree was fine for developing the kernel stack - it's well
> > isolated in the code so there was little cost in rebasing this feature)
> > - and because libspdm is packaged by almost no one. There were problems
> > building it with external crypto libraries etc.  
> 
> Thanks for pointing that out! I didn't realise there was existing QEMU
> work. I'm glad others are looking into it
> 
> Building with libspdm is difficult. Even this series does have weird
> issues with openssl's crypto library. I have been working towards
> packaging libspdm into buildroot, which has helped cleanup libspdm to
> be more user friendly. libspdm 3.0 for example is now a lot easier to
> build/package, but I expect to continue to improve things.
> 
> There will need to be more improvements to libspdm before this series
> could be merged.
> 
> >
> > Looks like you have had a lot more success than I ever did in getting that
> > to work. I tried a few times in the past and ended up sticking with
> > the Avery design folks approach of a socket connection to spdm-emu
> > Given you cc'd them I'm guessing you came across that work which is what
> > we used for testing the kernel code Lukas is working on currently.
> >
> > https://lore.kernel.org/qemu-devel/20210629132520.00000d1f@Huawei.com/
> >
> > https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8  
> 
> Thanks for that!
> 
> In the past the QEMU community has refused to accept upstream changes
> that expose QEMU internals via sockets. So I suspect linking with
> libspdm will be a more upstreamable approach.
> 
> On top of that requiring user to run an external socket application is clunky.
> 
> >
> > So I think we have 2 choices here.
> > 1) Do what you have done and build the library as you are doing.
> >  - Can fix a version - so don't care if they change the ABI etc other
> >    than needing to move the version QEMU uses forwards when we need
> >    to for new features.  
> 
> I agree
> 
> >  - Cert management is going to add a lot of complexity into QEMU.
> >    I've not looked at how much infrastructure we can reuse from elsewhere.
> >    Maybe this is a solved problem.  
> 
> Could we not just point to a cert when running QEMU?

Yes, but it's going to be multiple cert trees per PCI device with all the
association with particular SPDM instances etc. Not too bad I guess.

> 
> >
> > 2) Keep the SPDM emulation external.
> >  - I'm not sure the socket protocol used by spdm-emu is fixed in stone
> >    as people tend to change both ends.
> >  - Cert management and protocol options etc are already available.  
> 
> I suspect this will never get upstream though. My aim is to get
> something upstream, so this is probably a no go (unless someone jumps
> in and says this is ok).

There is precedence with the TPM stuff using swtpm as the provider.
https://qemu.readthedocs.io/en/latest/specs/tpm.html#the-qemu-tpm-emulator-device

> 
> >
> > Personally I prefer the control option 1 gives us, even if it's a lot more
> > code.  Option 2 also rather limits our ability to do anything with
> > the encrypted data as well. It's fine if the aim is just verification
> > of simple flows, but if we need to inject particular measurements etc
> > it doesn't really work.  
> 
> I like option 1 as well :)
> 
> I don't envisage QEMU supporting extremely complex flows. I was more
> aiming for a certificate and some measurement data and maybe a
> manifest. My hope was basic SPDM support, not a complete test suite.

I do envision complex flows. Need to be able to do TDISP for the
confidential computing stuff (and other specs that aren't released yet).

Most of the interest we've had in SPDM in general has been to get things
up and running for that stuff.

Jonathan


> 
> Alistair
> 
> >
> > Jonathan
> >
> >
> >  
> > >
> > > Alistair Francis (3):
> > >   subprojects: libspdm: Initial support
> > >   hw: nvme: ctrl: Initial support for DOE
> > >   hw: nvme: ctrl: Process SPDM requests
> > >
> > >  meson.build                   | 78 +++++++++++++++++++++++++++++++++++
> > >  hw/nvme/nvme.h                |  4 ++
> > >  include/hw/pci/pcie_doe.h     |  1 +
> > >  hw/nvme/ctrl.c                | 35 ++++++++++++++++
> > >  .gitmodules                   |  3 ++
> > >  meson_options.txt             |  3 ++
> > >  scripts/meson-buildoptions.sh |  3 ++
> > >  subprojects/.gitignore        |  1 +
> > >  subprojects/libspdm.wrap      |  5 +++
> > >  9 files changed, 133 insertions(+)
> > >  create mode 100644 subprojects/libspdm.wrap
> > >  
> >  

Re: [RFC v1 0/3] Initial support for SPDM

[Alistair Francis]

On Thu, Aug 10, 2023 at 6:18 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Wed, 9 Aug 2023 12:45:35 -0400
> Alistair Francis <alistair23@gmail.com> wrote:
>
> > On Wed, Aug 9, 2023 at 8:11 AM Jonathan Cameron
> > <Jonathan.Cameron@huawei.com> wrote:
> > >
> > > On Tue,  8 Aug 2023 11:51:21 -0400
> > > Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > > The Security Protocol and Data Model (SPDM) Specification defines
> > > > messages, data objects, and sequences for performing message exchanges
> > > > over a variety of transport and physical media.
> > > >  - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf
> > > >
> > > > This series is a very initial start on adding SPDM support to QEMU.
> > > >
> > > > This series uses libspdm [1] which is a reference implementation of
> > > > SPDM.
> > > >
> > > > The series starts by adding support for building and linking with
> > > > libspdm. It then progresses to handling SPDM requests in the NVMe device
> > > > over the PCIe DOE protocol.
> > > >
> > > > This is a very early attempt. The code quality is not super high, the C
> > > > code hasn't been tested at all. This is really just an RFC to see if
> > > > QEMU will accept linking with libspdm.
> > > >
> > > > So, the main question is, how do people feel about linking with libspdm
> > > > to support SPDM?
> > > >
> > > > 1: https://github.com/DMTF/libspdm
> > >
> > > Hi Alastair,
> > >
> > > For reference / background we've had SPDM support for CXL emulated devices
> > > in our staging tree for quite a while - it's not upstream because of
> > > exactly this question (+ no one had upstreaming this as a high priority
> > > as out of tree was fine for developing the kernel stack - it's well
> > > isolated in the code so there was little cost in rebasing this feature)
> > > - and because libspdm is packaged by almost no one. There were problems
> > > building it with external crypto libraries etc.
> >
> > Thanks for pointing that out! I didn't realise there was existing QEMU
> > work. I'm glad others are looking into it
> >
> > Building with libspdm is difficult. Even this series does have weird
> > issues with openssl's crypto library. I have been working towards
> > packaging libspdm into buildroot, which has helped cleanup libspdm to
> > be more user friendly. libspdm 3.0 for example is now a lot easier to
> > build/package, but I expect to continue to improve things.
> >
> > There will need to be more improvements to libspdm before this series
> > could be merged.
> >
> > >
> > > Looks like you have had a lot more success than I ever did in getting that
> > > to work. I tried a few times in the past and ended up sticking with
> > > the Avery design folks approach of a socket connection to spdm-emu
> > > Given you cc'd them I'm guessing you came across that work which is what
> > > we used for testing the kernel code Lukas is working on currently.
> > >
> > > https://lore.kernel.org/qemu-devel/20210629132520.00000d1f@Huawei.com/
> > >
> > > https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8
> >
> > Thanks for that!
> >
> > In the past the QEMU community has refused to accept upstream changes
> > that expose QEMU internals via sockets. So I suspect linking with
> > libspdm will be a more upstreamable approach.
> >
> > On top of that requiring user to run an external socket application is clunky.
> >
> > >
> > > So I think we have 2 choices here.
> > > 1) Do what you have done and build the library as you are doing.
> > >  - Can fix a version - so don't care if they change the ABI etc other
> > >    than needing to move the version QEMU uses forwards when we need
> > >    to for new features.
> >
> > I agree
> >
> > >  - Cert management is going to add a lot of complexity into QEMU.
> > >    I've not looked at how much infrastructure we can reuse from elsewhere.
> > >    Maybe this is a solved problem.
> >
> > Could we not just point to a cert when running QEMU?
>
> Yes, but it's going to be multiple cert trees per PCI device with all the
> association with particular SPDM instances etc. Not too bad I guess.
>
> >
> > >
> > > 2) Keep the SPDM emulation external.
> > >  - I'm not sure the socket protocol used by spdm-emu is fixed in stone
> > >    as people tend to change both ends.
> > >  - Cert management and protocol options etc are already available.
> >
> > I suspect this will never get upstream though. My aim is to get
> > something upstream, so this is probably a no go (unless someone jumps
> > in and says this is ok).
>
> There is precedence with the TPM stuff using swtpm as the provider.
> https://qemu.readthedocs.io/en/latest/specs/tpm.html#the-qemu-tpm-emulator-device

Good point. I forgot about the TPM.

I have CCed Stefan in case they have any comments

>
> >
> > >
> > > Personally I prefer the control option 1 gives us, even if it's a lot more
> > > code.  Option 2 also rather limits our ability to do anything with
> > > the encrypted data as well. It's fine if the aim is just verification
> > > of simple flows, but if we need to inject particular measurements etc
> > > it doesn't really work.
> >
> > I like option 1 as well :)
> >
> > I don't envisage QEMU supporting extremely complex flows. I was more
> > aiming for a certificate and some measurement data and maybe a
> > manifest. My hope was basic SPDM support, not a complete test suite.
>
> I do envision complex flows. Need to be able to do TDISP for the
> confidential computing stuff (and other specs that aren't released yet).
>
> Most of the interest we've had in SPDM in general has been to get things
> up and running for that stuff.

Hmmm.... That might be trickier to get with an inbuilt library.

Let's wait for feedback on this RFC and then go from there

Alistair

>
> Jonathan
>
>
> >
> > Alistair
> >
> > >
> > > Jonathan
> > >
> > >
> > >
> > > >
> > > > Alistair Francis (3):
> > > >   subprojects: libspdm: Initial support
> > > >   hw: nvme: ctrl: Initial support for DOE
> > > >   hw: nvme: ctrl: Process SPDM requests
> > > >
> > > >  meson.build                   | 78 +++++++++++++++++++++++++++++++++++
> > > >  hw/nvme/nvme.h                |  4 ++
> > > >  include/hw/pci/pcie_doe.h     |  1 +
> > > >  hw/nvme/ctrl.c                | 35 ++++++++++++++++
> > > >  .gitmodules                   |  3 ++
> > > >  meson_options.txt             |  3 ++
> > > >  scripts/meson-buildoptions.sh |  3 ++
> > > >  subprojects/.gitignore        |  1 +
> > > >  subprojects/libspdm.wrap      |  5 +++
> > > >  9 files changed, 133 insertions(+)
> > > >  create mode 100644 subprojects/libspdm.wrap
> > > >
> > >
>