[Buildroot] [PATCH v2 1/2] package/libopenssl: Copy the crypto include files

[Buildroot] [PATCH v2 1/2] package/libopenssl: Copy the crypto include files

[Alistair Francis]

libspdm uses the low level OpenSSL crypto APIs.

In a traditional build libspdm has OpenSSL as a submodule and gets the
include from there, when building with buildroot we want to use the
prebuilt OpenSSL library, so we need to expose these crypto includes.

This patch installs the crypto private headers so that libspdm can use
them.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 package/libopenssl/libopenssl.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/libopenssl/libopenssl.mk b/package/libopenssl/libopenssl.mk
index 06eac8f3d9..c66a842cdf 100644
--- a/package/libopenssl/libopenssl.mk
+++ b/package/libopenssl/libopenssl.mk
@@ -126,6 +126,10 @@ endef
 
 define LIBOPENSSL_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
+	mkdir -p $(STAGING_DIR)/usr/include/crypto
+	$(INSTALL) -m 644 $(@D)/include/crypto/* $(STAGING_DIR)/usr/include/crypto/
+	mkdir -p $(STAGING_DIR)/usr/include/internal
+	$(INSTALL) -m 644 $(@D)/include/internal/* $(STAGING_DIR)/usr/include/internal/
 endef
 
 define HOST_LIBOPENSSL_INSTALL_CMDS
-- 
2.40.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 2/2] package/libspdm: Initial commit

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 package/Config.in            |  1 +
 package/libspdm/Config.in    | 20 ++++++++++++
 package/libspdm/libspdm.hash |  3 ++
 package/libspdm/libspdm.mk   | 59 ++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+)
 create mode 100644 package/libspdm/Config.in
 create mode 100644 package/libspdm/libspdm.hash
 create mode 100644 package/libspdm/libspdm.mk

diff --git a/package/Config.in b/package/Config.in
index 1e551d17c4..bc7e54113c 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -192,6 +192,7 @@ menu "Development tools"
 	source "package/jo/Config.in"
 	source "package/jq/Config.in"
 	source "package/libtool/Config.in"
+	source "package/libspdm/Config.in"
 	source "package/make/Config.in"
 	source "package/mawk/Config.in"
 	source "package/patch/Config.in"
diff --git a/package/libspdm/Config.in b/package/libspdm/Config.in
new file mode 100644
index 0000000000..b732662842
--- /dev/null
+++ b/package/libspdm/Config.in
@@ -0,0 +1,20 @@
+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
+	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
+	default BR2_ARCH  # Yes, this should work...\
diff --git a/package/libspdm/libspdm.hash b/package/libspdm/libspdm.hash
new file mode 100644
index 0000000000..d06af29397
--- /dev/null
+++ b/package/libspdm/libspdm.hash
@@ -0,0 +1,3 @@
+# Locally calculated
+sha256  3a40daa59f32843062c3d2699acee09bd0ee217eb8ebf0378ae12b60b6db0636  libspdm-3.0.0.tar.gz
+sha256  337130631a714eeae017556cad101d5324c2961214120b6214741d3d43667086  LICENSE.md
diff --git a/package/libspdm/libspdm.mk b/package/libspdm/libspdm.mk
new file mode 100644
index 0000000000..daf6639808
--- /dev/null
+++ b/package/libspdm/libspdm.mk
@@ -0,0 +1,59 @@
+################################################################################
+#
+# libspdm
+#
+################################################################################
+
+LIBSPDM_VERSION = 3.0.0
+LIBSPDM_SITE = $(call github,DMTF,libspdm,$(LIBSPDM_VERSION))
+LIBSPDM_LICENSE = BSD-3-Clause
+LIBSPDM_LICENSE_FILES = LICENSE.md
+
+LIBSPDM_INSTALL_STAGING = YES
+LIBSPDM_INSTALL_TARGET = YES
+
+LIBSPDM_DEPENDENCIES += openssl
+
+LIBSPDM_TARGET_CPU_FAMILY = $(call qstrip,$(BR2_PACKAGE_LIBSPDM_CPU_FAMILLY))
+
+LIBSPDM_CONF_OPTS = \
+	-DARCH=$(LIBSPDM_TARGET_CPU_FAMILY) \
+	-DTOOLCHAIN=NONE \
+	-DTARGET=Release \
+	-DCRYPTO=openssl \
+	-DENABLE_BINARY_BUILD=1 \
+	-DCOMPILED_LIBCRYPTO_PATH=/usr/lib/ \
+	-DCOMPILED_LIBSSL_PATH=/usr/lib/ \
+	-DDISABLE_TESTS=1 \
+	-DLINK_FLAGS=$(STAGING_DIR)
+
+define LIBSPDM_INSTALL_STAGING_CMDS
+	$(INSTALL) -m 0755 -t $(STAGING_DIR)/usr/lib/ $(@D)/lib/*
+
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/library
+	$(INSTALL) -D -m 0755 $(@D)/include/library/* $(STAGING_DIR)/usr/include/libspdm/library
+
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/internal
+	$(INSTALL) -D -m 0755 $(@D)/include/internal/*.h $(STAGING_DIR)/usr/include/libspdm/internal/
+
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/cryptlib
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/requester
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/responder
+	$(INSTALL) -D -m 0755 $(@D)/include/hal/*.h $(STAGING_DIR)/usr/include/libspdm/hal/
+	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/*.h $(STAGING_DIR)/usr/include/libspdm/hal/library/
+	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/cryptlib/* $(STAGING_DIR)/usr/include/libspdm/hal/library/cryptlib/
+	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/requester/* $(STAGING_DIR)/usr/include/libspdm/hal/library/requester/
+	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/responder/* $(STAGING_DIR)/usr/include/libspdm/hal/library/responder/
+
+	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/industry_standard
+	$(INSTALL) -D -m 0755 $(@D)/include/industry_standard/* $(STAGING_DIR)/usr/include/libspdm/industry_standard
+
+	$(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
+endef
+
+define LIBSPDM_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/lib/* $(TARGET_DIR)/usr/lib/
+endef
+
+$(eval $(cmake-package))
-- 
2.40.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/2] package/libopenssl: Copy the crypto include files

[Thomas Petazzoni via buildroot]

On Mon, 24 Jul 2023 11:48:31 +1000
Alistair Francis <alistair23@gmail.com> wrote:

> diff --git a/package/libopenssl/libopenssl.mk b/package/libopenssl/libopenssl.mk
> index 06eac8f3d9..c66a842cdf 100644
> --- a/package/libopenssl/libopenssl.mk
> +++ b/package/libopenssl/libopenssl.mk
> @@ -126,6 +126,10 @@ endef
>  
>  define LIBOPENSSL_INSTALL_STAGING_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
> +	mkdir -p $(STAGING_DIR)/usr/include/crypto
> +	$(INSTALL) -m 644 $(@D)/include/crypto/* $(STAGING_DIR)/usr/include/crypto/
> +	mkdir -p $(STAGING_DIR)/usr/include/internal
> +	$(INSTALL) -m 644 $(@D)/include/internal/* $(STAGING_DIR)/usr/include/internal/

I'm still quite concerned about this. Why the heck does libspdm require
the internal/private headers of OpenSSL, and no other package needs
that? It seems very odd, and potentially very wrong. One can assume
such internal headers expose internal APIs that don't have the same
backward compatibility guarantees as public APIs.

What makes libspdm so special? Is there any potential side effect of
exposing those OpenSSL private/internal headers to the wider world, as
your patch does?

Best regards,

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

Re: [Buildroot] [PATCH v2 2/2] package/libspdm: Initial commit

[Thomas Petazzoni via buildroot]

Hello,

On Mon, 24 Jul 2023 11:48:32 +1000
Alistair Francis <alistair23@gmail.com> wrote:

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Commit title should be:

	package/libspdm: new package

> ---
>  package/Config.in            |  1 +
>  package/libspdm/Config.in    | 20 ++++++++++++
>  package/libspdm/libspdm.hash |  3 ++
>  package/libspdm/libspdm.mk   | 59 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+)
>  create mode 100644 package/libspdm/Config.in
>  create mode 100644 package/libspdm/libspdm.hash
>  create mode 100644 package/libspdm/libspdm.mk

A new entry in the DEVELOPERS file is needed.

> diff --git a/package/libspdm/Config.in b/package/libspdm/Config.in
> new file mode 100644
> index 0000000000..b732662842
> --- /dev/null
> +++ b/package/libspdm/Config.in
> @@ -0,0 +1,20 @@
> +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
> +	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
> +	default BR2_ARCH  # Yes, this should work...\

Are all CPU families really supported? Usually, when a package needs to
know the CPU family/platform, it is a very strong sign that the package
has some architecture-specific stuff, and that some of our "special"
architectures will not be supported.

According to the CMakeLists.txt, I'm pretty correct:


if(ARCH STREQUAL "x64")
    MESSAGE("ARCH = x64")
elseif(ARCH STREQUAL "ia32")
    MESSAGE("ARCH = ia32")
elseif(ARCH STREQUAL "arm")
    MESSAGE("ARCH = arm")
elseif(ARCH STREQUAL "aarch64")
    MESSAGE("ARCH = aarch64")
elseif(ARCH STREQUAL "riscv32")
    MESSAGE("ARCH = riscv32")
elseif(ARCH STREQUAL "riscv64")
    MESSAGE("ARCH = riscv64")
elseif(ARCH STREQUAL "arc")
    MESSAGE("ARCH = arc")
elseif(ARCH STREQUAL "nios2")
    MESSAGE("ARCH = nios2")
else()
    MESSAGE(FATAL_ERROR "Unknown ARCH")
endif()

Your package will fail to build on any other architectures than the
ones listed here.

> diff --git a/package/libspdm/libspdm.mk b/package/libspdm/libspdm.mk
> new file mode 100644
> index 0000000000..daf6639808
> --- /dev/null
> +++ b/package/libspdm/libspdm.mk
> @@ -0,0 +1,59 @@
> +################################################################################
> +#
> +# libspdm
> +#
> +################################################################################
> +
> +LIBSPDM_VERSION = 3.0.0
> +LIBSPDM_SITE = $(call github,DMTF,libspdm,$(LIBSPDM_VERSION))
> +LIBSPDM_LICENSE = BSD-3-Clause
> +LIBSPDM_LICENSE_FILES = LICENSE.md
> +
> +LIBSPDM_INSTALL_STAGING = YES
> +LIBSPDM_INSTALL_TARGET = YES

This line is not needed, it's the default.

> +
> +LIBSPDM_DEPENDENCIES += openssl

Just =, not +=.

> +
> +LIBSPDM_TARGET_CPU_FAMILY = $(call qstrip,$(BR2_PACKAGE_LIBSPDM_CPU_FAMILLY))
> +
> +LIBSPDM_CONF_OPTS = \
> +	-DARCH=$(LIBSPDM_TARGET_CPU_FAMILY) \
> +	-DTOOLCHAIN=NONE \
> +	-DTARGET=Release \
> +	-DCRYPTO=openssl \
> +	-DENABLE_BINARY_BUILD=1 \
> +	-DCOMPILED_LIBCRYPTO_PATH=/usr/lib/ \
> +	-DCOMPILED_LIBSSL_PATH=/usr/lib/ \
> +	-DDISABLE_TESTS=1 \
> +	-DLINK_FLAGS=$(STAGING_DIR)
> +
> +define LIBSPDM_INSTALL_STAGING_CMDS
> +	$(INSTALL) -m 0755 -t $(STAGING_DIR)/usr/lib/ $(@D)/lib/*
> +
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/library
> +	$(INSTALL) -D -m 0755 $(@D)/include/library/* $(STAGING_DIR)/usr/include/libspdm/library
> +
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/internal
> +	$(INSTALL) -D -m 0755 $(@D)/include/internal/*.h $(STAGING_DIR)/usr/include/libspdm/internal/
> +
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/cryptlib
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/requester
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/hal/library/responder
> +	$(INSTALL) -D -m 0755 $(@D)/include/hal/*.h $(STAGING_DIR)/usr/include/libspdm/hal/
> +	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/*.h $(STAGING_DIR)/usr/include/libspdm/hal/library/
> +	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/cryptlib/* $(STAGING_DIR)/usr/include/libspdm/hal/library/cryptlib/
> +	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/requester/* $(STAGING_DIR)/usr/include/libspdm/hal/library/requester/
> +	$(INSTALL) -D -m 0755 $(@D)/include/hal/library/responder/* $(STAGING_DIR)/usr/include/libspdm/hal/library/responder/
> +
> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/libspdm/industry_standard
> +	$(INSTALL) -D -m 0755 $(@D)/include/industry_standard/* $(STAGING_DIR)/usr/include/libspdm/industry_standard
> +
> +	$(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

This seems very verbose. If you're copying all files/directories of
$(@D)/include/* to $(STAGING_DIR)/usr/include/libspdm, why not:

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

> +endef
> +
> +define LIBSPDM_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/lib/* $(TARGET_DIR)/usr/lib/

This is not good, as it could create a file called "lib" in
$(TARGET_DIR)/usr if $(TARGET_DIR)/usr/lib doesn't already exist. So:

	mkdir -p $(TARGET_DIR)/usr/lib
	cp -dpfr $(@D)/lib/* $(TARGET_DIR)/usr/lib/

Thanks,

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

Re: [Buildroot] [PATCH v2 1/2] package/libopenssl: Copy the crypto include files

[Alistair Francis]

On Sun, Aug 6, 2023 at 9:49 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 24 Jul 2023 11:48:31 +1000
> Alistair Francis <alistair23@gmail.com> wrote:
>
> > diff --git a/package/libopenssl/libopenssl.mk b/package/libopenssl/libopenssl.mk
> > index 06eac8f3d9..c66a842cdf 100644
> > --- a/package/libopenssl/libopenssl.mk
> > +++ b/package/libopenssl/libopenssl.mk
> > @@ -126,6 +126,10 @@ endef
> >
> >  define LIBOPENSSL_INSTALL_STAGING_CMDS
> >       $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
> > +     mkdir -p $(STAGING_DIR)/usr/include/crypto
> > +     $(INSTALL) -m 644 $(@D)/include/crypto/* $(STAGING_DIR)/usr/include/crypto/
> > +     mkdir -p $(STAGING_DIR)/usr/include/internal
> > +     $(INSTALL) -m 644 $(@D)/include/internal/* $(STAGING_DIR)/usr/include/internal/
>
> I'm still quite concerned about this. Why the heck does libspdm require
> the internal/private headers of OpenSSL, and no other package needs
> that? It seems very odd, and potentially very wrong. One can assume
> such internal headers expose internal APIs that don't have the same
> backward compatibility guarantees as public APIs.

Yeah, you are right. I'm going to work with upstream libspdm to remove
the requirement on these and then I'll try again.

Alistair

>
> What makes libspdm so special? Is there any potential side effect of
> exposing those OpenSSL private/internal headers to the wider world, as
> your patch does?
>
> Best regards,
>
> 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