All of lore.kernel.org
 help / color / mirror / Atom feed
* Broken build with disabling OpenSSL crypto
@ 2021-10-06 21:27 Jernej Škrabec
  2021-10-06 22:05 ` Alex G.
  2021-10-07 19:41 ` Tom Rini
  0 siblings, 2 replies; 12+ messages in thread
From: Jernej Škrabec @ 2021-10-06 21:27 UTC (permalink / raw)
  To: u-boot, pali, mr.nuke.me

Hi everyone!

Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable 
OpenSSL") recently introduced option to disable usage of OpenSSL via 
CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9 
("tools: kwbimage: Do not hide usage of secure header under 
CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally 
defeats the purpose of first commit. I suggest that it gets reverted.

I would like disable OpenSSL for my usage, since it gives me troubles when 
cross-compiling U-Boot inside LibreELEC build system. It's not needed for our 
case anyway.

Best regards,
Jernej



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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-06 21:27 Broken build with disabling OpenSSL crypto Jernej Škrabec
@ 2021-10-06 22:05 ` Alex G.
  2021-10-08  9:34   ` Rudi Heitbaum
                     ` (3 more replies)
  2021-10-07 19:41 ` Tom Rini
  1 sibling, 4 replies; 12+ messages in thread
From: Alex G. @ 2021-10-06 22:05 UTC (permalink / raw)
  To: Jernej Škrabec, u-boot, pali

Hi Jernej,

On 10/6/21 4:27 PM, Jernej Škrabec wrote:
> Hi everyone!
> 
> Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
> OpenSSL") recently introduced option to disable usage of OpenSSL via
> CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
> ("tools: kwbimage: Do not hide usage of secure header under
> CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
> defeats the purpose of first commit. I suggest that it gets reverted.
> 
> I would like disable OpenSSL for my usage, since it gives me troubles when
> cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
> case anyway.
> 
> Best regards,
> 

Can you please give the following diff a try, and if it works for you, submit as patch?

Alex

diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..7f72ff9645 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \

  # Cryptographic helpers that depend on openssl/libcrypto
  LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
-					fdt-libcrypto.o)
+					fdt-libcrypto.o) \
+					kwbimage.o

  ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o

@@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
  			imximage.o \
  			imx8image.o \
  			imx8mimage.o \
-			kwbimage.o \
  			lib/md5.o \
  			lpc32xximage.o \
  			mxsimage.o \
@@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
  HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
  endif

-# MXSImage needs LibSSL
-ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
+# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
+ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
  HOSTCFLAGS_kwbimage.o += \
  	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
  HOSTLDLIBS_mkimage += \

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-06 21:27 Broken build with disabling OpenSSL crypto Jernej Škrabec
  2021-10-06 22:05 ` Alex G.
@ 2021-10-07 19:41 ` Tom Rini
  2021-10-10 11:04   ` Jernej Škrabec
  1 sibling, 1 reply; 12+ messages in thread
From: Tom Rini @ 2021-10-07 19:41 UTC (permalink / raw)
  To: Jernej Škrabec; +Cc: u-boot, pali, mr.nuke.me

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

On Wed, Oct 06, 2021 at 11:27:43PM +0200, Jernej Škrabec wrote:

> Hi everyone!
> 
> Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable 
> OpenSSL") recently introduced option to disable usage of OpenSSL via 
> CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9 
> ("tools: kwbimage: Do not hide usage of secure header under 
> CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally 
> defeats the purpose of first commit. I suggest that it gets reverted.
> 
> I would like disable OpenSSL for my usage, since it gives me troubles when 
> cross-compiling U-Boot inside LibreELEC build system. It's not needed for our 
> case anyway.

How hard is it to specify openssl as a dependency for U-Boot, in the
LibreELEC build system?  I assume openssl is being used in other parts
of the build anyhow.  Thanks!

-- 
Tom

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

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-06 22:05 ` Alex G.
@ 2021-10-08  9:34   ` Rudi Heitbaum
  2021-10-10 11:06   ` Jernej Škrabec
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Rudi Heitbaum @ 2021-10-08  9:34 UTC (permalink / raw)
  To: u-boot

On Wed, Oct 06, 2021 at 05:05:24PM -0500, Alex G. wrote:
> Hi Jernej,
> 
> On 10/6/21 4:27 PM, Jernej ?krabec wrote:
> > Hi everyone!
> > 
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
> > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
> > ("tools: kwbimage: Do not hide usage of secure header under
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
> > defeats the purpose of first commit. I suggest that it gets reverted.
> > 
> > I would like disable OpenSSL for my usage, since it gives me troubles when
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
> > case anyway.
> > 
> > Best regards,
> > 
> 
> Can you please give the following diff a try, and if it works for you, submit as patch?
> 
> Alex

Hi Alex,

I can confirm the patch does work as expected.

https://github.com/LibreELEC/LibreELEC.tv/pull/5719/files

Setting the CONFIG_TOOLS_LIBCRYPTO=n - now results in a successful
build. It fixes the must have SSL regression introduced in b4f3cc2c42d9.

Regards
Rudi

> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f64..7f72ff9645 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
> 
>   # Cryptographic helpers that depend on openssl/libcrypto
>   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> -					fdt-libcrypto.o)
> +					fdt-libcrypto.o) \
> +					kwbimage.o
> 
>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> 
> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>   			imximage.o \
>   			imx8image.o \
>   			imx8mimage.o \
> -			kwbimage.o \
>   			lib/md5.o \
>   			lpc32xximage.o \
>   			mxsimage.o \
> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>   endif
> 
> -# MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>   HOSTCFLAGS_kwbimage.o += \
>   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>   HOSTLDLIBS_mkimage += \

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

* Re: Re: Broken build with disabling OpenSSL crypto
  2021-10-07 19:41 ` Tom Rini
@ 2021-10-10 11:04   ` Jernej Škrabec
  0 siblings, 0 replies; 12+ messages in thread
From: Jernej Škrabec @ 2021-10-10 11:04 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, pali, mr.nuke.me

Hi!

Dne četrtek, 07. oktober 2021 ob 21:41:00 CEST je Tom Rini napisal(a):
> On Wed, Oct 06, 2021 at 11:27:43PM +0200, Jernej Škrabec wrote:
> 
> > Hi everyone!
> > 
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to 
enable 
> > OpenSSL") recently introduced option to disable usage of OpenSSL via 
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit 
b4f3cc2c42d9 
> > ("tools: kwbimage: Do not hide usage of secure header under 
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That 
totally 
> > defeats the purpose of first commit. I suggest that it gets reverted.
> > 
> > I would like disable OpenSSL for my usage, since it gives me troubles when 
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for 
our 
> > case anyway.
> 
> How hard is it to specify openssl as a dependency for U-Boot, in the
> LibreELEC build system?  I assume openssl is being used in other parts
> of the build anyhow.  Thanks!

Sure, OpenSSL package is present and we fixed issue with pkg-config in the 
meantime (it picked target paths instead for the host). However, that doesn't 
change anything. CONFIG_TOOLS_LIBCRYPTO is still borked and should be either 
fixed or dropped. I prefer first option because OpenSSL dependency can be 
removed and that allows more concurrency (multiple packages are built at the 
same time).

Best regards,
Jernej



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

* Re: Re: Broken build with disabling OpenSSL crypto
  2021-10-06 22:05 ` Alex G.
  2021-10-08  9:34   ` Rudi Heitbaum
@ 2021-10-10 11:06   ` Jernej Škrabec
  2021-10-11 16:48     ` Alex G.
  2021-10-13  5:26   ` Andre Heider
  2021-10-15 11:34   ` Pali Rohár
  3 siblings, 1 reply; 12+ messages in thread
From: Jernej Škrabec @ 2021-10-10 11:06 UTC (permalink / raw)
  To: u-boot, pali, Alex G.

Hi Alex!

Dne četrtek, 07. oktober 2021 ob 00:05:24 CEST je Alex G. napisal(a):
> Hi Jernej,
> 
> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
> > Hi everyone!
> > 
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to 
enable
> > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit 
b4f3cc2c42d9
> > ("tools: kwbimage: Do not hide usage of secure header under
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That 
totally
> > defeats the purpose of first commit. I suggest that it gets reverted.
> > 
> > I would like disable OpenSSL for my usage, since it gives me troubles when
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for 
our
> > case anyway.
> > 
> > Best regards,
> > 
> 
> Can you please give the following diff a try, and if it works for you, submit 
as patch?

This works, I'll submit it as a patch. Should I keep you as original author 
and add your SoB tag?

Best regards,
Jernej

> 
> Alex
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f64..7f72ff9645 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, 
\
> 
>   # Cryptographic helpers that depend on openssl/libcrypto
>   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> -					fdt-libcrypto.o)
> +					fdt-libcrypto.o) \
> +					kwbimage.o
> 
>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> 
> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>   			imximage.o \
>   			imx8image.o \
>   			imx8mimage.o \
> -			kwbimage.o \
>   			lib/md5.o \
>   			lpc32xximage.o \
>   			mxsimage.o \
> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -
DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>   endif
> 
> -# MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$
(CONFIG_TOOLS_LIBCRYPTO),)
> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>   HOSTCFLAGS_kwbimage.o += \
>   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo 
"")
>   HOSTLDLIBS_mkimage += \
> 



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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-10 11:06   ` Jernej Škrabec
@ 2021-10-11 16:48     ` Alex G.
  0 siblings, 0 replies; 12+ messages in thread
From: Alex G. @ 2021-10-11 16:48 UTC (permalink / raw)
  To: Jernej Škrabec, u-boot, pali

On 10/10/21 6:06 AM, Jernej Škrabec wrote:
> Dne četrtek, 07. oktober 2021 ob 00:05:24 CEST je Alex G. napisal(a):
>> Can you please give the following diff a try, and if it works for you, submit
> as patch?
> 
> This works, I'll submit it as a patch. Should I keep you as original author
> and add your SoB tag?

You can take authorship.

Alex

> Best regards,
> Jernej
> 
>>
>> Alex
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 4a86321f64..7f72ff9645 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/,
> \
>>
>>    # Cryptographic helpers that depend on openssl/libcrypto
>>    LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
>> -					fdt-libcrypto.o)
>> +					fdt-libcrypto.o) \
>> +					kwbimage.o
>>
>>    ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>>
>> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>    			imximage.o \
>>    			imx8image.o \
>>    			imx8mimage.o \
>> -			kwbimage.o \
>>    			lib/md5.o \
>>    			lpc32xximage.o \
>>    			mxsimage.o \
>> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -
> DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>>    HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>>    endif
>>
>> -# MXSImage needs LibSSL
>> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$
> (CONFIG_TOOLS_LIBCRYPTO),)
>> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
>> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>>    HOSTCFLAGS_kwbimage.o += \
>>    	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo
> "")
>>    HOSTLDLIBS_mkimage += \
>>
> 
> 

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-06 22:05 ` Alex G.
  2021-10-08  9:34   ` Rudi Heitbaum
  2021-10-10 11:06   ` Jernej Škrabec
@ 2021-10-13  5:26   ` Andre Heider
  2021-10-15 11:34   ` Pali Rohár
  3 siblings, 0 replies; 12+ messages in thread
From: Andre Heider @ 2021-10-13  5:26 UTC (permalink / raw)
  To: Alex G., Jernej Škrabec, u-boot

Hi,

On 07/10/2021 00:05, Alex G. wrote:
> Hi Jernej,
> 
> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
>> Hi everyone!
>>
>> Commit cb9faa6f98ae ("tools: Use a single target-independent config to 
>> enable
>> OpenSSL") recently introduced option to disable usage of OpenSSL via
>> CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit 
>> b4f3cc2c42d9
>> ("tools: kwbimage: Do not hide usage of secure header under
>> CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That 
>> totally
>> defeats the purpose of first commit. I suggest that it gets reverted.
>>
>> I would like disable OpenSSL for my usage, since it gives me troubles 
>> when
>> cross-compiling U-Boot inside LibreELEC build system. It's not needed 
>> for our
>> case anyway.
>>
>> Best regards,
>>
> 
> Can you please give the following diff a try, and if it works for you, 
> submit as patch?

it looks like this is still required, as this fixes it for me too ;)

Thanks,
Andre

> 
> Alex
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f64..7f72ff9645 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix 
> lib/aes/, \
> 
>   # Cryptographic helpers that depend on openssl/libcrypto
>   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> -                    fdt-libcrypto.o)
> +                    fdt-libcrypto.o) \
> +                    kwbimage.o
> 
>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> 
> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>               imximage.o \
>               imx8image.o \
>               imx8mimage.o \
> -            kwbimage.o \
>               lib/md5.o \
>               lpc32xximage.o \
>               mxsimage.o \
> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS    += 
> -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>   HOST_EXTRACFLAGS    += -DCONFIG_FIT_CIPHER
>   endif
> 
> -# MXSImage needs LibSSL
> -ifneq 
> ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),) 
> 
> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>   HOSTCFLAGS_kwbimage.o += \
>       $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>   HOSTLDLIBS_mkimage += \


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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-06 22:05 ` Alex G.
                     ` (2 preceding siblings ...)
  2021-10-13  5:26   ` Andre Heider
@ 2021-10-15 11:34   ` Pali Rohár
  2021-10-15 14:35     ` Alex G.
  3 siblings, 1 reply; 12+ messages in thread
From: Pali Rohár @ 2021-10-15 11:34 UTC (permalink / raw)
  To: Alex G.; +Cc: Jernej Škrabec, u-boot

On Wednesday 06 October 2021 17:05:24 Alex G. wrote:
> Hi Jernej,
> 
> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
> > Hi everyone!
> > 
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
> > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
> > ("tools: kwbimage: Do not hide usage of secure header under
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
> > defeats the purpose of first commit. I suggest that it gets reverted.
> > 
> > I would like disable OpenSSL for my usage, since it gives me troubles when
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
> > case anyway.
> > 
> > Best regards,
> > 
> 
> Can you please give the following diff a try, and if it works for you, submit as patch?

This change is incorrect and will break mvebu builds. mvebu requires
kwbimage for building boot images and so you cannot disable it or make
it optional.

> Alex
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f64..7f72ff9645 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
> 
>  # Cryptographic helpers that depend on openssl/libcrypto
>  LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> -					fdt-libcrypto.o)
> +					fdt-libcrypto.o) \
> +					kwbimage.o
> 
>  ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> 
> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>  			imximage.o \
>  			imx8image.o \
>  			imx8mimage.o \
> -			kwbimage.o \
>  			lib/md5.o \
>  			lpc32xximage.o \
>  			mxsimage.o \
> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>  HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>  endif
> 
> -# MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>  HOSTCFLAGS_kwbimage.o += \
>  	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>  HOSTLDLIBS_mkimage += \

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-15 11:34   ` Pali Rohár
@ 2021-10-15 14:35     ` Alex G.
  2021-10-15 20:30       ` Pali Rohár
  0 siblings, 1 reply; 12+ messages in thread
From: Alex G. @ 2021-10-15 14:35 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Jernej Škrabec, u-boot

On 10/15/21 6:34 AM, Pali Rohár wrote:
> On Wednesday 06 October 2021 17:05:24 Alex G. wrote:
>> Hi Jernej,
>>
>> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
>>> Hi everyone!
>>>
>>> Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
>>> OpenSSL") recently introduced option to disable usage of OpenSSL via
>>> CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
>>> ("tools: kwbimage: Do not hide usage of secure header under
>>> CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
>>> defeats the purpose of first commit. I suggest that it gets reverted.
>>>
>>> I would like disable OpenSSL for my usage, since it gives me troubles when
>>> cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
>>> case anyway.
>>>
>>> Best regards,
>>>
>>
>> Can you please give the following diff a try, and if it works for you, submit as patch?
> 
> This change is incorrect and will break mvebu builds. mvebu requires
> kwbimage for building boot images and so you cannot disable it or make
> it optional.
> 

If kwbimage is required and missing the CI builds and tests don't catch 
that. I ran buildman with the change, and nothing broke. Sounds like 
that needs to be addressed.

That being said, I'm not okay with making everyone a slave to OpenSSL 
because of any given platform.

I propose to revert commit b4f3cc2c42d9 ("tools: kwbimage: Do not hide 
usage of secure header under CONFIG_ARMADA_38X"), and rework it such 
that it doesn't force libcrypto on everyone. And we very likely need a 
CI test against libcrypto linkage when TOOLS_LIBCRYPTO is not set.

Alex

>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 4a86321f64..7f72ff9645 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
>>
>>   # Cryptographic helpers that depend on openssl/libcrypto
>>   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
>> -					fdt-libcrypto.o)
>> +					fdt-libcrypto.o) \
>> +					kwbimage.o
>>
>>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>>
>> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>   			imximage.o \
>>   			imx8image.o \
>>   			imx8mimage.o \
>> -			kwbimage.o \
>>   			lib/md5.o \
>>   			lpc32xximage.o \
>>   			mxsimage.o \
>> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>>   endif
>>
>> -# MXSImage needs LibSSL
>> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
>> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
>> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>>   HOSTCFLAGS_kwbimage.o += \
>>   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>>   HOSTLDLIBS_mkimage += \

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-15 14:35     ` Alex G.
@ 2021-10-15 20:30       ` Pali Rohár
  2021-10-18 14:04         ` Alex G.
  0 siblings, 1 reply; 12+ messages in thread
From: Pali Rohár @ 2021-10-15 20:30 UTC (permalink / raw)
  To: Alex G.; +Cc: Jernej Škrabec, u-boot

On Friday 15 October 2021 09:35:43 Alex G. wrote:
> On 10/15/21 6:34 AM, Pali Rohár wrote:
> > On Wednesday 06 October 2021 17:05:24 Alex G. wrote:
> > > Hi Jernej,
> > > 
> > > On 10/6/21 4:27 PM, Jernej Škrabec wrote:
> > > > Hi everyone!
> > > > 
> > > > Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
> > > > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > > > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
> > > > ("tools: kwbimage: Do not hide usage of secure header under
> > > > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
> > > > defeats the purpose of first commit. I suggest that it gets reverted.
> > > > 
> > > > I would like disable OpenSSL for my usage, since it gives me troubles when
> > > > cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
> > > > case anyway.
> > > > 
> > > > Best regards,
> > > > 
> > > 
> > > Can you please give the following diff a try, and if it works for you, submit as patch?
> > 
> > This change is incorrect and will break mvebu builds. mvebu requires
> > kwbimage for building boot images and so you cannot disable it or make
> > it optional.
> > 
> 
> If kwbimage is required and missing the CI builds and tests don't catch
> that. I ran buildman with the change, and nothing broke. Sounds like that
> needs to be addressed.

It is possible that tests do not covert all scenarios.

> That being said, I'm not okay with making everyone a slave to OpenSSL
> because of any given platform.
> 
> I propose to revert commit b4f3cc2c42d9 ("tools: kwbimage: Do not hide usage
> of secure header under CONFIG_ARMADA_38X"), and rework it such that it
> doesn't force libcrypto on everyone. And we very likely need a CI test
> against libcrypto linkage when TOOLS_LIBCRYPTO is not set.

Reverting that commit is not a solution as it can lead to broken
kwbimage (when crypto stuff is not enabled). Plus there is lot of other
changes and fixes in kwboot and kwbimage...

Some information with another approach how to solve build issues are in
this email:
https://lore.kernel.org/u-boot/20211015114735.rig3e4cuc7mn6a7e@pali/

> Alex
> 
> > > 
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index 4a86321f64..7f72ff9645 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
> > > 
> > >   # Cryptographic helpers that depend on openssl/libcrypto
> > >   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> > > -					fdt-libcrypto.o)
> > > +					fdt-libcrypto.o) \
> > > +					kwbimage.o
> > > 
> > >   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> > > 
> > > @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
> > >   			imximage.o \
> > >   			imx8image.o \
> > >   			imx8mimage.o \
> > > -			kwbimage.o \
> > >   			lib/md5.o \
> > >   			lpc32xximage.o \
> > >   			mxsimage.o \
> > > @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
> > >   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
> > >   endif
> > > 
> > > -# MXSImage needs LibSSL
> > > -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
> > > +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> > > +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
> > >   HOSTCFLAGS_kwbimage.o += \
> > >   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
> > >   HOSTLDLIBS_mkimage += \

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

* Re: Broken build with disabling OpenSSL crypto
  2021-10-15 20:30       ` Pali Rohár
@ 2021-10-18 14:04         ` Alex G.
  0 siblings, 0 replies; 12+ messages in thread
From: Alex G. @ 2021-10-18 14:04 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Jernej Škrabec, u-boot

On 10/15/21 3:30 PM, Pali Rohár wrote:
> On Friday 15 October 2021 09:35:43 Alex G. wrote:
>> On 10/15/21 6:34 AM, Pali Rohár wrote:
>>> On Wednesday 06 October 2021 17:05:24 Alex G. wrote:
>>>> Hi Jernej,
>>>>
>>>> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
>>>>> Hi everyone!
>>>>>
>>>>> Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
>>>>> OpenSSL") recently introduced option to disable usage of OpenSSL via
>>>>> CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
>>>>> ("tools: kwbimage: Do not hide usage of secure header under
>>>>> CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
>>>>> defeats the purpose of first commit. I suggest that it gets reverted.
>>>>>
>>>>> I would like disable OpenSSL for my usage, since it gives me troubles when
>>>>> cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
>>>>> case anyway.
>>>>>
>>>>> Best regards,
>>>>>
>>>>
>>>> Can you please give the following diff a try, and if it works for you, submit as patch?
>>>
>>> This change is incorrect and will break mvebu builds. mvebu requires
>>> kwbimage for building boot images and so you cannot disable it or make
>>> it optional.
>>>
>>
>> If kwbimage is required and missing the CI builds and tests don't catch
>> that. I ran buildman with the change, and nothing broke. Sounds like that
>> needs to be addressed.
> 
> It is possible that tests do not covert all scenarios.
> 
>> That being said, I'm not okay with making everyone a slave to OpenSSL
>> because of any given platform.
>>
>> I propose to revert commit b4f3cc2c42d9 ("tools: kwbimage: Do not hide usage
>> of secure header under CONFIG_ARMADA_38X"), and rework it such that it
>> doesn't force libcrypto on everyone. And we very likely need a CI test
>> against libcrypto linkage when TOOLS_LIBCRYPTO is not set.
> 
> Reverting that commit is not a solution as it can lead to broken
> kwbimage (when crypto stuff is not enabled). Plus there is lot of other
> changes and fixes in kwboot and kwbimage...

There are lots of CI tests that do not build usable images. I caution 
against conflating testability with usability, as anyone who intends to 
run images on hardware will likely have done their diligence and enabled 
TOOLS_LIBCRYPTO.

Alex



> Some information with another approach how to solve build issues are in
> this email:
> https://lore.kernel.org/u-boot/20211015114735.rig3e4cuc7mn6a7e@pali/
> 
>> Alex
>>
>>>>
>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>> index 4a86321f64..7f72ff9645 100644
>>>> --- a/tools/Makefile
>>>> +++ b/tools/Makefile
>>>> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
>>>>
>>>>    # Cryptographic helpers that depend on openssl/libcrypto
>>>>    LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
>>>> -					fdt-libcrypto.o)
>>>> +					fdt-libcrypto.o) \
>>>> +					kwbimage.o
>>>>
>>>>    ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>>>>
>>>> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>>>    			imximage.o \
>>>>    			imx8image.o \
>>>>    			imx8mimage.o \
>>>> -			kwbimage.o \
>>>>    			lib/md5.o \
>>>>    			lpc32xximage.o \
>>>>    			mxsimage.o \
>>>> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
>>>>    HOST_EXTRACFLAGS	+= -DCONFIG_FIT_CIPHER
>>>>    endif
>>>>
>>>> -# MXSImage needs LibSSL
>>>> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
>>>> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
>>>> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
>>>>    HOSTCFLAGS_kwbimage.o += \
>>>>    	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>>>>    HOSTLDLIBS_mkimage += \

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

end of thread, other threads:[~2021-10-18 14:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 21:27 Broken build with disabling OpenSSL crypto Jernej Škrabec
2021-10-06 22:05 ` Alex G.
2021-10-08  9:34   ` Rudi Heitbaum
2021-10-10 11:06   ` Jernej Škrabec
2021-10-11 16:48     ` Alex G.
2021-10-13  5:26   ` Andre Heider
2021-10-15 11:34   ` Pali Rohár
2021-10-15 14:35     ` Alex G.
2021-10-15 20:30       ` Pali Rohár
2021-10-18 14:04         ` Alex G.
2021-10-07 19:41 ` Tom Rini
2021-10-10 11:04   ` Jernej Škrabec

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.