Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import/mbedtls 3.4.0 take3 #12

Merged
merged 22 commits into from
Oct 6, 2023

Conversation

jenswi-linaro
Copy link
Owner

Take3 is to use mbedtls_rsa_deduce_primes() and mbedtls_rsa_deduce_crt() to deduce eventual missing fields.

jenswi-linaro and others added 15 commits September 28, 2023 14:15
Imports Mbed TLS 3.4.0 from https://github.com/Mbed-TLS/mbedtls.git
tags mbedtls-3.4.0, v3.4.0.

Files that are not needed are removed:

cd lib/libmbedtls
rm -rf mbedtls
cp -R path/to/mbedtls-3.4.0/mbedtls .
cd mbedtls
rm CMakeLists.txt DartConfiguration.tcl Makefile
rm .gitignore .travis.yml .pylintrc .globalrc .mypy.ini BRANCHES.md
rm include/.gitignore include/CMakeLists.txt library/.gitignore
rm library/CMakeLists.txt library/Makefile
rm mbedtls/cmake/MbedTLSConfig.cmake.in
rm -rf .git .github doxygen configs programs scripts tests visualc
rm -rf 3rdparty ChangeLog.d docs
rm library/psa_* library/mps_*
cd ..
git add mbedtls

This is a complete overwrite of previous code so earlier changes in the
previous branch import/mbedtls-2.28.1 will be added on top of this commit.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Removes default config include/mbedtls/config.h

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.22.0]
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jf: rebased onto mbedtls-2.28.1]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Configures mbedtls to reach outside the imported source tree for
configuration .h file.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.22.0]
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Makes mbedtls_mpi_montg_init(), mbedtls_mpi_montmul() and
mbedtls_mpi_montred() available for external use.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.22.0]
[jf: rebased onto mbedtls-2.27.0, keep static functions]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jf: rebased onto mbedtls-2.28.1]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds mbedtls_mpi_init_mempool() which initializes a mbedtls_mpi struct
to use the mempool mbedtls_mpi_mempool if configured for memory
allocation. All local memory allocation are changed to use
mbedtls_mpi_init_mempool() instead of mbedtls_mpi_init(). This will give
a stack like alloc/free pattern for which the mempool is optimized.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.22.0]
[jf: rebased onto mbedtls-2.27.0, fold fixup commit:
 2df910b ("libmbedtls: mbedtls_mpi_shrink(): fix possible unwanted truncation"),
 adjust macro ECP_MPI_INIT]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0, adjust new coding style]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Increase the count limit when generating the witness in the Rabin-Miller
primality test. The previous number 30 was too low to reliably detect
000000022770A7DC599BC90B2FF981CCB5CF05703344C8F350418AAD as a prime
number.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.22.0]
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
For integrating into OPTEE_OS, it needs add some interfaces:
1. add mbedtls_cipher_clone() for cipher to copy context between two
operations.
2. add mbedtls_cipher_setup_info() for cipher. cipher need to get its
"cipher_info" according the key length, while the key length is not an
input in allocate function. So, use a default key len in the beginning.
It need to reset the cipher info again in init function.
3. add mbedtls_cipher_cmac_setup() for cmac. This function is separate
from mbedtls_cipher_cmac_starts().
4. copy hmac context in md.

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Edison Ai <edison.ai@arm.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebase onto mbedtls-2.22.0]
[jf: rebase onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jf: rebase onto mbedtls-2.28.1, fix typo in comment]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: rebase onto mbedtls-3.4.0, adjust new coding style]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
In NO_CRT mode, Q and P may be invalid. But Q and P will be re-filled
again if PRNG function is valid. So add judgement process if it is
in NO_CRT mode.

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Summer Qin <summer.qin@arm.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebase onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebase onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Initialize W using mbedtls_mpi_init_mempool() instead of memset() to
ensure that the temporary variable uses the designated memory pool if
configured.

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
The W variable is 3072 bytes on AArch64 with MBEDTLS_MPI_WINDOW_SIZE set
to 6 for maximum performance. Instead of allocating such a large
variable on the stack use mempool_alloc().

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
With W allocated in the mempool instead of the stack it is more important
to utilize the mempool in a stack like way.

With this patch allocation and initialization of W is moved to a point
where all following mempool allocations are free before the function
returns.

This reduces maximum memory consumption of mempool in regression case
8101 for an AArch64 TA in from 17280 to 7640 bytes. Figures for an
AArch32 TA are 12040 to 5288 bytes.

Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
When MBEDTLS_ECP_DP_SM2_ENABLED is set, adds support for the ECC curve
defined for the Chinese SM2 algorithm (G/MT 0003 Part 5, [1]).

