[PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[htafr]

(I) Summary
===========================================================================

This patch is the beginning of the support of the Security Protocol and 
Data Model (SPDM). There are some known issues (see II), but it's
usable and not many users are going to use this functionality for now,
but for those who will it may facilitate the development.

There are some people working with LibSPDM to implement the SPDM on 
emulated devices, however current works that use QEMU compile LibSPDM
out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM when 
user pass the parameter '--enable-libspdm' to configure file, this option 
is disabled by default. The following parameters were also added:

  --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls] (choices:
                           mbedtls/openssl)
  --libspdm-toolchain=VALUE
                           toolchain to use for LibSPDM compilation [GCC]

In order to facilitate future code development using LibSPDM API, this
patch also provides the definition of the macro 'CONFIG_LIBSPDM'.


(II) Known Limitations
===========================================================================

1. This patch enables LibSPDM in-tree compilation for Linux systems only.
2. LibSPDM compilation uses CMake, so meson build system is making use
   of the CMake module [4].
3. Some problems may occur when compiling LibSPDM with MbedTls such as:
    error: "_GNU_SOURCE" redefined [-Werror]
      10 | #define _GNU_SOURCE

   It's possible to compile using --disable-werror.

(III) Sample configuration
===========================================================================

../configure \
  --disable-werror \
  --enable-libspdm \
  --libspdm-crypto=mbedtls \
  --enable-gcov

References:
[1] riscv-spdm 
  Link: https://github.com/htafr/riscv-spdm
[2] spdm-benchmark 
  Link: https://github.com/rcaalves/spdm-benchmark
[3] qemu-spdm-emulation-guide
  Link: https://github.com/twilfredo/qemu-spdm-emulation-guide
[4] The Meson Build System: CMake module
  Link: https://mesonbuild.com/CMake-module.html

htafr (1):
  libspdm: insert LibSPDM as subproject

 .gitmodules                   |  3 ++
 Kconfig.host                  |  3 ++
 meson.build                   | 84 +++++++++++++++++++++++++++++++++++
 meson_options.txt             |  8 ++++
 scripts/make-config-poison.sh | 19 ++++----
 scripts/meson-buildoptions.sh |  9 ++++
 6 files changed, 117 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH 1/1] libspdm: insert LibSPDM as subproject

[htafr]

Signed-off-by: htafr <htafreit@gmail.com>
---
 .gitmodules                   |  3 ++
 Kconfig.host                  |  3 ++
 meson.build                   | 84 +++++++++++++++++++++++++++++++++++
 meson_options.txt             |  8 ++++
 scripts/make-config-poison.sh | 19 ++++----
 scripts/meson-buildoptions.sh |  9 ++++
 6 files changed, 117 insertions(+), 9 deletions(-)

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/Kconfig.host b/Kconfig.host
index 4ade7899d6..80bde9eb3d 100644
--- a/Kconfig.host
+++ b/Kconfig.host
@@ -23,6 +23,9 @@ config IVSHMEM
 config TPM
     bool
 
+config SPDM
+    bool
+
 config FDT
     bool
 
diff --git a/meson.build b/meson.build
index d26690ce20..40bef9a5cc 100644
--- a/meson.build
+++ b/meson.build
@@ -17,6 +17,7 @@ not_found = dependency('', required: false)
 keyval = import('keyval')
 ss = import('sourceset')
 fs = import('fs')
+cmake = import('cmake')
 
 host_os = host_machine.system()
 config_host = keyval.load(meson.current_build_dir() / 'config-host.mak')
@@ -163,6 +164,10 @@ have_tpm = get_option('tpm') \
   .require(host_os != 'windows', error_message: 'TPM emulation only available on POSIX systems') \
   .allowed()
 
+have_libspdm = get_option('libspdm') \
+  .require(host_os == 'linux', error_message: 'LibSPDM is supported only on Linux') \
+  .allowed()
+
 # vhost
 have_vhost_user = get_option('vhost_user') \
   .disable_auto_if(host_os != 'linux') \
@@ -280,6 +285,65 @@ if host_os != 'darwin'
   modular_tcg = ['i386-softmmu', 'x86_64-softmmu']
 endif
 
