All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies
@ 2022-06-11 14:35 Fabrice Fontaine
  2022-06-11 14:35 ` [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR Fabrice Fontaine
  2022-06-11 20:19 ` [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Dimi Tomov
  0 siblings, 2 replies; 9+ messages in thread
From: Fabrice Fontaine @ 2022-06-11 14:35 UTC (permalink / raw)
  To: buildroot; +Cc: Dimitar Tomov, Fabrice Fontaine

pkg-config is not used by wolftpm and wolfssl dependency is missing (and
optional) resulting in the following build failure since the addition of
the package in commit 4bb884a3c61c6b71e33f69453a90eb2a367f64b7:

configure: error: WolfSSL library not found. You can get it from http://www.wolfssl.com/download.html
        If it's already installed, specify its path using --with-wolfcrypt=/dir or --prefix=/dir

Moreover, BR2_PACKAGE_WOLFSSL_ALL, threads and dynamic libray support
are not mandatory:

./utils/test-pkg -p wolftpm
                    bootlin-armv5-uclibc [1/6]: OK
                     bootlin-armv7-glibc [2/6]: OK
                   bootlin-armv7m-uclibc [3/6]: OK
                     bootlin-x86-64-musl [4/6]: OK
                      br-arm-full-static [5/6]: OK
                            sourcery-arm [6/6]: OK
6 builds, 0 skipped, 0 build failed, 0 legal-info failed, 0 show-info failed

Fixes:
 - http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/wolftpm/Config.in  |  7 -------
 package/wolftpm/wolftpm.mk | 13 ++++++++++---
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
index 0fe094f277..741d92aa4e 100644
--- a/package/wolftpm/Config.in
+++ b/package/wolftpm/Config.in
@@ -1,9 +1,5 @@
 config BR2_PACKAGE_WOLFTPM
 	bool "wolftpm"
-	depends on BR2_TOOLCHAIN_HAS_THREADS
-	depends on !BR2_STATIC_LIBS
-	select BR2_PACKAGE_WOLFSSL
-	select BR2_PACKAGE_WOLFSSL_ALL
 	help
 	  wolfTPM is a portable, open-source TPM 2.0 stack with
 	  backward API compatibility, designed for embedded use.
@@ -11,6 +7,3 @@ config BR2_PACKAGE_WOLFTPM
 	  resource usage.
 
 	  https://www.wolfssl.com/
-
-comment "wolftpm needs a toolchain w/ threads, dynamic library"
-	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
index ea01eaad6b..f0cf0df0d3 100644
--- a/package/wolftpm/wolftpm.mk
+++ b/package/wolftpm/wolftpm.mk
@@ -10,7 +10,6 @@ WOLFTPM_INSTALL_STAGING = YES
 WOLFTPM_LICENSE = GPL-2.0+
 WOLFTPM_LICENSE_FILES = LICENSE
 WOLFTPM_CPE_ID_VENDOR = wolfssl
-WOLFTPM_DEPENDENCIES = host-pkgconf
 WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
 
 # wolfTPM's source code is released without a configure script,
@@ -19,8 +18,7 @@ WOLFTPM_AUTORECONF = YES
 
 WOLFTPM_CONF_OPTS = \
 	--disable-examples \
-	--enable-devtpm \
-	--with-wolfcrypt=$(STAGING_DIR)/usr
+	--enable-devtpm
 
 # Fix for missing config.rpath in the codebase
 define WOLFTPM_TOUCH_CONFIG_RPATH
@@ -29,4 +27,13 @@ define WOLFTPM_TOUCH_CONFIG_RPATH
 endef
 WOLFTPM_PRE_CONFIGURE_HOOKS += WOLFTPM_TOUCH_CONFIG_RPATH
 
+ifeq ($(BR2_PACKAGE_WOLFSSL),y)
+WOLFTPM_CONF_OPTS += \
+	--enable-wolfcrypt \
+	--with-wolfcrypt=$(STAGING_DIR)/usr
+WOLFTPM_DEPENDENCIES += wolfssl
+else
+WOLFTPM_CONF_OPTS += --disable-wolfcrypt
+endif
+
 $(eval $(autotools-package))
-- 
2.35.1

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

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

* [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR
  2022-06-11 14:35 [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Fabrice Fontaine
@ 2022-06-11 14:35 ` Fabrice Fontaine
  2022-06-11 20:24   ` Dimi Tomov
  2022-06-11 20:19 ` [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Dimi Tomov
  1 sibling, 1 reply; 9+ messages in thread
From: Fabrice Fontaine @ 2022-06-11 14:35 UTC (permalink / raw)
  To: buildroot; +Cc: Dimitar Tomov, Fabrice Fontaine

cpe:2.3:a:wolfssl:wolftpm has never been a valid CPE identifier for this
package:

  https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolftpm

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/wolftpm/wolftpm.mk | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
index f0cf0df0d3..042ccd22e1 100644
--- a/package/wolftpm/wolftpm.mk
+++ b/package/wolftpm/wolftpm.mk
@@ -9,7 +9,6 @@ WOLFTPM_SITE = $(call github,wolfSSL,wolfTPM,v$(WOLFTPM_VERSION))
 WOLFTPM_INSTALL_STAGING = YES
 WOLFTPM_LICENSE = GPL-2.0+
 WOLFTPM_LICENSE_FILES = LICENSE
-WOLFTPM_CPE_ID_VENDOR = wolfssl
 WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
 
 # wolfTPM's source code is released without a configure script,
-- 
2.35.1

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

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

* Re: [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies
  2022-06-11 14:35 [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Fabrice Fontaine
  2022-06-11 14:35 ` [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR Fabrice Fontaine
@ 2022-06-11 20:19 ` Dimi Tomov
  2022-06-11 21:24   ` Fabrice Fontaine
  1 sibling, 1 reply; 9+ messages in thread
From: Dimi Tomov @ 2022-06-11 20:19 UTC (permalink / raw)
  To: Fabrice Fontaine, Thomas Petazzoni; +Cc: buildroot

Hi Fabrice,

1) I advise strongly against this patch in its current form due to 
security concerns. Please find my motivation below.

2) Existing wolfTPM dependencies are propagated from the wolfssl 
package.

3) I am unable to reproduce your build error. Please provide the exact 
commands and order you are running them on a clean buildroot master.

Topic #1

In order to have Man-in-the-middle protection, wolfTPM needs a 
cryptogrpahic provider to set a secure channel of communication between 
the HOST CPU and the TPM 2.0 HSM chip. Without MITM protection, keys and 
other sensitive data can be sniffed, making it pointless to use a TPM 
2.0 security chip in the first place.

Therefore, making the default wolfTPM build work without wolfssl is a 
SECURITY RED FLAG.

Please consider adding an OPTION to build wolfTPM without wolfssl, 
however keep the default wolfTPM build require the wolfssl library.

Topic #2

The threads and dynamic library support dependencies are propagated from 
the wolfssl package and per recommendation of Thomas (adding to our 
discussion).

Both, wolfssl and wolfTPM could be build statically, however this is not 
how I found the wolfssl package working in buildroot. Therefore, I used 
the existing code base and added the wolftpm package accordingly.

Topic #3

It is a good practice when reporting an error to share how it can be 
reproduced. I am unable to reproduce your build error.

Last

I find it difficult to address multiple topics/major changes in one 
patch. I have tried to decouple the topics above and I am ready to 
discuss further. Thank you for providing this feedback. I think there 
are improvements we could make based on what is the expectation and 
buildroot maintainers' requirements.

Regards,
Dimi
-- 
Founder of TPM.dev


On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
> pkg-config is not used by wolftpm and wolfssl dependency is missing 
> (and
> optional) resulting in the following build failure since the addition 
> of
> the package in commit 4bb884a3c61c6b71e33f69453a90eb2a367f64b7:
> 
> configure: error: WolfSSL library not found. You can get it from
> http://www.wolfssl.com/download.html
>         If it's already installed, specify its path using
> --with-wolfcrypt=/dir or --prefix=/dir
> 
> Moreover, BR2_PACKAGE_WOLFSSL_ALL, threads and dynamic libray support
> are not mandatory:
> 
> ./utils/test-pkg -p wolftpm
>                     bootlin-armv5-uclibc [1/6]: OK
>                      bootlin-armv7-glibc [2/6]: OK
>                    bootlin-armv7m-uclibc [3/6]: OK
>                      bootlin-x86-64-musl [4/6]: OK
>                       br-arm-full-static [5/6]: OK
>                             sourcery-arm [6/6]: OK
> 6 builds, 0 skipped, 0 build failed, 0 legal-info failed, 0 show-info 
> failed
> 
> Fixes:
>  -
> http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/wolftpm/Config.in  |  7 -------
>  package/wolftpm/wolftpm.mk | 13 ++++++++++---
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
> index 0fe094f277..741d92aa4e 100644
> --- a/package/wolftpm/Config.in
> +++ b/package/wolftpm/Config.in
> @@ -1,9 +1,5 @@
>  config BR2_PACKAGE_WOLFTPM
>  	bool "wolftpm"
> -	depends on BR2_TOOLCHAIN_HAS_THREADS
> -	depends on !BR2_STATIC_LIBS
> -	select BR2_PACKAGE_WOLFSSL
> -	select BR2_PACKAGE_WOLFSSL_ALL
>  	help
>  	  wolfTPM is a portable, open-source TPM 2.0 stack with
>  	  backward API compatibility, designed for embedded use.
> @@ -11,6 +7,3 @@ config BR2_PACKAGE_WOLFTPM
>  	  resource usage.
> 
>  	  https://www.wolfssl.com/
> -
> -comment "wolftpm needs a toolchain w/ threads, dynamic library"
> -	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
> diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> index ea01eaad6b..f0cf0df0d3 100644
> --- a/package/wolftpm/wolftpm.mk
> +++ b/package/wolftpm/wolftpm.mk
> @@ -10,7 +10,6 @@ WOLFTPM_INSTALL_STAGING = YES
>  WOLFTPM_LICENSE = GPL-2.0+
>  WOLFTPM_LICENSE_FILES = LICENSE
>  WOLFTPM_CPE_ID_VENDOR = wolfssl
> -WOLFTPM_DEPENDENCIES = host-pkgconf
>  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
> 
>  # wolfTPM's source code is released without a configure script,
> @@ -19,8 +18,7 @@ WOLFTPM_AUTORECONF = YES
> 
>  WOLFTPM_CONF_OPTS = \
>  	--disable-examples \
> -	--enable-devtpm \
> -	--with-wolfcrypt=$(STAGING_DIR)/usr
> +	--enable-devtpm
> 
>  # Fix for missing config.rpath in the codebase
>  define WOLFTPM_TOUCH_CONFIG_RPATH
> @@ -29,4 +27,13 @@ define WOLFTPM_TOUCH_CONFIG_RPATH
>  endef
>  WOLFTPM_PRE_CONFIGURE_HOOKS += WOLFTPM_TOUCH_CONFIG_RPATH
> 
> +ifeq ($(BR2_PACKAGE_WOLFSSL),y)
> +WOLFTPM_CONF_OPTS += \
> +	--enable-wolfcrypt \
> +	--with-wolfcrypt=$(STAGING_DIR)/usr
> +WOLFTPM_DEPENDENCIES += wolfssl
> +else
> +WOLFTPM_CONF_OPTS += --disable-wolfcrypt
> +endif
> +
>  $(eval $(autotools-package))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR
  2022-06-11 14:35 ` [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR Fabrice Fontaine
@ 2022-06-11 20:24   ` Dimi Tomov
  2022-06-11 20:38     ` Fabrice Fontaine
  0 siblings, 1 reply; 9+ messages in thread
From: Dimi Tomov @ 2022-06-11 20:24 UTC (permalink / raw)
  To: Fabrice Fontaine; +Cc: buildroot

wolfssl[1] and wolfTPM[2] are open-source products of the same company, 
wolfSSL Inc. [3]

Therefore, the wolfssl and wolftpm package share the same 
WOLFTPM_CPE_ID_VENDOR.

In case that the CPE_ID_VENDOR is incorrect then this is true also for 
the wolfssl package where the value originated.

Thank you for bringing this topic up for discussion.

[1] https://www.wolfssl.com/products/wolfssl/
[2] https://www.wolfssl.com/products/wolftpm/
[3] https://www.wolfssl.com/

Regards,
Dimi
-- 
Founder of TPM.dev

On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
> cpe:2.3:a:wolfssl:wolftpm has never been a valid CPE identifier for 
> this
> package:
> 
> 
> https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolftpm
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/wolftpm/wolftpm.mk | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> index f0cf0df0d3..042ccd22e1 100644
> --- a/package/wolftpm/wolftpm.mk
> +++ b/package/wolftpm/wolftpm.mk
> @@ -9,7 +9,6 @@ WOLFTPM_SITE = $(call
> github,wolfSSL,wolfTPM,v$(WOLFTPM_VERSION))
>  WOLFTPM_INSTALL_STAGING = YES
>  WOLFTPM_LICENSE = GPL-2.0+
>  WOLFTPM_LICENSE_FILES = LICENSE
> -WOLFTPM_CPE_ID_VENDOR = wolfssl
>  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
> 
>  # wolfTPM's source code is released without a configure script,
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR
  2022-06-11 20:24   ` Dimi Tomov
@ 2022-06-11 20:38     ` Fabrice Fontaine
  2022-06-11 20:58       ` Dimi Tomov
  0 siblings, 1 reply; 9+ messages in thread
From: Fabrice Fontaine @ 2022-06-11 20:38 UTC (permalink / raw)
  To: Dimi Tomov; +Cc: Buildroot Mailing List

Le sam. 11 juin 2022 à 22:24, Dimi Tomov <dimi@tpm.dev> a écrit :
>
> wolfssl[1] and wolfTPM[2] are open-source products of the same company,
> wolfSSL Inc. [3]
>
> Therefore, the wolfssl and wolftpm package share the same
> WOLFTPM_CPE_ID_VENDOR.
>
> In case that the CPE_ID_VENDOR is incorrect then this is true also for
> the wolfssl package where the value originated.

wolfssl's CPE ID is correct as it is registered in the NVD NIST database [1].

However, wolftpm product has not been registered to the NVD NIST
database (presumably because no CVEs were found yet in wolftpm).
So, this patch is correct.
If you want to put back WOLFTPM_CPE_ID_VENDOR, I would advise to first
send an email to cpe_dictionary@nist.gov [2] to register wolftpm
product.

[1] https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolfssl
[2] https://nvd.nist.gov/products/cpe
>
> Thank you for bringing this topic up for discussion.
>
> [1] https://www.wolfssl.com/products/wolfssl/
> [2] https://www.wolfssl.com/products/wolftpm/
> [3] https://www.wolfssl.com/
>
> Regards,
> Dimi
> --
> Founder of TPM.dev
>
> On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
> > cpe:2.3:a:wolfssl:wolftpm has never been a valid CPE identifier for
> > this
> > package:
> >
> >
> > https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolftpm
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  package/wolftpm/wolftpm.mk | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> > index f0cf0df0d3..042ccd22e1 100644
> > --- a/package/wolftpm/wolftpm.mk
> > +++ b/package/wolftpm/wolftpm.mk
> > @@ -9,7 +9,6 @@ WOLFTPM_SITE = $(call
> > github,wolfSSL,wolfTPM,v$(WOLFTPM_VERSION))
> >  WOLFTPM_INSTALL_STAGING = YES
> >  WOLFTPM_LICENSE = GPL-2.0+
> >  WOLFTPM_LICENSE_FILES = LICENSE
> > -WOLFTPM_CPE_ID_VENDOR = wolfssl
> >  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
> >
> >  # wolfTPM's source code is released without a configure script,
Best Regards,

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

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

* Re: [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR
  2022-06-11 20:38     ` Fabrice Fontaine
@ 2022-06-11 20:58       ` Dimi Tomov
  0 siblings, 0 replies; 9+ messages in thread
From: Dimi Tomov @ 2022-06-11 20:58 UTC (permalink / raw)
  To: Fabrice Fontaine; +Cc: Buildroot Mailing List

Hello Fabrice,

Now I understand the motivation behind your second patch. I also believe 
that there has been no CVE (yet) in wolfTPM. This patch makes sense.

Thank you for the contribution.

Dimi
-- 
Founder of TPM.dev


On 2022-06-11 11:38 PM, Fabrice Fontaine wrote:
> Le sam. 11 juin 2022 à 22:24, Dimi Tomov <dimi@tpm.dev> a écrit :
>> 
>> wolfssl[1] and wolfTPM[2] are open-source products of the same 
>> company,
>> wolfSSL Inc. [3]
>> 
>> Therefore, the wolfssl and wolftpm package share the same
>> WOLFTPM_CPE_ID_VENDOR.
>> 
>> In case that the CPE_ID_VENDOR is incorrect then this is true also for
>> the wolfssl package where the value originated.
> 
> wolfssl's CPE ID is correct as it is registered in the NVD NIST 
> database [1].
> 
> However, wolftpm product has not been registered to the NVD NIST
> database (presumably because no CVEs were found yet in wolftpm).
> So, this patch is correct.
> If you want to put back WOLFTPM_CPE_ID_VENDOR, I would advise to first
> send an email to cpe_dictionary@nist.gov [2] to register wolftpm
> product.
> 
> [1] 
> https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolfssl
> [2] https://nvd.nist.gov/products/cpe
>> 
>> Thank you for bringing this topic up for discussion.
>> 
>> [1] https://www.wolfssl.com/products/wolfssl/
>> [2] https://www.wolfssl.com/products/wolftpm/
>> [3] https://www.wolfssl.com/
>> 
>> Regards,
>> Dimi
>> --
>> Founder of TPM.dev
>> 
>> On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
>> > cpe:2.3:a:wolfssl:wolftpm has never been a valid CPE identifier for
>> > this
>> > package:
>> >
>> >
>> > https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=cpe%3A2.3%3Aa%3Awolfssl%3Awolftpm
>> >
>> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>> > ---
>> >  package/wolftpm/wolftpm.mk | 1 -
>> >  1 file changed, 1 deletion(-)
>> >
>> > diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
>> > index f0cf0df0d3..042ccd22e1 100644
>> > --- a/package/wolftpm/wolftpm.mk
>> > +++ b/package/wolftpm/wolftpm.mk
>> > @@ -9,7 +9,6 @@ WOLFTPM_SITE = $(call
>> > github,wolfSSL,wolfTPM,v$(WOLFTPM_VERSION))
>> >  WOLFTPM_INSTALL_STAGING = YES
>> >  WOLFTPM_LICENSE = GPL-2.0+
>> >  WOLFTPM_LICENSE_FILES = LICENSE
>> > -WOLFTPM_CPE_ID_VENDOR = wolfssl
>> >  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
>> >
>> >  # wolfTPM's source code is released without a configure script,
> Best Regards,
> 
> Fabrice

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

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

* Re: [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies
  2022-06-11 20:19 ` [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Dimi Tomov
@ 2022-06-11 21:24   ` Fabrice Fontaine
  2022-06-12  4:13     ` Dimi Tomov
  0 siblings, 1 reply; 9+ messages in thread
From: Fabrice Fontaine @ 2022-06-11 21:24 UTC (permalink / raw)
  To: Dimi Tomov; +Cc: Thomas Petazzoni, Buildroot Mailing List

Hi Dimi,

Le sam. 11 juin 2022 à 22:19, Dimi Tomov <dimi@tpm.dev> a écrit :
>
> Hi Fabrice,
>
> 1) I advise strongly against this patch in its current form due to
> security concerns. Please find my motivation below.
>
> 2) Existing wolfTPM dependencies are propagated from the wolfssl
> package.
>
> 3) I am unable to reproduce your build error. Please provide the exact
> commands and order you are running them on a clean buildroot master.
>
> Topic #1
>
> In order to have Man-in-the-middle protection, wolfTPM needs a
> cryptogrpahic provider to set a secure channel of communication between
> the HOST CPU and the TPM 2.0 HSM chip. Without MITM protection, keys and
> other sensitive data can be sniffed, making it pointless to use a TPM
> 2.0 security chip in the first place.
>
> Therefore, making the default wolfTPM build work without wolfssl is a
> SECURITY RED FLAG.
>
> Please consider adding an OPTION to build wolfTPM without wolfssl,
> however keep the default wolfTPM build require the wolfssl library.

I could add a BR2_PACKAGE_WOLFTPM_WOLFSSL option that is enabled by
default in a v2.

However, do you know why wolfTPM can be built without wolfcrypt?
upstream added this option back in 2018 [1] for "cases where the
wolfTPM software was not using parameter encryption or session nonces"
[2].
I'm far from being a TPM expert but could it be that some TPM use
cases don't need encryption between host CPU and HSM chip?

[1] https://github.com/wolfSSL/wolfTPM/commit/d8174d4ef714fc5e6a18e26d0586c6e91c3275ab
[2] https://github.com/wolfSSL/wolfTPM/pull/24

>
> Topic #2
>
> The threads and dynamic library support dependencies are propagated from
> the wolfssl package and per recommendation of Thomas (adding to our
> discussion).

threads dependency must indeed be propagated to this new
BR2_PACKAGE_WOLFTPM_WOLFSSL option.
However, dynamic library support is not needed as, from my
understanding and build testing, wolftpm doesn't need
BR2_PACKAGE_WOLFSSL_ALL (and so can be statically built).

>
> Both, wolfssl and wolfTPM could be build statically, however this is not
> how I found the wolfssl package working in buildroot. Therefore, I used
> the existing code base and added the wolftpm package accordingly.
>
> Topic #3
>
> It is a good practice when reporting an error to share how it can be
> reproduced. I am unable to reproduce your build error.

The build failure was raised by one of the autobuilder, you can easily
reproduce it by retrieving the defconfig from the link in the commit
message [3].
Basically, this build failure will be raised on any "fresh" build
because wolfssl has not been added to WOLFTPM_DEPENDENCIES.
Section [4] of buildroot manual will probably help you to reproduce
this build failure.

[3] http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
[4] https://buildroot.org/downloads/manual/manual.html#_analyzing_and_fixing_autobuild_failures

>
> Last
>
> I find it difficult to address multiple topics/major changes in one
> patch. I have tried to decouple the topics above and I am ready to
> discuss further. Thank you for providing this feedback. I think there
> are improvements we could make based on what is the expectation and
> buildroot maintainers' requirements.
>
> Regards,
> Dimi
> --
> Founder of TPM.dev
>
>
> On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
> > pkg-config is not used by wolftpm and wolfssl dependency is missing
> > (and
> > optional) resulting in the following build failure since the addition
> > of
> > the package in commit 4bb884a3c61c6b71e33f69453a90eb2a367f64b7:
> >
> > configure: error: WolfSSL library not found. You can get it from
> > http://www.wolfssl.com/download.html
> >         If it's already installed, specify its path using
> > --with-wolfcrypt=/dir or --prefix=/dir
> >
> > Moreover, BR2_PACKAGE_WOLFSSL_ALL, threads and dynamic libray support
> > are not mandatory:
> >
> > ./utils/test-pkg -p wolftpm
> >                     bootlin-armv5-uclibc [1/6]: OK
> >                      bootlin-armv7-glibc [2/6]: OK
> >                    bootlin-armv7m-uclibc [3/6]: OK
> >                      bootlin-x86-64-musl [4/6]: OK
> >                       br-arm-full-static [5/6]: OK
> >                             sourcery-arm [6/6]: OK
> > 6 builds, 0 skipped, 0 build failed, 0 legal-info failed, 0 show-info
> > failed
> >
> > Fixes:
> >  -
> > http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  package/wolftpm/Config.in  |  7 -------
> >  package/wolftpm/wolftpm.mk | 13 ++++++++++---
> >  2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
> > index 0fe094f277..741d92aa4e 100644
> > --- a/package/wolftpm/Config.in
> > +++ b/package/wolftpm/Config.in
> > @@ -1,9 +1,5 @@
> >  config BR2_PACKAGE_WOLFTPM
> >       bool "wolftpm"
> > -     depends on BR2_TOOLCHAIN_HAS_THREADS
> > -     depends on !BR2_STATIC_LIBS
> > -     select BR2_PACKAGE_WOLFSSL
> > -     select BR2_PACKAGE_WOLFSSL_ALL
> >       help
> >         wolfTPM is a portable, open-source TPM 2.0 stack with
> >         backward API compatibility, designed for embedded use.
> > @@ -11,6 +7,3 @@ config BR2_PACKAGE_WOLFTPM
> >         resource usage.
> >
> >         https://www.wolfssl.com/
> > -
> > -comment "wolftpm needs a toolchain w/ threads, dynamic library"
> > -     depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
> > diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> > index ea01eaad6b..f0cf0df0d3 100644
> > --- a/package/wolftpm/wolftpm.mk
> > +++ b/package/wolftpm/wolftpm.mk
> > @@ -10,7 +10,6 @@ WOLFTPM_INSTALL_STAGING = YES
> >  WOLFTPM_LICENSE = GPL-2.0+
> >  WOLFTPM_LICENSE_FILES = LICENSE
> >  WOLFTPM_CPE_ID_VENDOR = wolfssl
> > -WOLFTPM_DEPENDENCIES = host-pkgconf
> >  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
> >
> >  # wolfTPM's source code is released without a configure script,
> > @@ -19,8 +18,7 @@ WOLFTPM_AUTORECONF = YES
> >
> >  WOLFTPM_CONF_OPTS = \
> >       --disable-examples \
> > -     --enable-devtpm \
> > -     --with-wolfcrypt=$(STAGING_DIR)/usr
> > +     --enable-devtpm
> >
> >  # Fix for missing config.rpath in the codebase
> >  define WOLFTPM_TOUCH_CONFIG_RPATH
> > @@ -29,4 +27,13 @@ define WOLFTPM_TOUCH_CONFIG_RPATH
> >  endef
> >  WOLFTPM_PRE_CONFIGURE_HOOKS += WOLFTPM_TOUCH_CONFIG_RPATH
> >
> > +ifeq ($(BR2_PACKAGE_WOLFSSL),y)
> > +WOLFTPM_CONF_OPTS += \
> > +     --enable-wolfcrypt \
> > +     --with-wolfcrypt=$(STAGING_DIR)/usr
> > +WOLFTPM_DEPENDENCIES += wolfssl
> > +else
> > +WOLFTPM_CONF_OPTS += --disable-wolfcrypt
> > +endif
> > +
> >  $(eval $(autotools-package))

Best Regards,

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

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

* Re: [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies
  2022-06-11 21:24   ` Fabrice Fontaine
@ 2022-06-12  4:13     ` Dimi Tomov
  2022-06-12  8:44       ` Fabrice Fontaine
  0 siblings, 1 reply; 9+ messages in thread
From: Dimi Tomov @ 2022-06-12  4:13 UTC (permalink / raw)
  To: Fabrice Fontaine; +Cc: Thomas Petazzoni, Buildroot Mailing List

Good Morning Fabrice,

Please find my answers in-line below and thank you for asking good 
questions. It really helps the discussion.

On 2022-06-12 12:24 AM, Fabrice Fontaine wrote:
> Hi Dimi,
> 
> Le sam. 11 juin 2022 à 22:19, Dimi Tomov <dimi@tpm.dev> a écrit :
>> 
>> Hi Fabrice,
>> 
>> 1) I advise strongly against this patch in its current form due to
>> security concerns. Please find my motivation below.
>> 
>> 2) Existing wolfTPM dependencies are propagated from the wolfssl
>> package.
>> 
>> 3) I am unable to reproduce your build error. Please provide the exact
>> commands and order you are running them on a clean buildroot master.
>> 
>> Topic #1
>> 
>> In order to have Man-in-the-middle protection, wolfTPM needs a
>> cryptogrpahic provider to set a secure channel of communication 
>> between
>> the HOST CPU and the TPM 2.0 HSM chip. Without MITM protection, keys 
>> and
>> other sensitive data can be sniffed, making it pointless to use a TPM
>> 2.0 security chip in the first place.
>> 
>> Therefore, making the default wolfTPM build work without wolfssl is a
>> SECURITY RED FLAG.
>> 
>> Please consider adding an OPTION to build wolfTPM without wolfssl,
>> however keep the default wolfTPM build require the wolfssl library.
> 
> I could add a BR2_PACKAGE_WOLFTPM_WOLFSSL option that is enabled by
> default in a v2.

I think this is the best path forward.

> 
> However, do you know why wolfTPM can be built without wolfcrypt?
> upstream added this option back in 2018 [1] for "cases where the
> wolfTPM software was not using parameter encryption or session nonces"
> [2].

I am happy to provide you with more details. Hardware security, 
including TPM, is my passion.

- XOR Parameter Encryption was added in Sep 2020 
https://github.com/wolfSSL/wolfTPM/pull/122
- AES CFB Parameter Encryption was added in Dec 2020 
https://github.com/wolfSSL/wolfTPM/pull/129
- HMAC sessions were also enabled in Dec 2020 also PR129

- Parameter Encryption is the TPM 2.0 capability to encrypt commands 
with sensitive data between the HOST CPU and the TPM 2.0 chip,  like 
generating a new key pair, key authorization to perform 
encryption/decryption/message signing, etc.
- For TPM 2.0 commands that do not support parameter encryption there 
are HMAC sessions to encrypt the communication to and from the TPM 2.0 
chip.
- Without these two capabilities, the SPI/I2C communication between the 
HOST CPU and the TPM 2.0 chip is send plain/raw.

The reason wolfTPM can be built without wolfcrypt is because the TPM 2.0 
chip allows it. Unfortunately, this is how the famous Dolos attack 
happened - https://trmm.net/tpm-sniffing/ , a basic attack that could 
have been easily mitigated if Parameter Encryption was enabled.

Great example of what happens when OEMs do not enable TPM 2.0 Parameter 
Encryption at boot and even Microsoft miss to require it from the 
UEFI/Firmware boot.

> I'm far from being a TPM expert but could it be that some TPM use
> cases don't need encryption between host CPU and HSM chip?

In my professional opinion, there is only one use case where not 
enabling Parameter Encryption may make sense - When a softwareTPM is 
used instead of a discrete TPM chip. Then, the softwareTPM is run in a 
Trusted Execution Environment together with the firmware using wolfTPM, 
thus the TPM 2.0 "chip" is actually in the same isolated memory space as 
the firmware using it (there is no physical bus to be sniffed).

> 
> [1] 
> https://github.com/wolfSSL/wolfTPM/commit/d8174d4ef714fc5e6a18e26d0586c6e91c3275ab
> [2] https://github.com/wolfSSL/wolfTPM/pull/24
> 
>> 
>> Topic #2
>> 
>> The threads and dynamic library support dependencies are propagated 
>> from
>> the wolfssl package and per recommendation of Thomas (adding to our
>> discussion).
> 
> threads dependency must indeed be propagated to this new
> BR2_PACKAGE_WOLFTPM_WOLFSSL option.
> However, dynamic library support is not needed as, from my
> understanding and build testing, wolftpm doesn't need
> BR2_PACKAGE_WOLFSSL_ALL (and so can be statically built).

wolfTPM has functionalities that depend on some very specific wolfSSL 
options like --openssl-extras , --enable-aes-cfb, etc. By using the 
wolfSSL_ALL we guarantee that these are built in. Otherwise there is 
also --enable-wolftpm configure line for wolfssl, however this one does 
not include --openssl-extras that is also needed if we want to export 
TPM public keys to PEM/DER.

I would recommend enabling wolfSSL by default for wolfTPM with the 
following "./configure --enable-wolftpm --enable-opensslextra 
--enable-keygen". This could be an extra wolfSSL package option and then 
wolfTPM selects this option if wolfSSL_ALL is not already chosen. 
Perhaps such improvement is possible?

> 
>> 
>> Both, wolfssl and wolfTPM could be build statically, however this is 
>> not
>> how I found the wolfssl package working in buildroot. Therefore, I 
>> used
>> the existing code base and added the wolftpm package accordingly.
>> 
>> Topic #3
>> 
>> It is a good practice when reporting an error to share how it can be
>> reproduced. I am unable to reproduce your build error.
> 
> The build failure was raised by one of the autobuilder, you can easily
> reproduce it by retrieving the defconfig from the link in the commit
> message [3].
> Basically, this build failure will be raised on any "fresh" build
> because wolfssl has not been added to WOLFTPM_DEPENDENCIES.
> Section [4] of buildroot manual will probably help you to reproduce
> this build failure.

This is odd. I thought depends on does this and guarantees wolfSSL is 
built before wolfTPM.

> 
> [3] 
> http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
> [4] 
> https://buildroot.org/downloads/manual/manual.html#_analyzing_and_fixing_autobuild_failures
> 
>> 
>> Last
>> 
>> I find it difficult to address multiple topics/major changes in one
>> patch. I have tried to decouple the topics above and I am ready to
>> discuss further. Thank you for providing this feedback. I think there
>> are improvements we could make based on what is the expectation and
>> buildroot maintainers' requirements.
>> 
>> Regards,
>> Dimi
>> --
>> Founder of TPM.dev
>> 
>> 
>> On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
>> > pkg-config is not used by wolftpm and wolfssl dependency is missing
>> > (and
>> > optional) resulting in the following build failure since the addition
>> > of
>> > the package in commit 4bb884a3c61c6b71e33f69453a90eb2a367f64b7:
>> >
>> > configure: error: WolfSSL library not found. You can get it from
>> > http://www.wolfssl.com/download.html
>> >         If it's already installed, specify its path using
>> > --with-wolfcrypt=/dir or --prefix=/dir
>> >
>> > Moreover, BR2_PACKAGE_WOLFSSL_ALL, threads and dynamic libray support
>> > are not mandatory:
>> >
>> > ./utils/test-pkg -p wolftpm
>> >                     bootlin-armv5-uclibc [1/6]: OK
>> >                      bootlin-armv7-glibc [2/6]: OK
>> >                    bootlin-armv7m-uclibc [3/6]: OK
>> >                      bootlin-x86-64-musl [4/6]: OK
>> >                       br-arm-full-static [5/6]: OK
>> >                             sourcery-arm [6/6]: OK
>> > 6 builds, 0 skipped, 0 build failed, 0 legal-info failed, 0 show-info
>> > failed
>> >
>> > Fixes:
>> >  -
>> > http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
>> >
>> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>> > ---
>> >  package/wolftpm/Config.in  |  7 -------
>> >  package/wolftpm/wolftpm.mk | 13 ++++++++++---
>> >  2 files changed, 10 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
>> > index 0fe094f277..741d92aa4e 100644
>> > --- a/package/wolftpm/Config.in
>> > +++ b/package/wolftpm/Config.in
>> > @@ -1,9 +1,5 @@
>> >  config BR2_PACKAGE_WOLFTPM
>> >       bool "wolftpm"
>> > -     depends on BR2_TOOLCHAIN_HAS_THREADS
>> > -     depends on !BR2_STATIC_LIBS
>> > -     select BR2_PACKAGE_WOLFSSL
>> > -     select BR2_PACKAGE_WOLFSSL_ALL
>> >       help
>> >         wolfTPM is a portable, open-source TPM 2.0 stack with
>> >         backward API compatibility, designed for embedded use.
>> > @@ -11,6 +7,3 @@ config BR2_PACKAGE_WOLFTPM
>> >         resource usage.
>> >
>> >         https://www.wolfssl.com/
>> > -
>> > -comment "wolftpm needs a toolchain w/ threads, dynamic library"
>> > -     depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
>> > diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
>> > index ea01eaad6b..f0cf0df0d3 100644
>> > --- a/package/wolftpm/wolftpm.mk
>> > +++ b/package/wolftpm/wolftpm.mk
>> > @@ -10,7 +10,6 @@ WOLFTPM_INSTALL_STAGING = YES
>> >  WOLFTPM_LICENSE = GPL-2.0+
>> >  WOLFTPM_LICENSE_FILES = LICENSE
>> >  WOLFTPM_CPE_ID_VENDOR = wolfssl
>> > -WOLFTPM_DEPENDENCIES = host-pkgconf
>> >  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
>> >
>> >  # wolfTPM's source code is released without a configure script,
>> > @@ -19,8 +18,7 @@ WOLFTPM_AUTORECONF = YES
>> >
>> >  WOLFTPM_CONF_OPTS = \
>> >       --disable-examples \
>> > -     --enable-devtpm \
>> > -     --with-wolfcrypt=$(STAGING_DIR)/usr
>> > +     --enable-devtpm
>> >
>> >  # Fix for missing config.rpath in the codebase
>> >  define WOLFTPM_TOUCH_CONFIG_RPATH
>> > @@ -29,4 +27,13 @@ define WOLFTPM_TOUCH_CONFIG_RPATH
>> >  endef
>> >  WOLFTPM_PRE_CONFIGURE_HOOKS += WOLFTPM_TOUCH_CONFIG_RPATH
>> >
>> > +ifeq ($(BR2_PACKAGE_WOLFSSL),y)
>> > +WOLFTPM_CONF_OPTS += \
>> > +     --enable-wolfcrypt \
>> > +     --with-wolfcrypt=$(STAGING_DIR)/usr
>> > +WOLFTPM_DEPENDENCIES += wolfssl
>> > +else
>> > +WOLFTPM_CONF_OPTS += --disable-wolfcrypt
>> > +endif
>> > +
>> >  $(eval $(autotools-package))
> 
> Best Regards,
> 
> Fabrice

-- 
Founder of TPM.dev
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies
  2022-06-12  4:13     ` Dimi Tomov
@ 2022-06-12  8:44       ` Fabrice Fontaine
  0 siblings, 0 replies; 9+ messages in thread
From: Fabrice Fontaine @ 2022-06-12  8:44 UTC (permalink / raw)
  To: Dimi Tomov; +Cc: Thomas Petazzoni, Buildroot Mailing List

Hi Dimi,

Le dim. 12 juin 2022 à 06:13, Dimi Tomov <dimi@tpm.dev> a écrit :
>
> Good Morning Fabrice,
>
> Please find my answers in-line below and thank you for asking good
> questions. It really helps the discussion.
>
> On 2022-06-12 12:24 AM, Fabrice Fontaine wrote:
> > Hi Dimi,
> >
> > Le sam. 11 juin 2022 à 22:19, Dimi Tomov <dimi@tpm.dev> a écrit :
> >>
> >> Hi Fabrice,
> >>
> >> 1) I advise strongly against this patch in its current form due to
> >> security concerns. Please find my motivation below.
> >>
> >> 2) Existing wolfTPM dependencies are propagated from the wolfssl
> >> package.
> >>
> >> 3) I am unable to reproduce your build error. Please provide the exact
> >> commands and order you are running them on a clean buildroot master.
> >>
> >> Topic #1
> >>
> >> In order to have Man-in-the-middle protection, wolfTPM needs a
> >> cryptogrpahic provider to set a secure channel of communication
> >> between
> >> the HOST CPU and the TPM 2.0 HSM chip. Without MITM protection, keys
> >> and
> >> other sensitive data can be sniffed, making it pointless to use a TPM
> >> 2.0 security chip in the first place.
> >>
> >> Therefore, making the default wolfTPM build work without wolfssl is a
> >> SECURITY RED FLAG.
> >>
> >> Please consider adding an OPTION to build wolfTPM without wolfssl,
> >> however keep the default wolfTPM build require the wolfssl library.
> >
> > I could add a BR2_PACKAGE_WOLFTPM_WOLFSSL option that is enabled by
> > default in a v2.
>
> I think this is the best path forward.

I'll send a v2 shortly. Please review it.

>
> >
> > However, do you know why wolfTPM can be built without wolfcrypt?
> > upstream added this option back in 2018 [1] for "cases where the
> > wolfTPM software was not using parameter encryption or session nonces"
> > [2].
>
> I am happy to provide you with more details. Hardware security,
> including TPM, is my passion.
>
> - XOR Parameter Encryption was added in Sep 2020
> https://github.com/wolfSSL/wolfTPM/pull/122
> - AES CFB Parameter Encryption was added in Dec 2020
> https://github.com/wolfSSL/wolfTPM/pull/129
> - HMAC sessions were also enabled in Dec 2020 also PR129
>
> - Parameter Encryption is the TPM 2.0 capability to encrypt commands
> with sensitive data between the HOST CPU and the TPM 2.0 chip,  like
> generating a new key pair, key authorization to perform
> encryption/decryption/message signing, etc.
> - For TPM 2.0 commands that do not support parameter encryption there
> are HMAC sessions to encrypt the communication to and from the TPM 2.0
> chip.
> - Without these two capabilities, the SPI/I2C communication between the
> HOST CPU and the TPM 2.0 chip is send plain/raw.
>
> The reason wolfTPM can be built without wolfcrypt is because the TPM 2.0
> chip allows it. Unfortunately, this is how the famous Dolos attack
> happened - https://trmm.net/tpm-sniffing/ , a basic attack that could
> have been easily mitigated if Parameter Encryption was enabled.
>
> Great example of what happens when OEMs do not enable TPM 2.0 Parameter
> Encryption at boot and even Microsoft miss to require it from the
> UEFI/Firmware boot.

Thanks for all those detailed explanations.

>
> > I'm far from being a TPM expert but could it be that some TPM use
> > cases don't need encryption between host CPU and HSM chip?
>
> In my professional opinion, there is only one use case where not
> enabling Parameter Encryption may make sense - When a softwareTPM is
> used instead of a discrete TPM chip. Then, the softwareTPM is run in a
> Trusted Execution Environment together with the firmware using wolfTPM,
> thus the TPM 2.0 "chip" is actually in the same isolated memory space as
> the firmware using it (there is no physical bus to be sniffed).
>
> >
> > [1]
> > https://github.com/wolfSSL/wolfTPM/commit/d8174d4ef714fc5e6a18e26d0586c6e91c3275ab
> > [2] https://github.com/wolfSSL/wolfTPM/pull/24
> >
> >>
> >> Topic #2
> >>
> >> The threads and dynamic library support dependencies are propagated
> >> from
> >> the wolfssl package and per recommendation of Thomas (adding to our
> >> discussion).
> >
> > threads dependency must indeed be propagated to this new
> > BR2_PACKAGE_WOLFTPM_WOLFSSL option.
> > However, dynamic library support is not needed as, from my
> > understanding and build testing, wolftpm doesn't need
> > BR2_PACKAGE_WOLFSSL_ALL (and so can be statically built).
>
> wolfTPM has functionalities that depend on some very specific wolfSSL
> options like --openssl-extras , --enable-aes-cfb, etc. By using the
> wolfSSL_ALL we guarantee that these are built in. Otherwise there is
> also --enable-wolftpm configure line for wolfssl, however this one does
> not include --openssl-extras that is also needed if we want to export
> TPM public keys to PEM/DER.
>
> I would recommend enabling wolfSSL by default for wolfTPM with the
> following "./configure --enable-wolftpm --enable-opensslextra
> --enable-keygen". This could be an extra wolfSSL package option and then
> wolfTPM selects this option if wolfSSL_ALL is not already chosen.
> Perhaps such improvement is possible?

I'll keep the selection of wolfssl-all, I'll just add an additional comment.

>
> >
> >>
> >> Both, wolfssl and wolfTPM could be build statically, however this is
> >> not
> >> how I found the wolfssl package working in buildroot. Therefore, I
> >> used
> >> the existing code base and added the wolftpm package accordingly.
> >>
> >> Topic #3
> >>
> >> It is a good practice when reporting an error to share how it can be
> >> reproduced. I am unable to reproduce your build error.
> >
> > The build failure was raised by one of the autobuilder, you can easily
> > reproduce it by retrieving the defconfig from the link in the commit
> > message [3].
> > Basically, this build failure will be raised on any "fresh" build
> > because wolfssl has not been added to WOLFTPM_DEPENDENCIES.
> > Section [4] of buildroot manual will probably help you to reproduce
> > this build failure.
>
> This is odd. I thought depends on does this and guarantees wolfSSL is
> built before wolfTPM.

Nope, "depends on" is only used to forbid the activation of wolfTPM if
dependencies are not fulfilled.
You can find some information about kconfig here: [1]

[1] https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html

>
> >
> > [3]
> > http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
> > [4]
> > https://buildroot.org/downloads/manual/manual.html#_analyzing_and_fixing_autobuild_failures
> >
> >>
> >> Last
> >>
> >> I find it difficult to address multiple topics/major changes in one
> >> patch. I have tried to decouple the topics above and I am ready to
> >> discuss further. Thank you for providing this feedback. I think there
> >> are improvements we could make based on what is the expectation and
> >> buildroot maintainers' requirements.
> >>
> >> Regards,
> >> Dimi
> >> --
> >> Founder of TPM.dev
> >>
> >>
> >> On 2022-06-11 05:35 PM, Fabrice Fontaine wrote:
> >> > pkg-config is not used by wolftpm and wolfssl dependency is missing
> >> > (and
> >> > optional) resulting in the following build failure since the addition
> >> > of
> >> > the package in commit 4bb884a3c61c6b71e33f69453a90eb2a367f64b7:
> >> >
> >> > configure: error: WolfSSL library not found. You can get it from
> >> > http://www.wolfssl.com/download.html
> >> >         If it's already installed, specify its path using
> >> > --with-wolfcrypt=/dir or --prefix=/dir
> >> >
> >> > Moreover, BR2_PACKAGE_WOLFSSL_ALL, threads and dynamic libray support
> >> > are not mandatory:
> >> >
> >> > ./utils/test-pkg -p wolftpm
> >> >                     bootlin-armv5-uclibc [1/6]: OK
> >> >                      bootlin-armv7-glibc [2/6]: OK
> >> >                    bootlin-armv7m-uclibc [3/6]: OK
> >> >                      bootlin-x86-64-musl [4/6]: OK
> >> >                       br-arm-full-static [5/6]: OK
> >> >                             sourcery-arm [6/6]: OK
> >> > 6 builds, 0 skipped, 0 build failed, 0 legal-info failed, 0 show-info
> >> > failed
> >> >
> >> > Fixes:
> >> >  -
> >> > http://autobuild.buildroot.org/results/77a93521b909e701ef4e86f18524258b9242c721
> >> >
> >> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> >> > ---
> >> >  package/wolftpm/Config.in  |  7 -------
> >> >  package/wolftpm/wolftpm.mk | 13 ++++++++++---
> >> >  2 files changed, 10 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
> >> > index 0fe094f277..741d92aa4e 100644
> >> > --- a/package/wolftpm/Config.in
> >> > +++ b/package/wolftpm/Config.in
> >> > @@ -1,9 +1,5 @@
> >> >  config BR2_PACKAGE_WOLFTPM
> >> >       bool "wolftpm"
> >> > -     depends on BR2_TOOLCHAIN_HAS_THREADS
> >> > -     depends on !BR2_STATIC_LIBS
> >> > -     select BR2_PACKAGE_WOLFSSL
> >> > -     select BR2_PACKAGE_WOLFSSL_ALL
> >> >       help
> >> >         wolfTPM is a portable, open-source TPM 2.0 stack with
> >> >         backward API compatibility, designed for embedded use.
> >> > @@ -11,6 +7,3 @@ config BR2_PACKAGE_WOLFTPM
> >> >         resource usage.
> >> >
> >> >         https://www.wolfssl.com/
> >> > -
> >> > -comment "wolftpm needs a toolchain w/ threads, dynamic library"
> >> > -     depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
> >> > diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> >> > index ea01eaad6b..f0cf0df0d3 100644
> >> > --- a/package/wolftpm/wolftpm.mk
> >> > +++ b/package/wolftpm/wolftpm.mk
> >> > @@ -10,7 +10,6 @@ WOLFTPM_INSTALL_STAGING = YES
> >> >  WOLFTPM_LICENSE = GPL-2.0+
> >> >  WOLFTPM_LICENSE_FILES = LICENSE
> >> >  WOLFTPM_CPE_ID_VENDOR = wolfssl
> >> > -WOLFTPM_DEPENDENCIES = host-pkgconf
> >> >  WOLFTPM_CONFIG_SCRIPTS = wolftpm-config
> >> >
> >> >  # wolfTPM's source code is released without a configure script,
> >> > @@ -19,8 +18,7 @@ WOLFTPM_AUTORECONF = YES
> >> >
> >> >  WOLFTPM_CONF_OPTS = \
> >> >       --disable-examples \
> >> > -     --enable-devtpm \
> >> > -     --with-wolfcrypt=$(STAGING_DIR)/usr
> >> > +     --enable-devtpm
> >> >
> >> >  # Fix for missing config.rpath in the codebase
> >> >  define WOLFTPM_TOUCH_CONFIG_RPATH
> >> > @@ -29,4 +27,13 @@ define WOLFTPM_TOUCH_CONFIG_RPATH
> >> >  endef
> >> >  WOLFTPM_PRE_CONFIGURE_HOOKS += WOLFTPM_TOUCH_CONFIG_RPATH
> >> >
> >> > +ifeq ($(BR2_PACKAGE_WOLFSSL),y)
> >> > +WOLFTPM_CONF_OPTS += \
> >> > +     --enable-wolfcrypt \
> >> > +     --with-wolfcrypt=$(STAGING_DIR)/usr
> >> > +WOLFTPM_DEPENDENCIES += wolfssl
> >> > +else
> >> > +WOLFTPM_CONF_OPTS += --disable-wolfcrypt
> >> > +endif
> >> > +
> >> >  $(eval $(autotools-package))
> >
> > Best Regards,
> >
> > Fabrice
>
> --
> Founder of TPM.dev

Best Regards,

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

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

end of thread, other threads:[~2022-06-12  8:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-11 14:35 [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Fabrice Fontaine
2022-06-11 14:35 ` [Buildroot] [PATCH 2/2] package/wolftpm: drop WOLFTPM_CPE_ID_VENDOR Fabrice Fontaine
2022-06-11 20:24   ` Dimi Tomov
2022-06-11 20:38     ` Fabrice Fontaine
2022-06-11 20:58       ` Dimi Tomov
2022-06-11 20:19 ` [Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies Dimi Tomov
2022-06-11 21:24   ` Fabrice Fontaine
2022-06-12  4:13     ` Dimi Tomov
2022-06-12  8:44       ` Fabrice Fontaine

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.