Re: [Buildroot] [PATCH v3] package/libspdm: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Alistair,

On Wed, 30 Aug 2023 15:36:54 +1000
Alistair Francis <alistair23@gmail.com> wrote:

> Add the libspdm package (https://github.com/DMTF/libspdm).
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Thanks for this new iteration, it looks much better. I had fixed a few
things and was getting ready to apply, but it actually doesn't build.
See below some details to help you prepare a v4.

> diff --git a/DEVELOPERS b/DEVELOPERS
> index 9b500f3701..5b5556d492 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -122,6 +122,7 @@ F:	board/sifive/
>  F:	boot/opensbi/
>  F:	configs/hifive_unleashed_defconfig
>  F:	package/xen/
> +F:	package/libspdm/

Please respect alphabetic ordering.

>  
>  N:	Alvaro G. M <alvaro.gamez@hazent.com>
>  F:	package/dcron/
> diff --git a/package/Config.in b/package/Config.in
> index 54cddc3914..eda464262a 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -193,6 +193,7 @@ menu "Development tools"
>  	source "package/jo/Config.in"
>  	source "package/jq/Config.in"
>  	source "package/libtool/Config.in"
> +	source "package/libspdm/Config.in"

Please respect alphabetic ordering. Also, I believe this package should
be in "Libraries -> Crypto", and not in "Development tools".

> diff --git a/package/libspdm/0001-cryptlib_openssl-x509-Remove-internal-OpenSSL-crypto.patch b/package/libspdm/0001-cryptlib_openssl-x509-Remove-internal-OpenSSL-crypto.patch
> new file mode 100644
> index 0000000000..420098be11
> --- /dev/null
> +++ b/package/libspdm/0001-cryptlib_openssl-x509-Remove-internal-OpenSSL-crypto.patch
> @@ -0,0 +1,43 @@
> +From 7db883cdb3369cfaf9f0890b0eda503f47a5ffa3 Mon Sep 17 00:00:00 2001
> +From: Alistair Francis <alistair.francis@wdc.com>
> +Date: Fri, 11 Aug 2023 16:26:53 -0400
> +Subject: [PATCH] cryptlib_openssl: x509: Remove internal OpenSSL crypto
> + include
> +
> +The OpenSSL source code describes the crypto include as:
> +"Internal EC functions for other submodules: not for application use"
> + - https://github.com/openssl/openssl/blob/master/include/crypto/ec.h
> +
> +Using the internal APIS makes it difficult to use libspdm as a library
> +with other packages. So let's remove the uses of the internal API and
> +instead use the public API.
> +

Please add an "Upstream:" tag here and for all patches. Make sure to
run "make check-package" before submitting a patch, it will catch such
issues.

Here are the upstream tags:

0001-cryptlib_openssl-x509-Remove-internal-OpenSSL-crypto.patch:Upstream: https://github.com/DMTF/libspdm/commit/7db883cdb3369cfaf9f0890b0eda503f47a5ffa3
0002-cryptlib_openssl-ecd-Allow-disabling-code.patch:Upstream: https://github.com/DMTF/libspdm/commit/e87687d72688e980b929920b7d77dca26fff169e
0003-cryptlib_openssl-ec-Remove-internal-OpenSSL-crypto-i.patch:Upstream: https://github.com/DMTF/libspdm/commit/567b1c8ea731fe42650d43ede50a105b772dc7aa
0004-CMakeLists.txt-Allow-disabling-EDDSA-support-from-co.patch:Upstream: https://github.com/DMTF/libspdm/pull/2330


> diff --git a/package/libspdm/Config.in b/package/libspdm/Config.in
> new file mode 100644
> index 0000000000..2d0f46da85
> --- /dev/null
> +++ b/package/libspdm/Config.in
> @@ -0,0 +1,19 @@
> +config BR2_PACKAGE_LIBSPDM
> +	bool "libspdm"
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_OPENSSL_FORCE_LIBOPENSSL
> +	help
> +	  libspdm is a sample implementation that follows
> +	  the DMTF SPDM specifications
> +
> +	  https://github.com/DMTF/libspdm
> +
> +config BR2_PACKAGE_LIBSPDM_CPU_FAMILLY

FAMILY, not FAMILLY.

> +	string
> +	default "arc" if BR2_arcle || BR2_arceb
> +	default "arm" if BR2_arm || BR2_armeb
> +	default "aarch64" if BR2_aarch64 || BR2_aarch64_be
> +	default "ia32" if BR2_i386
> +	default "riscv32" if BR2_riscv && BR2_RISCV_32
> +	default "riscv64" if BR2_riscv && BR2_RISCV_64
> +	default "x64" if BR2_x86_64

Also, use this to provide an ARCH_SUPPORTS variable. Like this:

config BR2_PACKAGE_LIBSPDM_CPU_FAMILY
        string
        default "arc"           if BR2_arcle || BR2_arceb
        default "arm"           if BR2_arm || BR2_armeb
        default "aarch64"       if BR2_aarch64 || BR2_aarch64_be
        default "ia32"          if BR2_i386
        default "riscv32"       if BR2_riscv && BR2_RISCV_32
        default "riscv64"       if BR2_riscv && BR2_RISCV_64
        default "x64"           if BR2_x86_64

config BR2_PACKAGE_LIBSPDM_ARCH_SUPPORTS
        bool
        default y if BR2_PACKAGE_LIBSPDM_CPU_FAMILY != ""

config BR2_PACKAGE_LIBSPDM
        bool "libspdm"
        depends on BR2_PACKAGE_LIBSPDM_ARCH_SUPPORTS
        select BR2_PACKAGE_OPENSSL
        select BR2_PACKAGE_OPENSSL_FORCE_LIBOPENSSL
        help
          libspdm is a sample implementation that follows
          the DMTF SPDM specifications

          https://github.com/DMTF/libspdm

However, here is the problem: it doesn't build on ARM. Indeed, while
libspdm itself is OK with building on ARM, its OpenSSL backend is not,
causing this build failure:

CMake Error at os_stub/cryptlib_openssl/CMakeLists.txt:25 (MESSAGE):
  Unknown ARCH

Two solutions here:

(1) Only support the architectures that are supported by the OpenSSL
    backend

(2) Also support the mbedtls backend, with the appropriate architecture
    dependencies.


> +LIBSPDM_INSTALL_STAGING = YES
> +
> +LIBSPDM_DEPENDENCIES = openssl
> +
> +LIBSPDM_TARGET_CPU_FAMILY = $(call qstrip,$(BR2_PACKAGE_LIBSPDM_CPU_FAMILLY))
                                                                       ^^^^^^^ FAMILY

> +define LIBSPDM_INSTALL_STAGING_CMDS
> +	$(INSTALL) -m 0755 -t $(STAGING_DIR)/usr/lib/ $(@D)/lib/*
> +
> +	mkdir -p $(STAGING_DIR)/usr/include/libspdm/
> +	cp -dpfr $(@D)/include/* $(STAGING_DIR)/usr/include/libspdm/
> +
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/os_stub/spdm_crypt_ext_lib
> +	$(INSTALL) -D -m 0755 $(@D)/os_stub/spdm_crypt_ext_lib/*.h $(STAGING_DIR)/usr/include/libspdm/os_stub/spdm_crypt_ext_lib

Please be consistent: always create the directory with mkdir -p, always
copy the files with cp -dpfr. So:

define LIBSPDM_INSTALL_STAGING_CMDS
        mkdir -p $(STAGING_DIR)/usr/lib
        cp -dpfr $(@D)/lib/* $(STAGING_DIR)/usr/lib/

        mkdir -p $(STAGING_DIR)/usr/include/libspdm/
        cp -dpfr $(@D)/include/* $(STAGING_DIR)/usr/include/libspdm/

        mkdir -p $(STAGING_DIR)/usr/include/libspdm/os_stub/spdm_crypt_ext_lib
        cp -dpfr $(@D)/os_stub/spdm_crypt_ext_lib/*.h \
                $(STAGING_DIR)/usr/include/libspdm/os_stub/spdm_crypt_ext_lib/
endef

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot