All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools: Use a single target-independent config to enable OpenSSL
@ 2021-05-24 20:23 Alexandru Gagniuc
  2021-06-15 23:34 ` AKASHI Takahiro
  2021-06-22 13:31 ` Simon Glass
  0 siblings, 2 replies; 8+ messages in thread
From: Alexandru Gagniuc @ 2021-05-24 20:23 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, trini, Alexandru Gagniuc

Host tool features, such as mkimage's ability to sign FIT images were
enabled or disabled based on the target configuration. However, this
misses the point of a target-agnostic host tool.

A target's ability to verify FIT signatures is independent of
mkimage's ability to create those signatures. In fact, u-boot's build
system doesn't sign images. The target code can be successfully built
without relying on any ability to sign such code.

Conversely, mkimage's ability to sign images does not require that
those images will only work on targets which support FIT verification.
Linking mkimage cryptographic features to target support for FIT
verification is misguided.

Without loss of generality, we can say that host features are and
should be independent of target features.

While we prefer that a host tool always supports the same feature set,
we recognize the following
  - some users prefer to build u-boot without a dependency on OpenSSL.
  - some distros prefer to ship mkimage without linking to OpenSSL

To allow these use cases, introduce a host-only Kconfig which is used
to select or deselect libcrypto support. Some mkimage features or some
host tools might not be available, but this shouldn't affect the
u-boot build.

I also considered setting the default of this config based on
FIT_SIGNATURE. While it would preserve the old behaviour it's also
contrary to the goals of this change. I decided to enable it by
default, so that the default build yields the most feature-complete
mkimage.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---

This patch is designed to apply on top of
    [PATCH v2 00/18] image: Reduce #ifdef abuse in image code



 tools/Kconfig  | 11 +++++++++++
 tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/tools/Kconfig b/tools/Kconfig
index b2f5012240..214932ae30 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
 	  some cases the system dtc may not support all required features
 	  and the path to a different version should be given here.
 
+config TOOLS_USE_LIBCRYPTO
+	bool "Use OpenSSL's libcrypto library for host tools"
+	default y
+	help
+	  Cryptographic signature, verification, and encryption of images is
+	  provided by host tools using OpenSSL's libcrypto. Select 'n' here if
+	  you wish to build host tools without OpenSSL. mkimage will not have
+	  the ability to sign images.
+	  This selection does not affect target features, such as runtime FIT
+	  signature verification.
+
 endmenu
diff --git a/tools/Makefile b/tools/Makefile
index 722355e984..1f30a06cce 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -3,6 +3,25 @@
 # (C) Copyright 2000-2006
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
 
+# A note on target vs host configuration:
+#
+# Host tools can be used across multiple targets, or different configurations
+# of the same target. Thus, host tools must be able to handle any combination
+# of target configurations. To prevent having different variations of the same
+# tool, the tool build options may not depend on target configuration.
+#
+# Some linux distributions package these utilities as u-boot-tools, and it
+# would be unmaintainable to have a different tool variation for each
+# arch or configuration.
+#
+# A couple of simple rules:
+#
+# 1) Do not use target CONFIG_* options to enable or disable features in host
+#    tools. Only use the configs from tools/Kconfig
+# 2) It's okay to use target configs to disable building specific tools.
+#    That's as long as the features of those tools aren't modified.
+#
+
 # Enable all the config-independent tools
 ifneq ($(HOST_TOOLS_ALL),)
 CONFIG_ARCH_KIRKWOOD = y
@@ -53,30 +72,30 @@ hostprogs-y += mkenvimage
 mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
-hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_USE_LIBCRYPTO) += fit_info fit_check_sign
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
 FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
-FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o common/image-fit-sig.o
-FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
+FIT_SIG_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := image-sig-host.o common/image-fit-sig.o
+FIT_CIPHER_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := common/image-cipher.o
 
 # The following files are synced with upstream DTC.
 # Use synced versions from scripts/dtc/libfdt/.
 LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
 		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
 
-RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
+RSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/rsa/, \
 					rsa-sign.o rsa-verify.o \
 					rsa-mod-exp.o)
 
-ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
+ECDSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
 
-AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
+AES_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/aes/, \
 					aes-encrypt.o aes-decrypt.o)
 
 # Cryptographic helpers that depend on openssl/libcrypto
-LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
+LIBCRYPTO_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/, \
 					fdt-libcrypto.o)
 
 ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
@@ -136,22 +155,17 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
 file2include-objs := file2include.o
 
-ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
+ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
 # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
 # the mxsimage support within tools/mxsimage.c .
 HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
 endif
 
-ifdef CONFIG_FIT_SIGNATURE
+ifdef CONFIG_TOOLS_USE_LIBCRYPTO
 # This affects include/image.h, but including the board config file
 # is tricky, so manually define this options here.
 HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
-HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
-endif
-
-ifdef CONFIG_FIT_CIPHER
-# This affects include/image.h, but including the board config file
-# is tricky, so manually define this options here.
+HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
 HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
 endif
 
@@ -164,7 +178,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
 endif
 
 # MXSImage needs LibSSL
-ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
+ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
 HOSTCFLAGS_kwbimage.o += \
 	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
 HOSTLDLIBS_mkimage += \
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-05-24 20:23 [PATCH] tools: Use a single target-independent config to enable OpenSSL Alexandru Gagniuc
@ 2021-06-15 23:34 ` AKASHI Takahiro
  2021-06-16 14:34   ` Alex G.
  2021-06-22 13:31 ` Simon Glass
  1 sibling, 1 reply; 8+ messages in thread
From: AKASHI Takahiro @ 2021-06-15 23:34 UTC (permalink / raw)
  To: Alexandru Gagniuc; +Cc: u-boot, sjg, trini

A gentle ping.
What is the current review status?
Who will take care of this patch?

-Takahiro Akashi

On Mon, May 24, 2021 at 03:23:17PM -0500, Alexandru Gagniuc wrote:
> Host tool features, such as mkimage's ability to sign FIT images were
> enabled or disabled based on the target configuration. However, this
> misses the point of a target-agnostic host tool.
> 
> A target's ability to verify FIT signatures is independent of
> mkimage's ability to create those signatures. In fact, u-boot's build
> system doesn't sign images. The target code can be successfully built
> without relying on any ability to sign such code.
> 
> Conversely, mkimage's ability to sign images does not require that
> those images will only work on targets which support FIT verification.
> Linking mkimage cryptographic features to target support for FIT
> verification is misguided.
> 
> Without loss of generality, we can say that host features are and
> should be independent of target features.
> 
> While we prefer that a host tool always supports the same feature set,
> we recognize the following
>   - some users prefer to build u-boot without a dependency on OpenSSL.
>   - some distros prefer to ship mkimage without linking to OpenSSL
> 
> To allow these use cases, introduce a host-only Kconfig which is used
> to select or deselect libcrypto support. Some mkimage features or some
> host tools might not be available, but this shouldn't affect the
> u-boot build.
> 
> I also considered setting the default of this config based on
> FIT_SIGNATURE. While it would preserve the old behaviour it's also
> contrary to the goals of this change. I decided to enable it by
> default, so that the default build yields the most feature-complete
> mkimage.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> 
> This patch is designed to apply on top of
>     [PATCH v2 00/18] image: Reduce #ifdef abuse in image code
> 
> 
> 
>  tools/Kconfig  | 11 +++++++++++
>  tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
>  2 files changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/Kconfig b/tools/Kconfig
> index b2f5012240..214932ae30 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
>  	  some cases the system dtc may not support all required features
>  	  and the path to a different version should be given here.
>  
> +config TOOLS_USE_LIBCRYPTO
> +	bool "Use OpenSSL's libcrypto library for host tools"
> +	default y
> +	help
> +	  Cryptographic signature, verification, and encryption of images is
> +	  provided by host tools using OpenSSL's libcrypto. Select 'n' here if
> +	  you wish to build host tools without OpenSSL. mkimage will not have
> +	  the ability to sign images.
> +	  This selection does not affect target features, such as runtime FIT
> +	  signature verification.
> +
>  endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 722355e984..1f30a06cce 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -3,6 +3,25 @@
>  # (C) Copyright 2000-2006
>  # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>  
> +# A note on target vs host configuration:
> +#
> +# Host tools can be used across multiple targets, or different configurations
> +# of the same target. Thus, host tools must be able to handle any combination
> +# of target configurations. To prevent having different variations of the same
> +# tool, the tool build options may not depend on target configuration.
> +#
> +# Some linux distributions package these utilities as u-boot-tools, and it
> +# would be unmaintainable to have a different tool variation for each
> +# arch or configuration.
> +#
> +# A couple of simple rules:
> +#
> +# 1) Do not use target CONFIG_* options to enable or disable features in host
> +#    tools. Only use the configs from tools/Kconfig
> +# 2) It's okay to use target configs to disable building specific tools.
> +#    That's as long as the features of those tools aren't modified.
> +#
> +
>  # Enable all the config-independent tools
>  ifneq ($(HOST_TOOLS_ALL),)
>  CONFIG_ARCH_KIRKWOOD = y
> @@ -53,30 +72,30 @@ hostprogs-y += mkenvimage
>  mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
>  
>  hostprogs-y += dumpimage mkimage
> -hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
> +hostprogs-$(CONFIG_TOOLS_USE_LIBCRYPTO) += fit_info fit_check_sign
>  
>  hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>  
>  FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o common/image-fit-sig.o
> -FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
> +FIT_SIG_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := image-sig-host.o common/image-fit-sig.o
> +FIT_CIPHER_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := common/image-cipher.o
>  
>  # The following files are synced with upstream DTC.
>  # Use synced versions from scripts/dtc/libfdt/.
>  LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
>  		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
>  
> -RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
> +RSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/rsa/, \
>  					rsa-sign.o rsa-verify.o \
>  					rsa-mod-exp.o)
>  
> -ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> +ECDSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
>  
> -AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
> +AES_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/aes/, \
>  					aes-encrypt.o aes-decrypt.o)
>  
>  # Cryptographic helpers that depend on openssl/libcrypto
> -LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
> +LIBCRYPTO_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/, \
>  					fdt-libcrypto.o)
>  
>  ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> @@ -136,22 +155,17 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
>  fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
>  file2include-objs := file2include.o
>  
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
>  # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
>  # the mxsimage support within tools/mxsimage.c .
>  HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
>  endif
>  
> -ifdef CONFIG_FIT_SIGNATURE
> +ifdef CONFIG_TOOLS_USE_LIBCRYPTO
>  # This affects include/image.h, but including the board config file
>  # is tricky, so manually define this options here.
>  HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
> -HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
> -endif
> -
> -ifdef CONFIG_FIT_CIPHER
> -# This affects include/image.h, but including the board config file
> -# is tricky, so manually define this options here.
> +HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>  HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>  endif
>  
> @@ -164,7 +178,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
>  endif
>  
>  # MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
>  HOSTCFLAGS_kwbimage.o += \
>  	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>  HOSTLDLIBS_mkimage += \
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-06-15 23:34 ` AKASHI Takahiro
@ 2021-06-16 14:34   ` Alex G.
  2021-06-16 16:08     ` Tom Rini
  0 siblings, 1 reply; 8+ messages in thread
From: Alex G. @ 2021-06-16 14:34 UTC (permalink / raw)
  To: AKASHI Takahiro, u-boot, sjg, trini

On 6/15/21 6:34 PM, AKASHI Takahiro wrote:
> A gentle ping.
> What is the current review status?
> Who will take care of this patch?

Patchwork automatically delegates this to a maintainer [1], but anyone 
is welcome to comment and review.

Alex

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20210524202317.1492578-1-mr.nuke.me@gmail.com/

> -Takahiro Akashi
> 
> On Mon, May 24, 2021 at 03:23:17PM -0500, Alexandru Gagniuc wrote:
>> Host tool features, such as mkimage's ability to sign FIT images were
>> enabled or disabled based on the target configuration. However, this
>> misses the point of a target-agnostic host tool.
>>
>> A target's ability to verify FIT signatures is independent of
>> mkimage's ability to create those signatures. In fact, u-boot's build
>> system doesn't sign images. The target code can be successfully built
>> without relying on any ability to sign such code.
>>
>> Conversely, mkimage's ability to sign images does not require that
>> those images will only work on targets which support FIT verification.
>> Linking mkimage cryptographic features to target support for FIT
>> verification is misguided.
>>
>> Without loss of generality, we can say that host features are and
>> should be independent of target features.
>>
>> While we prefer that a host tool always supports the same feature set,
>> we recognize the following
>>    - some users prefer to build u-boot without a dependency on OpenSSL.
>>    - some distros prefer to ship mkimage without linking to OpenSSL
>>
>> To allow these use cases, introduce a host-only Kconfig which is used
>> to select or deselect libcrypto support. Some mkimage features or some
>> host tools might not be available, but this shouldn't affect the
>> u-boot build.
>>
>> I also considered setting the default of this config based on
>> FIT_SIGNATURE. While it would preserve the old behaviour it's also
>> contrary to the goals of this change. I decided to enable it by
>> default, so that the default build yields the most feature-complete
>> mkimage.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> ---
>>
>> This patch is designed to apply on top of
>>      [PATCH v2 00/18] image: Reduce #ifdef abuse in image code
>>
>>
>>
>>   tools/Kconfig  | 11 +++++++++++
>>   tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
>>   2 files changed, 41 insertions(+), 16 deletions(-)
>>
>> diff --git a/tools/Kconfig b/tools/Kconfig
>> index b2f5012240..214932ae30 100644
>> --- a/tools/Kconfig
>> +++ b/tools/Kconfig
>> @@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
>>   	  some cases the system dtc may not support all required features
>>   	  and the path to a different version should be given here.
>>   
>> +config TOOLS_USE_LIBCRYPTO
>> +	bool "Use OpenSSL's libcrypto library for host tools"
>> +	default y
>> +	help
>> +	  Cryptographic signature, verification, and encryption of images is
>> +	  provided by host tools using OpenSSL's libcrypto. Select 'n' here if
>> +	  you wish to build host tools without OpenSSL. mkimage will not have
>> +	  the ability to sign images.
>> +	  This selection does not affect target features, such as runtime FIT
>> +	  signature verification.
>> +
>>   endmenu
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 722355e984..1f30a06cce 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -3,6 +3,25 @@
>>   # (C) Copyright 2000-2006
>>   # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>>   
>> +# A note on target vs host configuration:
>> +#
>> +# Host tools can be used across multiple targets, or different configurations
>> +# of the same target. Thus, host tools must be able to handle any combination
>> +# of target configurations. To prevent having different variations of the same
>> +# tool, the tool build options may not depend on target configuration.
>> +#
>> +# Some linux distributions package these utilities as u-boot-tools, and it
>> +# would be unmaintainable to have a different tool variation for each
>> +# arch or configuration.
>> +#
>> +# A couple of simple rules:
>> +#
>> +# 1) Do not use target CONFIG_* options to enable or disable features in host
>> +#    tools. Only use the configs from tools/Kconfig
>> +# 2) It's okay to use target configs to disable building specific tools.
>> +#    That's as long as the features of those tools aren't modified.
>> +#
>> +
>>   # Enable all the config-independent tools
>>   ifneq ($(HOST_TOOLS_ALL),)
>>   CONFIG_ARCH_KIRKWOOD = y
>> @@ -53,30 +72,30 @@ hostprogs-y += mkenvimage
>>   mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
>>   
>>   hostprogs-y += dumpimage mkimage
>> -hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
>> +hostprogs-$(CONFIG_TOOLS_USE_LIBCRYPTO) += fit_info fit_check_sign
>>   
>>   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>>   
>>   FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
>> -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o common/image-fit-sig.o
>> -FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
>> +FIT_SIG_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := image-sig-host.o common/image-fit-sig.o
>> +FIT_CIPHER_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := common/image-cipher.o
>>   
>>   # The following files are synced with upstream DTC.
>>   # Use synced versions from scripts/dtc/libfdt/.
>>   LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
>>   		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
>>   
>> -RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
>> +RSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/rsa/, \
>>   					rsa-sign.o rsa-verify.o \
>>   					rsa-mod-exp.o)
>>   
>> -ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
>> +ECDSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
>>   
>> -AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
>> +AES_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/aes/, \
>>   					aes-encrypt.o aes-decrypt.o)
>>   
>>   # Cryptographic helpers that depend on openssl/libcrypto
>> -LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
>> +LIBCRYPTO_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/, \
>>   					fdt-libcrypto.o)
>>   
>>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>> @@ -136,22 +155,17 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
>>   fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
>>   file2include-objs := file2include.o
>>   
>> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
>> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
>>   # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
>>   # the mxsimage support within tools/mxsimage.c .
>>   HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
>>   endif
>>   
>> -ifdef CONFIG_FIT_SIGNATURE
>> +ifdef CONFIG_TOOLS_USE_LIBCRYPTO
>>   # This affects include/image.h, but including the board config file
>>   # is tricky, so manually define this options here.
>>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
>> -HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
>> -endif
>> -
>> -ifdef CONFIG_FIT_CIPHER
>> -# This affects include/image.h, but including the board config file
>> -# is tricky, so manually define this options here.
>> +HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>>   endif
>>   
>> @@ -164,7 +178,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
>>   endif
>>   
>>   # MXSImage needs LibSSL
>> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
>> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
>>   HOSTCFLAGS_kwbimage.o += \
>>   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>>   HOSTLDLIBS_mkimage += \
>> -- 
>> 2.31.1
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-06-16 14:34   ` Alex G.
@ 2021-06-16 16:08     ` Tom Rini
  2021-06-16 22:28       ` AKASHI Takahiro
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Rini @ 2021-06-16 16:08 UTC (permalink / raw)
  To: Alex G.; +Cc: AKASHI Takahiro, u-boot, sjg

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

On Wed, Jun 16, 2021 at 09:34:27AM -0500, Alex G. wrote:
> On 6/15/21 6:34 PM, AKASHI Takahiro wrote:
> > A gentle ping.
> > What is the current review status?
> > Who will take care of this patch?
> 
> Patchwork automatically delegates this to a maintainer [1], but anyone is
> welcome to comment and review.

Note that it's not automatic, I typically do it for anything that hasn't
been picked up by a custodian already.

I think the patch in question is fine, and since I think the series it
depends on is also fine with Simon, I'll be picking it up either for
next or after release.

> 
> Alex
> 
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20210524202317.1492578-1-mr.nuke.me@gmail.com/
> 
> > -Takahiro Akashi
> > 
> > On Mon, May 24, 2021 at 03:23:17PM -0500, Alexandru Gagniuc wrote:
> > > Host tool features, such as mkimage's ability to sign FIT images were
> > > enabled or disabled based on the target configuration. However, this
> > > misses the point of a target-agnostic host tool.
> > > 
> > > A target's ability to verify FIT signatures is independent of
> > > mkimage's ability to create those signatures. In fact, u-boot's build
> > > system doesn't sign images. The target code can be successfully built
> > > without relying on any ability to sign such code.
> > > 
> > > Conversely, mkimage's ability to sign images does not require that
> > > those images will only work on targets which support FIT verification.
> > > Linking mkimage cryptographic features to target support for FIT
> > > verification is misguided.
> > > 
> > > Without loss of generality, we can say that host features are and
> > > should be independent of target features.
> > > 
> > > While we prefer that a host tool always supports the same feature set,
> > > we recognize the following
> > >    - some users prefer to build u-boot without a dependency on OpenSSL.
> > >    - some distros prefer to ship mkimage without linking to OpenSSL
> > > 
> > > To allow these use cases, introduce a host-only Kconfig which is used
> > > to select or deselect libcrypto support. Some mkimage features or some
> > > host tools might not be available, but this shouldn't affect the
> > > u-boot build.
> > > 
> > > I also considered setting the default of this config based on
> > > FIT_SIGNATURE. While it would preserve the old behaviour it's also
> > > contrary to the goals of this change. I decided to enable it by
> > > default, so that the default build yields the most feature-complete
> > > mkimage.
> > > 
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > ---
> > > 
> > > This patch is designed to apply on top of
> > >      [PATCH v2 00/18] image: Reduce #ifdef abuse in image code
> > > 
> > > 
> > > 
> > >   tools/Kconfig  | 11 +++++++++++
> > >   tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
> > >   2 files changed, 41 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/tools/Kconfig b/tools/Kconfig
> > > index b2f5012240..214932ae30 100644
> > > --- a/tools/Kconfig
> > > +++ b/tools/Kconfig
> > > @@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
> > >   	  some cases the system dtc may not support all required features
> > >   	  and the path to a different version should be given here.
> > > +config TOOLS_USE_LIBCRYPTO
> > > +	bool "Use OpenSSL's libcrypto library for host tools"
> > > +	default y
> > > +	help
> > > +	  Cryptographic signature, verification, and encryption of images is
> > > +	  provided by host tools using OpenSSL's libcrypto. Select 'n' here if
> > > +	  you wish to build host tools without OpenSSL. mkimage will not have
> > > +	  the ability to sign images.
> > > +	  This selection does not affect target features, such as runtime FIT
> > > +	  signature verification.
> > > +
> > >   endmenu
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index 722355e984..1f30a06cce 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -3,6 +3,25 @@
> > >   # (C) Copyright 2000-2006
> > >   # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> > > +# A note on target vs host configuration:
> > > +#
> > > +# Host tools can be used across multiple targets, or different configurations
> > > +# of the same target. Thus, host tools must be able to handle any combination
> > > +# of target configurations. To prevent having different variations of the same
> > > +# tool, the tool build options may not depend on target configuration.
> > > +#
> > > +# Some linux distributions package these utilities as u-boot-tools, and it
> > > +# would be unmaintainable to have a different tool variation for each
> > > +# arch or configuration.
> > > +#
> > > +# A couple of simple rules:
> > > +#
> > > +# 1) Do not use target CONFIG_* options to enable or disable features in host
> > > +#    tools. Only use the configs from tools/Kconfig
> > > +# 2) It's okay to use target configs to disable building specific tools.
> > > +#    That's as long as the features of those tools aren't modified.
> > > +#
> > > +
> > >   # Enable all the config-independent tools
> > >   ifneq ($(HOST_TOOLS_ALL),)
> > >   CONFIG_ARCH_KIRKWOOD = y
> > > @@ -53,30 +72,30 @@ hostprogs-y += mkenvimage
> > >   mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
> > >   hostprogs-y += dumpimage mkimage
> > > -hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
> > > +hostprogs-$(CONFIG_TOOLS_USE_LIBCRYPTO) += fit_info fit_check_sign
> > >   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
> > >   FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> > > -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o common/image-fit-sig.o
> > > -FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
> > > +FIT_SIG_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := image-sig-host.o common/image-fit-sig.o
> > > +FIT_CIPHER_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := common/image-cipher.o
> > >   # The following files are synced with upstream DTC.
> > >   # Use synced versions from scripts/dtc/libfdt/.
> > >   LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
> > >   		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
> > > -RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
> > > +RSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/rsa/, \
> > >   					rsa-sign.o rsa-verify.o \
> > >   					rsa-mod-exp.o)
> > > -ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> > > +ECDSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> > > -AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
> > > +AES_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/aes/, \
> > >   					aes-encrypt.o aes-decrypt.o)
> > >   # Cryptographic helpers that depend on openssl/libcrypto
> > > -LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
> > > +LIBCRYPTO_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/, \
> > >   					fdt-libcrypto.o)
> > >   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> > > @@ -136,22 +155,17 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
> > >   fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
> > >   file2include-objs := file2include.o
> > > -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
> > > +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
> > >   # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
> > >   # the mxsimage support within tools/mxsimage.c .
> > >   HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
> > >   endif
> > > -ifdef CONFIG_FIT_SIGNATURE
> > > +ifdef CONFIG_TOOLS_USE_LIBCRYPTO
> > >   # This affects include/image.h, but including the board config file
> > >   # is tricky, so manually define this options here.
> > >   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
> > > -HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
> > > -endif
> > > -
> > > -ifdef CONFIG_FIT_CIPHER
> > > -# This affects include/image.h, but including the board config file
> > > -# is tricky, so manually define this options here.
> > > +HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
> > >   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
> > >   endif
> > > @@ -164,7 +178,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
> > >   endif
> > >   # MXSImage needs LibSSL
> > > -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
> > > +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
> > >   HOSTCFLAGS_kwbimage.o += \
> > >   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
> > >   HOSTLDLIBS_mkimage += \
> > > -- 
> > > 2.31.1
> > > 

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-06-16 16:08     ` Tom Rini
@ 2021-06-16 22:28       ` AKASHI Takahiro
  0 siblings, 0 replies; 8+ messages in thread
From: AKASHI Takahiro @ 2021-06-16 22:28 UTC (permalink / raw)
  To: Tom Rini; +Cc: Alex G., u-boot, sjg

On Wed, Jun 16, 2021 at 12:08:00PM -0400, Tom Rini wrote:
> On Wed, Jun 16, 2021 at 09:34:27AM -0500, Alex G. wrote:
> > On 6/15/21 6:34 PM, AKASHI Takahiro wrote:
> > > A gentle ping.
> > > What is the current review status?
> > > Who will take care of this patch?
> > 
> > Patchwork automatically delegates this to a maintainer [1], but anyone is
> > welcome to comment and review.
> 
> Note that it's not automatic, I typically do it for anything that hasn't
> been picked up by a custodian already.

Yes, I have recognized it.

> I think the patch in question is fine, and since I think the series it
> depends on is also fine with Simon, I'll be picking it up either for
> next or after release.

Thank you, Tom.

-Takahiro Akashi

> > 
> > Alex
> > 
> > [1] https://patchwork.ozlabs.org/project/uboot/patch/20210524202317.1492578-1-mr.nuke.me@gmail.com/
> > 
> > > -Takahiro Akashi
> > > 
> > > On Mon, May 24, 2021 at 03:23:17PM -0500, Alexandru Gagniuc wrote:
> > > > Host tool features, such as mkimage's ability to sign FIT images were
> > > > enabled or disabled based on the target configuration. However, this
> > > > misses the point of a target-agnostic host tool.
> > > > 
> > > > A target's ability to verify FIT signatures is independent of
> > > > mkimage's ability to create those signatures. In fact, u-boot's build
> > > > system doesn't sign images. The target code can be successfully built
> > > > without relying on any ability to sign such code.
> > > > 
> > > > Conversely, mkimage's ability to sign images does not require that
> > > > those images will only work on targets which support FIT verification.
> > > > Linking mkimage cryptographic features to target support for FIT
> > > > verification is misguided.
> > > > 
> > > > Without loss of generality, we can say that host features are and
> > > > should be independent of target features.
> > > > 
> > > > While we prefer that a host tool always supports the same feature set,
> > > > we recognize the following
> > > >    - some users prefer to build u-boot without a dependency on OpenSSL.
> > > >    - some distros prefer to ship mkimage without linking to OpenSSL
> > > > 
> > > > To allow these use cases, introduce a host-only Kconfig which is used
> > > > to select or deselect libcrypto support. Some mkimage features or some
> > > > host tools might not be available, but this shouldn't affect the
> > > > u-boot build.
> > > > 
> > > > I also considered setting the default of this config based on
> > > > FIT_SIGNATURE. While it would preserve the old behaviour it's also
> > > > contrary to the goals of this change. I decided to enable it by
> > > > default, so that the default build yields the most feature-complete
> > > > mkimage.
> > > > 
> > > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > > ---
> > > > 
> > > > This patch is designed to apply on top of
> > > >      [PATCH v2 00/18] image: Reduce #ifdef abuse in image code
> > > > 
> > > > 
> > > > 
> > > >   tools/Kconfig  | 11 +++++++++++
> > > >   tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
> > > >   2 files changed, 41 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/tools/Kconfig b/tools/Kconfig
> > > > index b2f5012240..214932ae30 100644
> > > > --- a/tools/Kconfig
> > > > +++ b/tools/Kconfig
> > > > @@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
> > > >   	  some cases the system dtc may not support all required features
> > > >   	  and the path to a different version should be given here.
> > > > +config TOOLS_USE_LIBCRYPTO
> > > > +	bool "Use OpenSSL's libcrypto library for host tools"
> > > > +	default y
> > > > +	help
> > > > +	  Cryptographic signature, verification, and encryption of images is
> > > > +	  provided by host tools using OpenSSL's libcrypto. Select 'n' here if
> > > > +	  you wish to build host tools without OpenSSL. mkimage will not have
> > > > +	  the ability to sign images.
> > > > +	  This selection does not affect target features, such as runtime FIT
> > > > +	  signature verification.
> > > > +
> > > >   endmenu
> > > > diff --git a/tools/Makefile b/tools/Makefile
> > > > index 722355e984..1f30a06cce 100644
> > > > --- a/tools/Makefile
> > > > +++ b/tools/Makefile
> > > > @@ -3,6 +3,25 @@
> > > >   # (C) Copyright 2000-2006
> > > >   # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> > > > +# A note on target vs host configuration:
> > > > +#
> > > > +# Host tools can be used across multiple targets, or different configurations
> > > > +# of the same target. Thus, host tools must be able to handle any combination
> > > > +# of target configurations. To prevent having different variations of the same
> > > > +# tool, the tool build options may not depend on target configuration.
> > > > +#
> > > > +# Some linux distributions package these utilities as u-boot-tools, and it
> > > > +# would be unmaintainable to have a different tool variation for each
> > > > +# arch or configuration.
> > > > +#
> > > > +# A couple of simple rules:
> > > > +#
> > > > +# 1) Do not use target CONFIG_* options to enable or disable features in host
> > > > +#    tools. Only use the configs from tools/Kconfig
> > > > +# 2) It's okay to use target configs to disable building specific tools.
> > > > +#    That's as long as the features of those tools aren't modified.
> > > > +#
> > > > +
> > > >   # Enable all the config-independent tools
> > > >   ifneq ($(HOST_TOOLS_ALL),)
> > > >   CONFIG_ARCH_KIRKWOOD = y
> > > > @@ -53,30 +72,30 @@ hostprogs-y += mkenvimage
> > > >   mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
> > > >   hostprogs-y += dumpimage mkimage
> > > > -hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
> > > > +hostprogs-$(CONFIG_TOOLS_USE_LIBCRYPTO) += fit_info fit_check_sign
> > > >   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
> > > >   FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> > > > -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o common/image-fit-sig.o
> > > > -FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
> > > > +FIT_SIG_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := image-sig-host.o common/image-fit-sig.o
> > > > +FIT_CIPHER_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := common/image-cipher.o
> > > >   # The following files are synced with upstream DTC.
> > > >   # Use synced versions from scripts/dtc/libfdt/.
> > > >   LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
> > > >   		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
> > > > -RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
> > > > +RSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/rsa/, \
> > > >   					rsa-sign.o rsa-verify.o \
> > > >   					rsa-mod-exp.o)
> > > > -ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> > > > +ECDSA_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> > > > -AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
> > > > +AES_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/aes/, \
> > > >   					aes-encrypt.o aes-decrypt.o)
> > > >   # Cryptographic helpers that depend on openssl/libcrypto
> > > > -LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
> > > > +LIBCRYPTO_OBJS-$(CONFIG_TOOLS_USE_LIBCRYPTO) := $(addprefix lib/, \
> > > >   					fdt-libcrypto.o)
> > > >   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> > > > @@ -136,22 +155,17 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
> > > >   fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
> > > >   file2include-objs := file2include.o
> > > > -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
> > > > +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
> > > >   # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
> > > >   # the mxsimage support within tools/mxsimage.c .
> > > >   HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
> > > >   endif
> > > > -ifdef CONFIG_FIT_SIGNATURE
> > > > +ifdef CONFIG_TOOLS_USE_LIBCRYPTO
> > > >   # This affects include/image.h, but including the board config file
> > > >   # is tricky, so manually define this options here.
> > > >   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
> > > > -HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
> > > > -endif
> > > > -
> > > > -ifdef CONFIG_FIT_CIPHER
> > > > -# This affects include/image.h, but including the board config file
> > > > -# is tricky, so manually define this options here.
> > > > +HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
> > > >   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
> > > >   endif
> > > > @@ -164,7 +178,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
> > > >   endif
> > > >   # MXSImage needs LibSSL
> > > > -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
> > > > +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_USE_LIBCRYPTO),)
> > > >   HOSTCFLAGS_kwbimage.o += \
> > > >   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
> > > >   HOSTLDLIBS_mkimage += \
> > > > -- 
> > > > 2.31.1
> > > > 
> 
> -- 
> Tom



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-05-24 20:23 [PATCH] tools: Use a single target-independent config to enable OpenSSL Alexandru Gagniuc
  2021-06-15 23:34 ` AKASHI Takahiro
