Re: [PATCH v4 00/29] Integrate MbedTLS v3.6 LTS with U-Boot

[Tom Rini <trini@konsulko.com>]

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

On Tue, Jul 23, 2024 at 03:24:29PM -0400, Raymond Mao wrote:
> Hi Tom,
> 
> On Tue, 2 Jul 2024 at 21:26, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Tue, Jul 02, 2024 at 11:22:36AM -0700, Raymond Mao wrote:
> >
> > > Integrate MbedTLS v3.6 LTS (currently v3.6.0-RC1) with U-Boot.
> > >
> > > Motivations:
> > > ------------
> > >
> > > 1. MbedTLS is well maintained with LTS versions.
> > > 2. LWIP is integrated with MbedTLS and easily to enable HTTPS.
> > > 3. MbedTLS recently switched license back to GPLv2.
> > >
> > > Prerequisite:
> > > -------------
> > >
> > > This patch series requires mbedtls git repo to be added as a
> > > subtree to the main U-Boot repo via:
> > >     $ git subtree add --prefix lib/mbedtls/external/mbedtls \
> > >           https://github.com/Mbed-TLS/mbedtls.git \
> > >           v3.6.0 --squash
> > > Moreover, due to the Windows-style files from mbedtls git repo,
> > > we need to convert the CRLF endings to LF and do a commit manually:
> > >     $ git add --renormalize .
> > >     $ git commit
> > >
> > > New Kconfig options:
> > > --------------------
> > >
> > > `MBEDTLS_LIB` is for MbedTLS general switch.
> > > `MBEDTLS_LIB_CRYPTO` is for replacing original digest and crypto libs
> > with
> > > MbedTLS.
> > > `MBEDTLS_LIB_X509` is for replacing original X509, PKCS7, MSCode, ASN1,
> > > and Pubkey parser with MbedTLS.
> > > `MBEDTLS_LIB_TLS` is for SSL/TLS (Disabled until LWIP port for MbedTLS is
> > > ready).
> > > `LEGACY_CRYPTO` is introduced as a main switch for legacy crypto library.
> > > `LEGACY_CRYPTO_BASIC` is for the basic crypto functionalities and
> > > `LEGACY_CRYPTO_CERT` is for the certificate related functionalities.
> > > For each of the algorithm, a pair of `<alg>_LEGACY` and `<alg>_MBEDTLS`
> > > Kconfig options are introduced. Meanwhile, `SPL_` Kconfig options are
> > > introduced.
> > >
> > > In this patch set, MBEDTLS_LIB, MBEDTLS_LIB_CRYPTO and MBEDTLS_LIB_X509
> > > are by default enabled in qemu_arm64_defconfig for testing purpose.
> > >
> > > Patches for external MbedTLS project:
> > > -------------------------------------
> > >
> > > Since U-Boot uses Microsoft Authentication Code to verify PE/COFFs
> > > executables which is not supported by MbedTLS at the moment,
> > > addtional patches for MbedTLS are created to adapt with the EFI loader:
> > > 1. Decoding of Microsoft Authentication Code.
> > > 2. Decoding of PKCS#9 Authenticate Attributes.
> > > 3. Extending MbedTLS PKCS#7 lib to support multiple signer's
> > certificates.
> > > 4. MbedTLS native test suites for PKCS#7 signer's info.
> > >
> > > All above 4 patches (tagged with `mbedtls/external`) are submitted to
> > > MbedTLS project and being reviewed, eventually they should be part of
> > > MbedTLS LTS release.
> > > But before that, please merge them into U-Boot, otherwise the building
> > > will be broken when MBEDTLS_LIB_X509 is enabled.
> > >
> > > See below PR link for the reference:
> > > https://github.com/Mbed-TLS/mbedtls/pull/9001
> > >
> > > Miscellaneous:
> > > --------------
> > >
> > > Optimized MbedTLS library size by tailoring the config file
> > > and disabling all unnecessary features for EFI loader.
> > > From v2, original libs (rsa, asn1_decoder, rsa_helper, md5, sha1, sha256,
> > > sha512) are completely replaced when MbedTLS is enabled.
> > > From v3, the size-growth is slightly reduced by refactoring Hash
> > functions.
> > >
> > > Target(QEMU arm64) size-growth when enabling MbedTLS:
> > > v1: 6.03%
> > > v2: 4.66%
> > > v3 & v4: 4.55%
> > >
> > > Please see the latest output of bloat-o-meter for the reference of the
> > > size-growth on QEMU arm64 target [1].
> > >
> > > Tests done:
> > > -----------
> > >
> > > EFI Secure Boot test (EFI variables loading and verifying, EFI signed
> > image
> > > verifying and booting) via U-Boot console.
> > > EFI Secure Boot and Capsule sandbox test passed.
> > >
> > > Known issues:
> > > -------------
> > >
> > > None.
> > >
> > > [1]: bloat-o-meter output between disabling/enabling MbedTLS (QEMU arm64)
> > > ```
> > > add/remove: 206/81 grow/shrink: 19/17 up/down: 55548/-17495 (38053)
> >
> > bloat-o-meter is a bit off then, since buildman shows:
> > u-boot: add: 243/-17, grow: 18/-17 bytes: 65723/-8480 (57243)
> >
> > (Please use buildman for the size comparisons in the future).
> >
> 
> I have a problem with buildman.
> As I followed the buildman/README.rst and run below command, but cannot get
> any
> output size summary. Is anything missing? I saw some artifacts of building
> each
> commit being generated in the upper dir though.
> ```
> ./tools/buildman/buildman -b <my_branch_name> --boards qemu_arm64 -sSdB
> ```
> I have set my branch upstream to upstream/next.

You have to tell it twice, once to build and a second to summarize
things. My wrapper looks like:
#!/bin/bash

# Initial and constant buildman args
ARGS="-devl -PEWM"
ALL=0
KEEP=0

# Find our arguments
while test $# -ne 0; do
	if [ "$1" == "--all" ]; then
		ALL=1
		shift 1
	elif [ "$1" == "--branch" ]; then
		BRANCH=$2
		shift 2
	elif [ "$1" == "--keep" ]; then
		KEEP=1
		ARGS="$ARGS -k"
		shift 1
	elif [ "$1" == "--board" ]; then
		MACHINE="--board $2"
		OUTDIR=/tmp/$2
		shift 2
	else
		MACHINE=$1
		shift 1
	fi
done

OUTDIR=${OUTDIR:-/tmp/$MACHINE}

if [ -z "$MACHINE" ]; then
	echo Usage: $0 MACHINE [--all] [--keep] [--branch BRANCH]
	exit 1
fi

# If not all, then only first/last
if [ $ALL -ne 1 ]; then
	ARGS="$ARGS --step 0"
fi

if [ ! -z $BRANCH ]; then
	ARGS="$ARGS -b $BRANCH"
else
	ARGS="$ARGS -b `git rev-parse --abbrev-ref HEAD`"
fi

mkdir -p ${OUTDIR}

export SOURCE_DATE_EPOCH=`date +%s`
./tools/buildman/buildman -o ${OUTDIR} $ARGS -SBC $MACHINE
./tools/buildman/buildman -o ${OUTDIR} $ARGS -SsB $MACHINE

[ $KEEP -eq 0 ] && rm -rf ${OUTDIR}

-- 
Tom

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