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

Libtomcrypt 1.18.2 develop 20240412 #6803

Conversation

jenswi-linaro
Copy link
Contributor

Update libtomcrypt

Once we're happy with the commits here, the branch should be pushed to a new branch with the name import/libtomcrypt-1.18.2-develop-20240412. Then I'll create a squash commit for a new pull request to be merged to the master branch, as described at https://optee.readthedocs.io/en/latest/building/gits/optee_os.html#import-branches

jenswi-linaro and others added 19 commits April 24, 2024 13:27
Delete the current LibTomCrypt source code (core/lib/libtomcrypt/src/*)
as a first step to upgrading to the latest version.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Imports LibTomCrypt (src directory only).

$ cd /tmp
$ git clone -b develop https://github.com/libtom/libtomcrypt.git
$ cd libtomcrypt
$ git describe
v1.18.2-755-gf7e6519fae1e
$ cd .../optee_os/core/lib/libtomcrypt
$ cp -a /tmp/libtomcrypt/src .
$ git add src

Follow-up commits will re-introduce the modifications done locally
in core/lib/libtomcrypt/src and update the integration code to
support the imported version.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
ECC is using a lot (80) temporary variables. These variables
are taken from a static pool, each being of the maximum key size
supported in OP-TEE: 4096bits, times 2 to include
wrapping multiplication in temporary computation.

With the introduction of being able to get temporary variables
of a given size, the current patch optimize the use of the variables
in case of ECC.

Thanks to this patch, the number of temporary variables is back to 50,
and the emulated esram size (QEMU / FVP / HiKey) is back to 200KB.

Note that further optimization can be performed, for ECC and also
for other algorithms (RSA,...).

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Pascal Brand <pascal.brand@linaro.org> (QEMU platform)
Signed-off-by: Pascal Brand <pascal.brand@st.com>
[jf: pick 6d914f6, keep LTC changes only, use new header paths]
[jf: resolve conflicts, applying 0c6c51d on top of LTC v1.18.2-681-ge6be20bf]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Apply the changes from commit de51851 ("arm64: SHA-1 using ARMv8-A
cryptographic extensions") that touch tomcrypt_private.h only. The rest
of the implementation is not under the path that we synchronize with
upstream (i.e., core/lib/libtomcrypt/src).

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Saves 1184 bytes by making prng_descriptor an array of pointers to
descriptors instead of an array of descriptors.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: pick commit 7892cb1 ("ltc: make prng_descriptor a pointer to descriptors")]
[jf: apply change to additional source files]
[jf: squash commit c2f5808 ("ltc: bugfix find_prng()")]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Saves 3288 bytes by making hash_descriptor an array of pointers to
descriptor instead of an array of descriptors.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: pick 3015f56, apply change to additional source files]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: pick 5c148a1, apply change to additional source files]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Saves 3376 bytes by making cipher_descriptor an array of pointers to
descriptor instead of an array of descriptors.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)
Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (QEMU)
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: pick f17691b, apply changes to additional files]
[jf: apply 271db0f onto LTC v1.18.2-681-ge6be20bf]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: pick 7520011, keep LTC changes only, squash fb7ef46 (SPXD fix)]
[jf: add tomcrypt_arm_neon.h from e9fa8da]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
[jw: squash commit ae54175 ("core: LTC use only _CFG_CORE_LTC_ variables")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
This change is part of commit 03121b2 ("core: crypto: libtomcrypt:
fix LTC_CLEAN_STACK bug"). LTC_CLEAN_STACK will be disabled in a later
commit because it is not supported anymore upstream ("LTC_CLEAN_STACK
is considered as broken"), so we keep only the zeromem() part of the
commit.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
This is a cherry-pick of commit 0d1e115 ("core: ltc: enable
thread-safety") and commit a189a57 ("ltc: mutex support, in case of
no mutex").

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Introduce OP-TEE specific function dh_generate_key() from commit
b010477 ("Open-source the TEE Core"). The code is initially from
LibTomCrypt, modified to add the xbits argument.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Add default: statements where missing.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
The functions accel_ecb_encrypt() and accel_ecb_decrypt() in
cipher_descriptor should not modify the input key, so it is better to
declare it 'const'. This helps calling these functions from places where
a const symmetric_key is available.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds the recommended curve parameters for SM2 [1] [2].

[1] http://www.gmbz.org.cn/main/postDetail.html?id=20180724110812
[2] https://tools.ietf.org/id/draft-ribose-openpgp-oscca-00.html

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

Calling x25519_make_key(prng_state *prng, int wprng, curve25519_key *key)
checks if prng_state is NULL. This would require the caller to pass
a valid pointer. Initializing prng_state in some configurations
can get very large. For instance, xtest for X25519 key generation
causes the TA to panic due to stack overrun

F/TC:? 0 trace_syscall:151 syscall OP-TEE#54 (syscall_obj_generate_key)
E/TC:1   Dead canary at end of 'stack_abt[3]' (0xe1a01fc)
E/TC:1   Panic at core/kernel/thread.c:124 <thread_check_canaries>
E/TC:1   TEE load address @ 0xe100000
E/TC:1   Call stack:
E/TC:1    0x0e108934 print_kernel_stack at optee_os/core/arch/arm/kernel/unwind_arm64.c:80
E/TC:1    0x0e113f24 __do_panic at optee_os/core/kernel/panic.c:24
E/TC:1    0x0e116eb4 thread_check_canaries at optee_os/core/kernel/thread.c:115
E/TC:1    0x0e106a60 thread_handle_std_smc at optee_os/core/arch/arm/kernel/thread_optee_smc.c:56

OP-TEE registers a custom prng descriptor (prng_crypto_desc) used for
LTC asymmetric crypto operations and prng_state is not used.

The LTC_ARGCHK(prng != NULL) check is not present in the LTC key generation
functions for ECC, RSA, DH and DSA implementations.

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Sohaib ul Hassan <sohaib.ul.hassan@unikie.com>
LTC_DER is always defined when LTC_NO_PKCS is NOT defined (which is our
case). Since LTC_DER depends on LTC_MPI let's set that one at the same
time. Fixes the following build error:

 $ make CFG_CRYPTO=n
 [...]
 core/lib/libtomcrypt/src/headers/tomcrypt_custom.h:629:5: error: #error ASN.1 DER requires MPI functionality
   629 |    #error ASN.1 DER requires MPI functionality
       |     ^~~~~

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Add the remaining LTC code from
c54b634 ("core: crypto: cleanup and fix CE accelerated AES CTR").
Most of that commit was made irrelevant after the move of accelerated
code out of core/lib/libtomcrypt/src, except for the bits that touch
ctr_encrypt() in core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c.

Re-introduce the needed change. Fixes failures in xtest 4003 when
CFG_CRYPTO_WITH_CE=y.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
[JW: update commit message]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
…key()

For the same reasons as in commit 2d7740f ("core: libtomcrypt:
Remove prng_state* NULL pointer check from x25519_make_key()"), remove
the NULL pointer check in ed25519_make_key().

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds fault mitigations in crypto_acipher_rsassa_verify() and dependent
functions in libTomCrypt in order to include the critical final
memcompare.

This fault mitigation is only enabled with the calling function enabled
fault mitigations and CFG_CORE_FAULT_MITIGATION is 'y'.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jforissier
Copy link
Contributor

  • For "core: ltc: update for libtomcrypt changes":
    Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
  • It looks like 8d4ddb4 is missing
  • I ran some of my release test on QEMUv8 and found no issue.

vsatoes and others added 2 commits May 2, 2024 13:25
When running a test with CFG_FAULT_MITIGATION=y and with a corrupted
message, hash verification fails and panic TEE core:

F/TC:? 0 trace_syscall:149 syscall OP-TEE#40 (syscall_asymm_verify)
E/TC:2 0 Panic at lib/libutils/ext/fault_mitigation.c:87 <___ftmn_callee_done_check>
E/TC:2 0 TEE load address @ 0x43200000
E/TC:2 0 Call stack:
E/TC:2 0  0x4320a9f0 print_kernel_stack at optee-os/core/arch/arm/kernel/unwind_arm64.c:91
E/TC:2 0  0x432203fc __do_panic at optee-os/core/kernel/panic.c:26 (discriminator 32)
E/TC:2 0  0x4327d324 ___ftmn_callee_done_check at optee-os/lib/libutils/ext/fault_mitigation.c:87
E/TC:2 0  0x43263aac __ftmn_callee_done_check at optee-os/lib/libutils/ext/include/fault_mitigation.h:349
E/TC:2 0  0x43258408 sw_crypto_acipher_rsassa_verify at optee-os/core/lib/libtomcrypt/rsa.c:669
E/TC:2 0  0x43247ecc syscall_asymm_verify at optee-os/core/tee/tee_svc_cryp.c:4420
E/TC:2 0  0x43206d18 scall_do_call at optee-os/core/arch/arm/kernel/arch_scall_a64.S:140
E/TC:2 0  0x43206798 thread_scall_handler at optee-os/core/arch/arm/kernel/thread.c:1115
E/TC:2 0  0x432043e8 el0_svc at optee-os/core/arch/arm/kernel/thread_a64.S:850

When CFG_FAULT_MITIGATION flag is enabled, ftmn_set_check_res_memcmp()
is used on the verification of RSA hash. ftmn.check.res is set with the
return value of the hash comparison. Since memcmp() is used, this can
be 0, when hash matches, or any non-zero number when hash does not match.

However, the value stored on ftmn.check.res is later compared with the
result of the signature comparison (!*stat), which can assume only two
values, 1==valid or 0==invalid.

With that, when ftmn_set_check_res_memcmp() returns any non-zero number,
force ftmn.check.res to 1 so that it matches the check with later
FTMN_CALLEE_DONE_CHECK().

Signed-off-by: Felix Freimann <felix.freimann@mediatek.com>
Signed-off-by: Vitor Sato Eschholz <vsatoes@baylibre.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Adjust as needed to into account the changes in upstream since the last
sync:
- New file aes_desc.c added
- rsa_decrypt_key_ex() the arguments mgf_hash and lparam_hash replace
  the previous hash_idx argument introduced with commit 63091c9e5c77
  ("Add possibility to use different hash algorithms in RSAES-OAEP") LTC
  upstream
- struct rijndael_key now uses pointer for eK and dK so where a symmetrik
  AES key is initialized those two pointers must be updated. Done in:
  + core/lib/libtomcrypt/aes.c crypto_aes_expand_enc_key() and
    crypto_aes_enc_block()
  + core/lib/libtomcrypt/aes_accel.c rijndael_setup()

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
@jenswi-linaro jenswi-linaro force-pushed the libtomcrypt-1.18.2-develop-20240412 branch from d8f694a to 165e1fe Compare May 2, 2024 11:26
@jenswi-linaro
Copy link
Contributor Author

Cherry-picked "core: ltc: rsa_verify_hash: fix panic on hash mismatch" and tag applied.

@jenswi-linaro
Copy link
Contributor Author

Is it OK to push this to import/libtomcrypt-1.18.2-develop-20240412 now?

@jenswi-linaro
Copy link
Contributor Author

I'll push this to import/libtomcrypt-1.18.2-develop-20240412 if there are no further comments.

@jforissier
Copy link
Contributor

@jenswi-linaro fine with me

@jenswi-linaro
Copy link
Contributor Author

I've pushed this to import/libtomcrypt-1.18.2-develop-20240412.

@jenswi-linaro jenswi-linaro deleted the libtomcrypt-1.18.2-develop-20240412 branch May 6, 2024 09:15
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.

None yet

4 participants