@ 2021-06-22 13:31 ` Simon Glass
  2021-06-22 14:13   ` Alex G.
  1 sibling, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-06-22 13:31 UTC (permalink / raw)
  To: Alexandru Gagniuc; +Cc: U-Boot Mailing List, Tom Rini

On Mon, 24 May 2021 at 14:23, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Host tool features, such as mkimage's ability to sign FIT images were
> enabled or disabled based on the target configuration. However, this
> misses the point of a target-agnostic host tool.
>
> A target's ability to verify FIT signatures is independent of
> mkimage's ability to create those signatures. In fact, u-boot's build
> system doesn't sign images. The target code can be successfully built
> without relying on any ability to sign such code.
>
> Conversely, mkimage's ability to sign images does not require that
> those images will only work on targets which support FIT verification.
> Linking mkimage cryptographic features to target support for FIT
> verification is misguided.
>
> Without loss of generality, we can say that host features are and
> should be independent of target features.
>
> While we prefer that a host tool always supports the same feature set,
> we recognize the following
>   - some users prefer to build u-boot without a dependency on OpenSSL.
>   - some distros prefer to ship mkimage without linking to OpenSSL
>
> To allow these use cases, introduce a host-only Kconfig which is used
> to select or deselect libcrypto support. Some mkimage features or some
> host tools might not be available, but this shouldn't affect the
> u-boot build.
>
> I also considered setting the default of this config based on
> FIT_SIGNATURE. While it would preserve the old behaviour it's also
> contrary to the goals of this change. I decided to enable it by
> default, so that the default build yields the most feature-complete
> mkimage.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>
> This patch is designed to apply on top of
>     [PATCH v2 00/18] image: Reduce #ifdef abuse in image code
>
>
>
>  tools/Kconfig  | 11 +++++++++++
>  tools/Makefile | 46 ++++++++++++++++++++++++++++++----------------
>  2 files changed, 41 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

See below

>
> diff --git a/tools/Kconfig b/tools/Kconfig
> index b2f5012240..214932ae30 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -9,4 +9,15 @@ config MKIMAGE_DTC_PATH
>           some cases the system dtc may not support all required features
>           and the path to a different version should be given here.
>
> +config TOOLS_USE_LIBCRYPTO

would HOST_LIBCRYPTO be better?

> +       bool "Use OpenSSL's libcrypto library for host tools"
> +       default y
> +       help
> +         Cryptographic signature, verification, and encryption of images is
> +         provided by host tools using OpenSSL's libcrypto. Select 'n' here if
> +         you wish to build host tools without OpenSSL. mkimage will not have
> +         the ability to sign images.
> +         This selection does not affect target features, such as runtime FIT
> +         signature verification.

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-06-22 13:31 ` Simon Glass
@ 2021-06-22 14:13   ` Alex G.
  2021-06-22 19:25     ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Alex G. @ 2021-06-22 14:13 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini

On 6/22/21 8:31 AM, Simon Glass wrote:

[snip]

>> +config TOOLS_USE_LIBCRYPTO
> 
> would HOST_LIBCRYPTO be better?

I had considered a shorter kconfig such as the above. Does it mean
     (1) The build host has libcrypto available?
     (2) We use the libcrypto on the host?
     (3) There is a libcrypto for the target?

"tools" is a subset of what we do on the host so it seems more concrete. 
I realize you could read "tools use libcrypto" as a statement rather 
than a question, which would seem odd in kconfig.

As far as having a verb, I thought it to be important because there 
isn't a substantially similar use of Kconfig in u-boot. I didn't want 
the name to be too vague.

Hope this clears things up,
Alex

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: Use a single target-independent config to enable OpenSSL
  2021-06-22 14:13   ` Alex G.
@ 2021-06-22 19:25     ` Simon Glass
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2021-06-22 19:25 UTC (permalink / raw)
  To: Alex G.; +Cc: U-Boot Mailing List, Tom Rini

Hi Alex,

On Tue, 22 Jun 2021 at 08:13, Alex G. <mr.nuke.me@gmail.com> wrote:
>
> On 6/22/21 8:31 AM, Simon Glass wrote:
>
> [snip]
>
> >> +config TOOLS_USE_LIBCRYPTO
> >
> > would HOST_LIBCRYPTO be better?
>
> I had considered a shorter kconfig such as the above. Does it mean
>      (1) The build host has libcrypto available?
>      (2) We use the libcrypto on the host?
>      (3) There is a libcrypto for the target?
>
> "tools" is a subset of what we do on the host so it seems more concrete.
> I realize you could read "tools use libcrypto" as a statement rather
> than a question, which would seem odd in kconfig.

This is the sort of thinking that got us the SPL_XXX_SUPPORT :-)

We never use Kconfig options for (1), so far as I know. That would
just produce a build error.

This is not a target Kconfig, so has nothing to do with that.

So clearly the answer to your question is 2. We should aim for
consistency in the Kconfig options.

>
> As far as having a verb, I thought it to be important because there
> isn't a substantially similar use of Kconfig in u-boot. I didn't want
> the name to be too vague.

I'd prefer to avoid the verb. It just makes it harder to remember and
it isn't as if we could mean that we would do anything else with the
option than use it...

>
> Hope this clears things up,
> Alex

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-06-22 19:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-24 20:23 [PATCH] tools: Use a single target-independent config to enable OpenSSL Alexandru Gagniuc
2021-06-15 23:34 ` AKASHI Takahiro
2021-06-16 14:34   ` Alex G.
2021-06-16 16:08     ` Tom Rini
2021-06-16 22:28       ` AKASHI Takahiro
2021-06-22 13:31 ` Simon Glass
2021-06-22 14:13   ` Alex G.
2021-06-22 19:25     ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.