[PATCH v6 00/28] Integrate MbedTLS v3.6 LTS with U-Boot

Fri Sep 6 19:20:17 CEST 2024

Hi Tom,

On Fri, 6 Sept 2024 at 11:27, Tom Rini <trini at konsulko.com> wrote:

> On Fri, Sep 06, 2024 at 10:50:15AM -0400, Raymond Mao wrote:
> > Hi Simon,
> >
> > On Thu, 5 Sept 2024 at 20:43, Simon Glass <sjg at chromium.org> wrote:
> >
> > > Hi Raymond,
> > >
> > > On Tue, 3 Sept 2024 at 08:59, Raymond Mao <raymond.mao at linaro.org>
> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sat, 17 Aug 2024 at 11:58, Simon Glass <sjg at chromium.org> wrote:
> > > >>
> > > >> Hi Raymond,
> > > >>
> > > >> On Fri, 16 Aug 2024 at 15:44, Raymond Mao <raymond.mao at linaro.org>
> > > wrote:
> > > >> >
> > > >> > Integrate MbedTLS v3.6 LTS (currently v3.6.0) 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.
> > > >> > `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 and
> sandbox_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.
> > > >> > From v6, smaller implementations for SHA256 and SHA512 are
> enabled and
> > > >> > target size reduce significantly.
> > > >> > Target(QEMU arm64) size-growth when enabling MbedTLS:
> > > >> > v1: 6.03%
> > > >> > v2: 4.66%
> > > >> > v3 - v5: 4.55%
> > > >> > v6: 2.90%
> > > >> >
> > > >> > Please see the latest output from buildman for size-growth on QEMU
> > > arm64,
> > > >> > Sandbox and Nanopi A64. [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.
> > > >>
> > > >> I wonder if we could leave out the SHA stuff? The algorithms are
> > > >> stable and this would seem to avoid much of the size growth, and all
> > > >> the pain of trying to integrate another yet another hashing layer
> (we
> > > >> already have normal, progressive and h/w acceleration, plus
> > > >> UCLASS_HASH which h/w acceleration should use but that migration
> never
> > > >> happened). I struggle to see any benefit in replacing U-Boot's very
> > > >> solid hashing infra with something else, particularly as this series
> > > >> adds yet another. Better to invest the time to refactor it. I asked
> > > >> about this before and was told that it would happen 'later'. Let's
> > > >> just not change it at all, then it is more likely someone will sort
> it
> > > >> out.
> > > >>
> > > > Unfortunately, MbedTLS depends on its own digest layer. Unless we
> patch
> > > MbedTLS
> > > > to allow an external digest library from U-Boot ...
> > >
> > > Yes that sounds best. It looks like only a few call sites, so it
> > > should be a matter of leaving out the MbedTLS code and adding some
> > > static inlines.
> > >
> > Inspired by Ilias's reply to patch #7, though we can use the MbedTLS hash
> > alternative options,
> > we still need to convert all U-Boot hash APIs to adapt to the MbedTLS
> style.
> > This will impact all callers in U-Boot and I don't think it worth to do,
> at
> > least now.
> > As the first patch set to introduce MbedTLS to U-Boot with turning on all
> > necessary features,
> > I think this patch set is in the best way with an overall consideration.
>
> To this point, you could just create wrappers? And perhaps mark those as
> inline, if they're small enough?
>
> The original hash_xxx functions are relatively small and I can move them to
the header file as inline.
And the MbedTLS wrappers will be kept in common/hash.c.

Regards,
Raymond