Link: [1] http://www.gmbz.org.cn/upload/2018-07-24/1532401863206085511.pdf
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
[jf: rebased onto mbedtls-2.27.0]
Signed-off-by: Jerome Forissier <jerome@forissier.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds fault mitigation in mbedtls_rsa_rsassa_pss_verify_ext() by using
the macro FTMN_CALLEE_DONE_MEMCMP() instead of memcmp() when checking
that the hash in the RSA signature is matching the expected value.

FTMN_CALLEE_DONE_MEMCMP() saves on success the result in a thread local
storage if fault mitigations was enabled when the function was called.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
…fy()

Adds fault mitigation in mbedtls_rsa_rsassa_pkcs1_v15_verify() by using
the macro FTMN_CALLEE_DONE_MEMCMP() instead of just
mbedtls_safer_memcmp() when checking that the hash in the RSA signature
is matching the expected value.

FTMN_CALLEE_DONE_MEMCMP() saves on success the result in a thread local
storage if fault mitigations was enabled when the function was called.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
For chacha20 and chachapoly, the *_ctx_clone() function is missing
and therefore the wrong function pointers are assigned to
.ctx_clone_func and .ctx_free_func when MBEDTLS_CHACHA20_C
or MBEDTLS_CHACHAPOLY_C is enabled.

Signed-off-by: Simon Ott <simon.ott@aisec.fraunhofer.de>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: rebased onto mbedtls-3.4.0]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
}

ret = mbedtls_ecp_mul(&key_pair_grp, &key_pair_Q, &key_pair_d,
&key_pair_grp.G, NULL, NULL);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The f_rng parameter to mbedtls_ecp_mul is mandatory in 3.4.

