Re: [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add trusted boot option

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Javad,

On Thu, 28 Mar 2024 19:12:47 +0100
Javad Rahimipetroudi via buildroot <buildroot@buildroot.org> wrote:

> This patch adds the required fields to enable Trusted Board Boot in
> TF-A. The users should provide ROT_KEY private key to build the TF-A in
> this mode. The ROT_KEY is used to sign the FIP image during the TF-A
> build. Furthermore, the source code of the mbedTLS is also used during
> the build process.
> 
> Signed-off-by: Javad Rahimipetroudi <javad.rahimipetroudi@mind.be>

Thanks for this contribution! It looks good, I only have one
doubt/issue with it.

> +ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_TRUSTED_BOOT),y)
> +ARM_TRUSTED_FIRMWARE_TRUSTED_BOOT_ROT_KEY = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_ROT_KEY))
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
> +	TRUSTED_BOARD_BOOT=1 \
> +	MBEDTLS_DIR=$(MBEDTLS_SRCDIR) \

This re-use of the mbedtls source code, outside of the mbedtls package
build itself sounded a bit suspicious to me. Indeed, mbedtls being a
dependency of arm-trusted-firmware, it means that 
$(MBEDTLS_SRCDIR) will contain an already built mbedtls. Would this be
a problem?

Looking at the arm-trusted-firmware build logic, it looks like it
isn't: the TF-A build system will rebuild in its own folder the mbedtls
library. However, when I see:

LIBMBEDTLS_SRCS         += $(addprefix ${MBEDTLS_DIR}/library/,         \
                                        aes.c                           \
                                        asn1parse.c                     \
                                        asn1write.c                     \
                                        cipher.c                        \
                                        cipher_wrap.c                   \
                                        constant_time.c                 \
                                        hash_info.c                     \
                                        memory_buffer_alloc.c           \
                                        oid.c                           \
                                        platform.c                      \
                                        platform_util.c                 \
                                        bignum.c                        \
                                        bignum_core.c                   \
                                        gcm.c                           \
                                        md.c                            \
                                        pk.c                            \
                                        pk_wrap.c                       \
                                        pkparse.c                       \
                                        pkwrite.c                       \
                                        sha256.c                        \
                                        sha512.c                        \
                                        ecdsa.c                         \
                                        ecp_curves.c                    \
                                        ecp.c                           \
                                        rsa.c                           \
                                        rsa_alt_helpers.c               \
                                        x509.c                          \
                                        x509_crt.c                      \
                                        )

in the TF-A build system, I'm a bit scared, because it means that there
is a pretty tight coupling between the version of TF-A and the version
of mbedtls. If we update mbedtls to a newer version which has an
additional source file... TF-A would have to be updated accordingly.
This looks a bit "meh" to me.

However, I don't really have a super great alternative to offer. The
only alternative that I can think of is to have
boot/arm-trusted-firmware/ download/extract its own copy of mbedtls, so
that (1) we control its version independently of the mbedtls package
and (2) we don't poke into the mbedtls source directory.

Let's see what the other maintainers think of this somewhat special
situation.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com

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