+libspdm_dep_targets = [
+  'debuglib',
+  'malloclib',
+  'memlib',
+  'platform_lib',
+  'rnglib',
+  'spdm_cert_verify_callback_sample',
+  'spdm_common_lib',
+  'spdm_crypt_ext_lib',
+  'spdm_crypt_lib',
+  'spdm_device_secret_lib_sample',
+  'spdm_requester_lib',
+  'spdm_responder_lib',
+  'spdm_secured_message_lib',
+  'spdm_transport_mctp_lib',
+  'spdm_transport_pcidoe_lib',
+  ]
+
+if cpu not in supported_cpus
+  libspdm_host_arch = 'unknown'
+  message(f'LibSPDM will not compile with CMake defines: -DARCH=@libspdm_host_arch@')
+elif cpu == 'x86_64'
+  libspdm_host_arch = 'x64'
+elif cpu == 'x86'
+  libspdm_host_arch = 'ia32'
+elif cpu == 'arm'
+  libspdm_host_arch = 'arm'
+elif cpu == 'aarch64'
+  libspdm_host_arch = 'aarch64'
+else
+  libspdm_host_arch = cpu
+endif
+
+if get_option('debug')
+  libspdm_target = 'Debug'
+else 
+  libspdm_target = 'Release'
+endif
+
+if get_option('libspdm-crypto') == 'openssl'
+  libspdm_dep_targets += [
+    'cryptlib_openssl',
+    'openssllib',
+  ]
+elif get_option('libspdm-crypto') == 'mbedtls'
+  libspdm_dep_targets += [
+    'cryptlib_mbedtls',
+    'mbedcrypto',
+    'mbedtls',
+    'mbedx509',
+  ]
+endif
+
+if get_option('b_coverage')
+  libspdm_gcov = 'ON'
+else 
+  libspdm_gcov = 'OFF'
+endif
+
 ##################
 # Compiler flags #
 ##################
@@ -2409,6 +2473,7 @@ if get_option('tcg').allowed()
   config_host_data.set('CONFIG_TCG', 1)
   config_host_data.set('CONFIG_TCG_INTERPRETER', tcg_arch == 'tci')
 endif
+config_host_data.set('CONFIG_LIBSPDM', have_libspdm)
 config_host_data.set('CONFIG_TPM', have_tpm)
 config_host_data.set('CONFIG_TSAN', get_option('tsan'))
 config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
@@ -3310,6 +3375,23 @@ if have_libvduse
   libvduse = libvduse_proj.get_variable('libvduse_dep')
 endif
 
+spdm = []
+if have_libspdm
+  libspdm_opt = cmake.subproject_options()
+  libspdm_opt.add_cmake_defines({'ARCH': libspdm_host_arch, \
+                                 'TOOLCHAIN': get_option('libspdm-toolchain'), \
+                                 'TARGET': libspdm_target, \
+                                 'CRYPTO': get_option('libspdm-crypto'), \
+                                 'DISABLE_TESTS': 1, \
+                                 'GCOV': libspdm_gcov, \
+                                 'BUILD_LINUX_SHARED_LIB': 'ON'})
+  libspdm_proj = cmake.subproject('libspdm', options: libspdm_opt)
+
+  foreach dep : libspdm_dep_targets
+    spdm += libspdm_proj.dependency(dep)
+  endforeach
+endif
+
 #####################
 # Generated sources #
 #####################
@@ -3892,6 +3974,7 @@ common_ss.add(hwcore)
 ###########
 
 system_ss.add(authz, blockdev, chardev, crypto, io, qmp)
+system_ss.add(when: spdm, if_true: spdm)
 common_ss.add(qom, qemuutil)
 
 common_ss.add_all(when: 'CONFIG_SYSTEM_ONLY', if_true: [system_ss])
@@ -4574,6 +4657,7 @@ summary_info += {'AF_ALG support':    have_afalg}
 summary_info += {'rng-none':          get_option('rng_none')}
 summary_info += {'Linux keyring':     have_keyring}
 summary_info += {'Linux keyutils':    keyutils}
