[Buildroot] [PATCH 1/2] package/wolftpm: fix dependencies

Sat Jun 11 21:24:42 UTC 2022

Hi Dimi,

Le sam. 11 juin 2022 à 22:19, Dimi Tomov <dimi at 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 at 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