@@ -2495,7 +2511,8 @@ static enum pkcs11_rc set_private_key_data_rsa(struct obj_attrs **head,
mbedtls_mpi_init(&p);
mbedtls_mpi_init(&q);

mbedtls_rc = mbedtls_pk_parse_key(&pk, data, key_size, NULL, 0);
mbedtls_rc = mbedtls_pk_parse_key(&pk, data, key_size,
NULL, 0, NULL, 0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here the f_rng parameter is mandatory for blinding.

Copy link

@larper2axis larper2axis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments about missing f_rng parameters.

@jenswi-linaro jenswi-linaro force-pushed the import/mbedtls-3.4.0-take3 branch from 3619de8 to 4bee995 Compare October 2, 2023 12:43
@jenswi-linaro
Copy link
Owner Author

See my comments about missing f_rng parameters.

Thanks, should be taken care of now.

@@ -604,7 +604,7 @@ static int rng_read(void *ignored __unused, unsigned char *buf, size_t blen)

static int isprime(void *a, int b __unused, int *c)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the __unused annotation

@@ -896,7 +896,8 @@ int32_t TEE_BigIntIsProbablePrime(const TEE_BigInt *op,

get_mpi(&mpi_op, op);

rc = mbedtls_mpi_is_prime(&mpi_op, rng_read, NULL);
rc = mbedtls_mpi_is_prime_ext(&mpi_op, MAX(confidenceLevel, 80U),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the __unused annotation on the parameter confidenceLevel.

@jenswi-linaro
Copy link
Owner Author

Comments addressed. @larper2axis do you want to add an Acked-by or Reviewed-by for some commits? OK if I squash in the updates?

@jforissier
Copy link

  • "libmbedtls: fix cipher_wrap.c for NIST AES Key Wrap mode"
    "libmedtls: core: update to mbedTLS 3.4.0 API"
    "core: ltc: configure internal SHA-1 and SHA-224"
    "core: ltc: configure internal MD5"

    Acked-By: Jerome Forissier <jerome.forissier@linaro.org>

  • "ta: pkcs11: update to mbedTLS 3.4.0 API" + fixups:

    The mbedtls_pk_parse_key() has two new paramters with the new 3.x API

    s/paramters/parameters/

    Acked-By: Jerome Forissier <jerome.forissier@linaro.org>

  • "core: LTC mpi_desc.c: update to mbedTLS 3.4.0 API" and "libutee: update to mbedTLS 3.4.0 API"

    The change to mbedtls_mpi_is_prime_ext() is not mentioned in the commit descriptions.

    Acked-By: Jerome Forissier <jerome.forissier@linaro.org>

  • "libmedtls: core: update to mbedTLS 3.4.0 API"

    "The AES API has changed enough to prevent an efficient implementation of
    crypto_aes_enc_block() so switch the one provided via LTC is used
    instead."

    -> "so switch to the one provided via LTC instead"

    Acked-By: Jerome Forissier <jerome.forissier@linaro.org>

@jforissier
Copy link

Should we be concerned with the CI errors? You've branched off of master haven't you, so I guess everything should build fine?

@jenswi-linaro
Copy link
Owner Author

This one is needed also jenswi-linaro/optee_test#1

For AES Key Wrap mode, the *_ctx_clone() function is missing and
therefore the wrong function pointers are assigned to .ctx_clone_func
and .ctx_free_func when MBEDTLS_NIST_KW_C is enabled.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
The mbedtls_pk_parse_key() has two new parameters with the new 3.x API
in. Fix the call of mbedtls_pk_parse_key().

The individual fields of struct mbedtls_ecp_keypair are no longer
supposed to be accessed directly, instead should helper functions like
mbedtls_ecp_export() be used when some fields needs to be manipulated
directly. Updates create_ec_priv_key_hidden_attributes() to use
mbedtls_ecp_export() in order to be able to manipulate the internal
fields of the ECP key pair.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
MbedTLS 3.4.0 normally doesn't permit access to the internal fields of
mbedtls_mpi. However, mpi_desc.c manipulates the internals of
mbedtls_mpi so define MBEDTLS_ALLOW_PRIVATE_ACCESS to allow accesses to
the internals of mbedtls_mpi.

mbedtls_mpi_is_prime() is replaced by mbedtls_mpi_is_prime_ext() so
switch to use that instead to implement isprime() in the internal LTC
mpi_desc interface.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
@jenswi-linaro
Copy link
Owner Author

Squashed and tags applied. Note that "libutee: update to mbedTLS 3.4.0 API" doesn't have any A-B or R-B tag yet.

@jforissier
Copy link

Squashed and tags applied. Note that "libutee: update to mbedTLS 3.4.0 API" doesn't have any A-B or R-B tag yet.

For that commit:

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

MbedTLS 3.4.0 normally doesn't permit access to the internal fields of
mbedtls_mpi. However, libutee manipulates the internals of mbedtls_mpi
so update define MBEDTLS_ALLOW_PRIVATE_ACCESS to allow accesses to the
internals of mbedtls_mpi.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Update for API changes for DH, ECC, RSA, hash, HMAC, and AES.

MBEDTLS_ALLOW_PRIVATE_ACCESS is always defined to enable access to
private elements in mbedtls structs.

The AES API has changed enough to prevent an efficient implementation of
crypto_aes_enc_block() so switch the one provided via LTC instead.

Remove MBEDTLS_RSA_NO_CRT and temporarily generate the primes and CRT
parts of a private RSA key if missing.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
Adds _CFG_CORE_LTC_SHA1_DESC and _CFG_CORE_LTC_SHA224_DESC to allow
enabling support for SHA-1 and SHA-224 internally in LTC.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
@jenswi-linaro jenswi-linaro force-pushed the import/mbedtls-3.4.0-take3 branch from fd0b529 to 3dcd3fd Compare October 5, 2023 08:36
@jenswi-linaro
Copy link
Owner Author

Tag applied. Are we happy with this? Should I push this branch to the new branch import/mbedtls-3.4.0? So I can create a new PR with a squash commit to be merged to master. Or should I wait a little longer?

@jforissier
Copy link

Tag applied. Are we happy with this? Should I push this branch to the new branch import/mbedtls-3.4.0? So I can create a new PR with a squash commit to be merged to master. Or should I wait a little longer?

I'd like to run a few tests from my release check list first if you don't mind. Hopefully by the end of the day.

@jenswi-linaro
Copy link
Owner Author

Sure, no problem.

@jforissier
Copy link

Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a, mbedtls)

For the record: I checked out a clean QEMUv8 environment and used the following command to build and run the tests:
make -j check CFG_CRYPTOLIB_NAME=mbedtls CFG_CRYPTOLIB_DIR=lib/libmbedtls check XTEST_ARGS="-x pkcs11_1021 -x pkcs11_1022 -x pkcs11_1023"

The failures in the PKCS#11 tests are a known issue with the previous MBedTLS already (OP-TEE#5418).

Adds _CFG_CORE_LTC_MD5_DESC to allow enabling support for MD5 224
internally in LTC.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-By: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a, mbedtls)
@jenswi-linaro jenswi-linaro force-pushed the import/mbedtls-3.4.0-take3 branch from 3dcd3fd to 15058b7 Compare October 6, 2023 14:37
@jenswi-linaro jenswi-linaro merged commit f037308 into import-mbedtls-3.4.0 Oct 6, 2023
0 of 14 checks passed
@jenswi-linaro
Copy link
Owner Author

Pushed to https://github.com/OP-TEE/optee_os/tree/import/mbedtls-3.4.0

@jenswi-linaro jenswi-linaro deleted the import/mbedtls-3.4.0-take3 branch October 6, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants