| Message ID | 20220611143544.844613-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] package/wolftpm: fix dependencies | expand |
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
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
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
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
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))
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(-)