+summary_info += {'LibSPDM algorithm': get_option('libspdm-crypto')}
 summary(summary_info, bool_yn: true, section: 'Crypto')
 
 # UI
diff --git a/meson_options.txt b/meson_options.txt
index 0ee4d7bb86..e991109048 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -376,3 +376,11 @@ option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'], value
 
 option('rust', type: 'feature', value: 'disabled',
        description: 'Rust support')
+
+option('libspdm', type: 'feature', value: 'disabled',
+  description: 'LibSPDM support')
+option('libspdm-crypto', type: 'combo', value: 'mbedtls',
+  choices: ['mbedtls', 'openssl'],
+  description: 'set LibSPDM crypto algorithm')
+option('libspdm-toolchain', type: 'string', value: 'GCC',
+  description: 'toolchain to use for LibSPDM compilation')
diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
index 2b36907e23..d4582007da 100755
--- a/scripts/make-config-poison.sh
+++ b/scripts/make-config-poison.sh
@@ -1,17 +1,18 @@
 #! /bin/sh
 
 if test $# = 0; then
-  exit 0
+    exit 0
 fi
 
 # Create list of config switches that should be poisoned in common code,
 # but filter out several which are handled manually.
 exec sed -n \
-  -e' /CONFIG_TCG/d' \
-  -e '/CONFIG_USER_ONLY/d' \
-  -e '/CONFIG_SOFTMMU/d' \
-  -e '/^#define / {' \
-  -e    's///' \
-  -e    's/ .*//' \
-  -e    's/^/#pragma GCC poison /p' \
-  -e '}' "$@" | sort -u
+    -e' /CONFIG_TCG/d' \
+    -e '/CONFIG_USER_ONLY/d' \
+    -e '/CONFIG_SOFTMMU/d' \
+    -e '/CONFIG_SPDM/d' \
+    -e '/^#define / {' \
+    -e 's///' \
+    -e 's/ .*//' \
+    -e 's/^/#pragma GCC poison /p' \
+    -e '}' "$@" | sort -u
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 6d08605b77..115113fdf4 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -62,6 +62,10 @@ meson_options_help() {
   printf "%s\n" '                           cpu name [/usr/gnemul/qemu-%M]'
   printf "%s\n" '  --libdir=VALUE           Library directory [system default]'
   printf "%s\n" '  --libexecdir=VALUE       Library executable directory [libexec]'
+  printf "%s\n" '  --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls] (choices:'
+  printf "%s\n" '                           mbedtls/openssl)'
+  printf "%s\n" '  --libspdm-toolchain=VALUE'
+  printf "%s\n" '                           toolchain to use for LibSPDM compilation [GCC]'
   printf "%s\n" '  --localedir=VALUE        Locale data directory [share/locale]'
   printf "%s\n" '  --localstatedir=VALUE    Localstate data directory [/var/local]'
   printf "%s\n" '  --mandir=VALUE           Manual page directory [share/man]'
@@ -139,6 +143,7 @@ meson_options_help() {
   printf "%s\n" '  libkeyutils     Linux keyutils support'
   printf "%s\n" '  libnfs          libnfs block device driver'
   printf "%s\n" '  libpmem         libpmem support'
+  printf "%s\n" '  libspdm         LibSPDM support'
   printf "%s\n" '  libssh          ssh block device support'
   printf "%s\n" '  libudev         Use libudev to enumerate host devices'
   printf "%s\n" '  libusb          libusb support for USB passthrough'
@@ -372,6 +377,10 @@ _meson_option_parse() {
     --disable-libnfs) printf "%s" -Dlibnfs=disabled ;;
     --enable-libpmem) printf "%s" -Dlibpmem=enabled ;;
     --disable-libpmem) printf "%s" -Dlibpmem=disabled ;;
+    --enable-libspdm) printf "%s" -Dlibspdm=enabled ;;
+    --disable-libspdm) printf "%s" -Dlibspdm=disabled ;;
+    --libspdm-crypto=*) quote_sh "-Dlibspdm-crypto=$2" ;;
+    --libspdm-toolchain=*) quote_sh "-Dlibspdm-toolchain=$2" ;;
     --enable-libssh) printf "%s" -Dlibssh=enabled ;;
     --disable-libssh) printf "%s" -Dlibssh=disabled ;;
     --enable-libudev) printf "%s" -Dlibudev=enabled ;;
-- 
2.43.0

Re: [PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[Alistair Francis]

On Thu, Oct 17, 2024 at 2:35 AM htafr <htafreit@gmail.com> wrote:
>
> (I) Summary
> ===========================================================================
>
> This patch is the beginning of the support of the Security Protocol and
> Data Model (SPDM). There are some known issues (see II), but it's
> usable and not many users are going to use this functionality for now,
> but for those who will it may facilitate the development.
>
> There are some people working with LibSPDM to implement the SPDM on
> emulated devices, however current works that use QEMU compile LibSPDM
> out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM when
> user pass the parameter '--enable-libspdm' to configure file, this option
> is disabled by default. The following parameters were also added:
>
>   --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls] (choices:
>                            mbedtls/openssl)
>   --libspdm-toolchain=VALUE
>                            toolchain to use for LibSPDM compilation [GCC]
>
> In order to facilitate future code development using LibSPDM API, this
> patch also provides the definition of the macro 'CONFIG_LIBSPDM'.

We have talked about this before, see
https://patchew.org/QEMU/cover.1691509717.git.alistair.francis@wdc.com/

The general agreement seemed to be that it will be hard to do SPDM
configuration inside QEMU, hence the external library (like the QEMU
TPM support).

>
>
> (II) Known Limitations
> ===========================================================================
>
> 1. This patch enables LibSPDM in-tree compilation for Linux systems only.
> 2. LibSPDM compilation uses CMake, so meson build system is making use
>    of the CMake module [4].
> 3. Some problems may occur when compiling LibSPDM with MbedTls such as:
>     error: "_GNU_SOURCE" redefined [-Werror]
>       10 | #define _GNU_SOURCE
>
>    It's possible to compile using --disable-werror.
>
> (III) Sample configuration
> ===========================================================================
>
> ../configure \
>   --disable-werror \
>   --enable-libspdm \
>   --libspdm-crypto=mbedtls \
>   --enable-gcov
>
> References:
> [1] riscv-spdm
>   Link: https://github.com/htafr/riscv-spdm
> [2] spdm-benchmark
>   Link: https://github.com/rcaalves/spdm-benchmark
> [3] qemu-spdm-emulation-guide
>   Link: https://github.com/twilfredo/qemu-spdm-emulation-guide

This one has been merged upstream and mainline QEMU supports it now:

https://www.qemu.org/docs/master/specs/spdm.html

Alistair

Re: [PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[Daniel P. Berrangé]

On Thu, Oct 17, 2024 at 02:00:35PM +1000, Alistair Francis wrote:
> On Thu, Oct 17, 2024 at 2:35 AM htafr <htafreit@gmail.com> wrote:
> >
> > (I) Summary
> > ===========================================================================
> >
> > This patch is the beginning of the support of the Security Protocol and
> > Data Model (SPDM). There are some known issues (see II), but it's
> > usable and not many users are going to use this functionality for now,
> > but for those who will it may facilitate the development.
> >
> > There are some people working with LibSPDM to implement the SPDM on
> > emulated devices, however current works that use QEMU compile LibSPDM
> > out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM when
> > user pass the parameter '--enable-libspdm' to configure file, this option
> > is disabled by default. The following parameters were also added:
> >
> >   --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls] (choices:
> >                            mbedtls/openssl)
> >   --libspdm-toolchain=VALUE
> >                            toolchain to use for LibSPDM compilation [GCC]
> >
> > In order to facilitate future code development using LibSPDM API, this
> > patch also provides the definition of the macro 'CONFIG_LIBSPDM'.
> 
> We have talked about this before, see
> https://patchew.org/QEMU/cover.1691509717.git.alistair.francis@wdc.com/
> 
> The general agreement seemed to be that it will be hard to do SPDM
> configuration inside QEMU, hence the external library (like the QEMU
> TPM support).

More generally, seeing this libspdm proposed for QEMU, without any
corresponding usage of it it dubious. It is hard to judge whether
it makes any sense, without seeing how it will be used in real
device code inside QEMU.

On the cryptography side, I'm not a fan of linking another
crypto library to QEMU, that's different from what we already
support in our crypto layer. openssl in particular is a problem
due to its licensing - people tend to hand-waive away the
licensing incompatibility by pretending openssl is a "system library"
but I disagree with that interpretation.

> > (II) Known Limitations
> > ===========================================================================
> >
> > 1. This patch enables LibSPDM in-tree compilation for Linux systems only.
> > 2. LibSPDM compilation uses CMake, so meson build system is making use
> >    of the CMake module [4].
> > 3. Some problems may occur when compiling LibSPDM with MbedTls such as:
> >     error: "_GNU_SOURCE" redefined [-Werror]
> >       10 | #define _GNU_SOURCE
> >
> >    It's possible to compile using --disable-werror.
> >
> > (III) Sample configuration
> > ===========================================================================
> >
> > ../configure \
> >   --disable-werror \
> >   --enable-libspdm \
> >   --libspdm-crypto=mbedtls \
> >   --enable-gcov
> >
> > References:
> > [1] riscv-spdm
> >   Link: https://github.com/htafr/riscv-spdm
> > [2] spdm-benchmark
> >   Link: https://github.com/rcaalves/spdm-benchmark
> > [3] qemu-spdm-emulation-guide
> >   Link: https://github.com/twilfredo/qemu-spdm-emulation-guide
> 
> This one has been merged upstream and mainline QEMU supports it now:
> 
> https://www.qemu.org/docs/master/specs/spdm.html

So with that merged, is this proposal for linking to libspdm redundant ?

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

Re: [PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[Ágatha Freitas]

[-- Attachment #1: Type: text/plain, Size: 4953 bytes --]

On Thu, Oct 17, 2024 at 7:00 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, Oct 17, 2024 at 02:00:35PM +1000, Alistair Francis wrote:
> > On Thu, Oct 17, 2024 at 2:35 AM htafr <htafreit@gmail.com> wrote:
> > >
> > > (I) Summary
> > >
> ===========================================================================
> > >
> > > This patch is the beginning of the support of the Security Protocol and
> > > Data Model (SPDM). There are some known issues (see II), but it's
> > > usable and not many users are going to use this functionality for now,
> > > but for those who will it may facilitate the development.
> > >
> > > There are some people working with LibSPDM to implement the SPDM on
> > > emulated devices, however current works that use QEMU compile LibSPDM
> > > out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM
> when
> > > user pass the parameter '--enable-libspdm' to configure file, this
> option
> > > is disabled by default. The following parameters were also added:
> > >
> > >   --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls]
> (choices:
> > >                            mbedtls/openssl)
> > >   --libspdm-toolchain=VALUE
> > >                            toolchain to use for LibSPDM compilation
> [GCC]
> > >
> > > In order to facilitate future code development using LibSPDM API, this
> > > patch also provides the definition of the macro 'CONFIG_LIBSPDM'.
> >
> > We have talked about this before, see
> > https://patchew.org/QEMU/cover.1691509717.git.alistair.francis@wdc.com/
> >
> > The general agreement seemed to be that it will be hard to do SPDM
> > configuration inside QEMU, hence the external library (like the QEMU
> > TPM support).
>
> More generally, seeing this libspdm proposed for QEMU, without any
> corresponding usage of it it dubious. It is hard to judge whether
> it makes any sense, without seeing how it will be used in real
> device code inside QEMU.
>

Currently, I'm working with EDK2 and QEMU so I have a branch [1] with
ongoing
modifications in files backends/spdm.c and hw/nvme/auth.c. Although the
current
modifications are able to exchange SPDM messages, it's far from being
complete
and it's not following better code practices yet. I'm making use of
Alistair's and
Mallawa's previous work in NVMe to authenticate it through PCI [2].

[1]  WIP: SPDM integration
      Link: https://github.com/htafr/qemu/tree/libspdm-dev
[2] WIP: SPDM in OVMF
      Link: https://github.com/htafr/edk2/tree/ovmf-spdm


>
> On the cryptography side, I'm not a fan of linking another
> crypto library to QEMU, that's different from what we already
> support in our crypto layer. openssl in particular is a problem
> due to its licensing - people tend to hand-waive away the
> licensing incompatibility by pretending openssl is a "system library"
> but I disagree with that interpretation.
>
> > > (II) Known Limitations
> > >
> ===========================================================================
> > >
> > > 1. This patch enables LibSPDM in-tree compilation for Linux systems
> only.
> > > 2. LibSPDM compilation uses CMake, so meson build system is making use
> > >    of the CMake module [4].
> > > 3. Some problems may occur when compiling LibSPDM with MbedTls such as:
> > >     error: "_GNU_SOURCE" redefined [-Werror]
> > >       10 | #define _GNU_SOURCE
> > >
> > >    It's possible to compile using --disable-werror.
> > >
> > > (III) Sample configuration
> > >
> ===========================================================================
> > >
> > > ../configure \
> > >   --disable-werror \
> > >   --enable-libspdm \
> > >   --libspdm-crypto=mbedtls \
> > >   --enable-gcov
> > >
> > > References:
> > > [1] riscv-spdm
> > >   Link: https://github.com/htafr/riscv-spdm
> > > [2] spdm-benchmark
> > >   Link: https://github.com/rcaalves/spdm-benchmark
> > > [3] qemu-spdm-emulation-guide
> > >   Link: https://github.com/twilfredo/qemu-spdm-emulation-guide
> >
> > This one has been merged upstream and mainline QEMU supports it now:
> >
> > https://www.qemu.org/docs/master/specs/spdm.html
>
> So with that merged, is this proposal for linking to libspdm redundant ?
>
> 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 :|
>
>
I'm not sure if I understood the redundancy. Would it be against QEMU
practices
to have another openssl as well as mbedtls linked inside it?

Also, I didn't know the LibSPDM insertion was already discussed previously
as
Alistair pointed out. I think I should have sent this patch as RFC instead.
As this is
my first interaction in any mail list, I'm sorry for any mistakes I made.

[-- Attachment #2: Type: text/html, Size: 7458 bytes --]

Re: [PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[Daniel P. Berrangé]

On Thu, Oct 17, 2024 at 10:37:21AM -0300, Ágatha Freitas wrote:
> On Thu, Oct 17, 2024 at 7:00 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Thu, Oct 17, 2024 at 02:00:35PM +1000, Alistair Francis wrote:
> > > On Thu, Oct 17, 2024 at 2:35 AM htafr <htafreit@gmail.com> wrote:
> > > >
> > > > (I) Summary
> > > >
> > ===========================================================================
> > > >
> > > > This patch is the beginning of the support of the Security Protocol and
> > > > Data Model (SPDM). There are some known issues (see II), but it's
> > > > usable and not many users are going to use this functionality for now,
> > > > but for those who will it may facilitate the development.
> > > >
> > > > There are some people working with LibSPDM to implement the SPDM on
> > > > emulated devices, however current works that use QEMU compile LibSPDM
> > > > out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM
> > when
> > > > user pass the parameter '--enable-libspdm' to configure file, this
> > option
> > > > is disabled by default. The following parameters were also added:
> > > >
> > > >   --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls]
> > (choices:
> > > >                            mbedtls/openssl)
> > > >   --libspdm-toolchain=VALUE
> > > >                            toolchain to use for LibSPDM compilation
> > [GCC]
> > > >
> > > > In order to facilitate future code development using LibSPDM API, this
> > > > patch also provides the definition of the macro 'CONFIG_LIBSPDM'.
> > >
> > > We have talked about this before, see
> > > https://patchew.org/QEMU/cover.1691509717.git.alistair.francis@wdc.com/
> > >
> > > The general agreement seemed to be that it will be hard to do SPDM
> > > configuration inside QEMU, hence the external library (like the QEMU
> > > TPM support).
> >
> > More generally, seeing this libspdm proposed for QEMU, without any
> > corresponding usage of it it dubious. It is hard to judge whether
> > it makes any sense, without seeing how it will be used in real
> > device code inside QEMU.
> >
> 
> Currently, I'm working with EDK2 and QEMU so I have a branch [1] with
> ongoing
> modifications in files backends/spdm.c and hw/nvme/auth.c. Although the
> current
> modifications are able to exchange SPDM messages, it's far from being
> complete
> and it's not following better code practices yet. I'm making use of
> Alistair's and
> Mallawa's previous work in NVMe to authenticate it through PCI [2].
> 
> [1]  WIP: SPDM integration
>       Link: https://github.com/htafr/qemu/tree/libspdm-dev
> [2] WIP: SPDM in OVMF
>       Link: https://github.com/htafr/edk2/tree/ovmf-spdm
> 
> 
> >
> > On the cryptography side, I'm not a fan of linking another
> > crypto library to QEMU, that's different from what we already
> > support in our crypto layer. openssl in particular is a problem
> > due to its licensing - people tend to hand-waive away the
> > licensing incompatibility by pretending openssl is a "system library"
> > but I disagree with that interpretation.
> >
> > > > (II) Known Limitations
> > > >
> > ===========================================================================
> > > >
> > > > 1. This patch enables LibSPDM in-tree compilation for Linux systems
> > only.
> > > > 2. LibSPDM compilation uses CMake, so meson build system is making use
> > > >    of the CMake module [4].
> > > > 3. Some problems may occur when compiling LibSPDM with MbedTls such as:
> > > >     error: "_GNU_SOURCE" redefined [-Werror]
> > > >       10 | #define _GNU_SOURCE
> > > >
> > > >    It's possible to compile using --disable-werror.
> > > >
> > > > (III) Sample configuration
> > > >
> > ===========================================================================
> > > >
> > > > ../configure \
> > > >   --disable-werror \
> > > >   --enable-libspdm \
> > > >   --libspdm-crypto=mbedtls \
> > > >   --enable-gcov
> > > >
> > > > References:
> > > > [1] riscv-spdm
> > > >   Link: https://github.com/htafr/riscv-spdm
> > > > [2] spdm-benchmark
> > > >   Link: https://github.com/rcaalves/spdm-benchmark
> > > > [3] qemu-spdm-emulation-guide
> > > >   Link: https://github.com/twilfredo/qemu-spdm-emulation-guide
> > >
> > > This one has been merged upstream and mainline QEMU supports it now:
> > >
> > > https://www.qemu.org/docs/master/specs/spdm.html
> >
> > So with that merged, is this proposal for linking to libspdm redundant ?
> >
>
> I'm not sure if I understood the redundancy. Would it be against QEMU
> practices
> to have another openssl as well as mbedtls linked inside it?

QEMU doesn't link to either of those libraries. We preferentially use
gnutls, with a fallback to gcrypt or nettle, to avoid the murky openssl
licensing situation, and to a lesser extent because openssl has an
unpleasant API to use. mbedtls isn't used because it is a more niche
solution compared to what we already support, so wasn't compelling to
support.

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

Re: [PATCH 0/1] Insert LibSPDM in QEMU enabling in-tree compilation

[Alistair Francis]

On Thu, Oct 17, 2024 at 11:37 PM Ágatha Freitas <htafreit@gmail.com> wrote:
>
>
>
> On Thu, Oct 17, 2024 at 7:00 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Thu, Oct 17, 2024 at 02:00:35PM +1000, Alistair Francis wrote:
>> > On Thu, Oct 17, 2024 at 2:35 AM htafr <htafreit@gmail.com> wrote:
>> > >
>> > > (I) Summary
>> > > ===========================================================================
>> > >
>> > > This patch is the beginning of the support of the Security Protocol and
>> > > Data Model (SPDM). There are some known issues (see II), but it's
>> > > usable and not many users are going to use this functionality for now,
>> > > but for those who will it may facilitate the development.
>> > >
>> > > There are some people working with LibSPDM to implement the SPDM on
>> > > emulated devices, however current works that use QEMU compile LibSPDM
>> > > out-of-tree [1][2][3]. This patch enables the compilation of LibSPDM when
>> > > user pass the parameter '--enable-libspdm' to configure file, this option
>> > > is disabled by default. The following parameters were also added:
>> > >
>> > >   --libspdm-crypto=CHOICE  set LibSPDM crypto algorithm [mbedtls] (choices:
>> > >                            mbedtls/openssl)
>> > >   --libspdm-toolchain=VALUE
>> > >                            toolchain to use for LibSPDM compilation [GCC]
>> > >
>> > > In order to facilitate future code development using LibSPDM API, this
>> > > patch also provides the definition of the macro 'CONFIG_LIBSPDM'.
>> >
>> > We have talked about this before, see
>> > https://patchew.org/QEMU/cover.1691509717.git.alistair.francis@wdc.com/
>> >
>> > The general agreement seemed to be that it will be hard to do SPDM
>> > configuration inside QEMU, hence the external library (like the QEMU
>> > TPM support).
>>
>> More generally, seeing this libspdm proposed for QEMU, without any
>> corresponding usage of it it dubious. It is hard to judge whether
>> it makes any sense, without seeing how it will be used in real
>> device code inside QEMU.
>
>
> Currently, I'm working with EDK2 and QEMU so I have a branch [1] with ongoing
> modifications in files backends/spdm.c and hw/nvme/auth.c. Although the current
> modifications are able to exchange SPDM messages, it's far from being complete
> and it's not following better code practices yet. I'm making use of Alistair's and
> Mallawa's previous work in NVMe to authenticate it through PCI [2].
>
> [1]  WIP: SPDM integration
>       Link: https://github.com/htafr/qemu/tree/libspdm-dev
> [2] WIP: SPDM in OVMF
>       Link: https://github.com/htafr/edk2/tree/ovmf-spdm

I also started working on this, see
https://github.com/tianocore/edk2/pull/5715 .
I was working towards SPDM communication over DOE as well.

Unfortunately it stalled with the EDK2 review process being so incredibly slow

>
>>
>>
>> On the cryptography side, I'm not a fan of linking another
>> crypto library to QEMU, that's different from what we already
>> support in our crypto layer. openssl in particular is a problem
>> due to its licensing - people tend to hand-waive away the
>> licensing incompatibility by pretending openssl is a "system library"
>> but I disagree with that interpretation.
>>
>> > > (II) Known Limitations
>> > > ===========================================================================
>> > >
>> > > 1. This patch enables LibSPDM in-tree compilation for Linux systems only.
>> > > 2. LibSPDM compilation uses CMake, so meson build system is making use
>> > >    of the CMake module [4].
>> > > 3. Some problems may occur when compiling LibSPDM with MbedTls such as:
>> > >     error: "_GNU_SOURCE" redefined [-Werror]
>> > >       10 | #define _GNU_SOURCE
>> > >
>> > >    It's possible to compile using --disable-werror.
>> > >
>> > > (III) Sample configuration
>> > > ===========================================================================
>> > >
>> > > ../configure \
>> > >   --disable-werror \
>> > >   --enable-libspdm \
>> > >   --libspdm-crypto=mbedtls \
>> > >   --enable-gcov
>> > >
>> > > References:
>> > > [1] riscv-spdm
>> > >   Link: https://github.com/htafr/riscv-spdm
>> > > [2] spdm-benchmark
>> > >   Link: https://github.com/rcaalves/spdm-benchmark
>> > > [3] qemu-spdm-emulation-guide
>> > >   Link: https://github.com/twilfredo/qemu-spdm-emulation-guide
>> >
>> > This one has been merged upstream and mainline QEMU supports it now:
>> >
>> > https://www.qemu.org/docs/master/specs/spdm.html
>>
>> So with that merged, is this proposal for linking to libspdm redundant ?
>>
>> 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 :|
>>
>
> I'm not sure if I understood the redundancy. Would it be against QEMU practices
> to have another openssl as well as mbedtls linked inside it?
>
> Also, I didn't know the LibSPDM insertion was already discussed previously as
> Alistair pointed out. I think I should have sent this patch as RFC instead. As this is
> my first interaction in any mail list, I'm sorry for any mistakes I made.

No worries, there were no mistakes made. I was just pointing out that
we have had this discussion before and settled on an external SPDM
implementation.

That doesn't mean we can't change that in the future, but I think it
needs a good justification and like Daniel says at least a partial
implementation to go with it

Alistair