wolfssl icon indicating copy to clipboard operation
wolfssl copied to clipboard

Add support for Microchip Trust Anchor TA100

Open tmael opened this issue 2 years ago • 9 comments

Description

This PR adds support for Microchip/Atmel Trust Anchor TA100 secure element from their portfolio of CryptoAutomotive™ security ICs for automotive security applications.

Fixes zd#

Testing

How did you test?

Checklist

  • [ ] added tests
  • [ ] updated/added doxygen
  • [ ] updated appropriate READMEs
  • [ ] Updated manual and documentation

tmael avatar Jun 03 '23 00:06 tmael

You have merge conflict. You did not ask for a re-review or assign to anyone else...

dgarske avatar Aug 28 '23 17:08 dgarske

@dgarske, resolved a merge conflict. Apologies for the oversight. I had asked for your review again but forgot to assign it to you. My mistake!

tmael avatar Aug 28 '23 17:08 tmael

@JacobBarthelmeh, Thanks for the review. Please let me know if I missed something.

  1. atmel_ecc_free() was an existing function and changing it would break compatibility.
  2. wc_MakeRsaKey() is necessary to generate a key pair in the TA100.
  3. Supplying the original message to wc_RsaSSL_Verify() is required because TA100 talib_verify() function mandates it.
  4. int wolfCrypt_ATECC_SetConfig(ATCAIfaceCfg* cfg) was an existing function, and I aimed to align the new wc_Microchip_SetSharedDataConfig function's prefix with the rest of the new prototypes, using wc_Microchip_*.

tmael avatar Oct 03 '23 18:10 tmael

@JacobBarthelmeh, Thanks for the review. Please let me know if I missed something.

1. `atmel_ecc_free()` was an existing function and changing it would break compatibility.

Missed that atmel_ecc_free was pre-existing, can we have it be unchanged then and removed from this PR as a code change.

2. `wc_MakeRsaKey()` is necessary to generate a key pair in the TA100.

Marked as resolved.

3. Supplying the original message to `wc_RsaSSL_Verify()` is required because TA100 `talib_verify()` function mandates it.

After looking at cryptoauthlib I see that there appears to not be an alternative to talib_verify() and that yes it requires the full message passed in and does not provide a decrypted buffer in return for RSA. Having the parameters to wc_RsaSSL_Verify be different for one build vs all others might lead to complications, would the way talib_verify() works fit well with our signature.c API?

4. `int wolfCrypt_ATECC_SetConfig(ATCAIfaceCfg* cfg)` was an existing function, and I aimed to align the new `wc_Microchip_SetSharedDataConfig` function's prefix with the rest of the new prototypes, using `wc_Microchip_*`.

OK, would prefer all functions match in the same header. If there is a reason that all new functions added have to be wc_ instead of wolfCrypt then will go ahead and mark as resolved.

JacobBarthelmeh avatar Oct 04 '23 15:10 JacobBarthelmeh

Having the parameters to wc_RsaSSL_Verify be different for one build vs all others might lead to complications, would the way talib_verify() works fit well with our signature.c API?

This could potentially be a good fit for signature.c, which is typically excluded from the wolfBoot build but might be useful for TLS. I assume TA100 would primarily be utilized within the context of secure boot. supporting TLS with RSA is a stretch goal and not within the scope of this particular pull request.

However, we can certainly consider the possibility of extending support for TLS and calling the following functions from the signature.c file:

wc_Microchip_rsa_sign wc_Microchip_rsa_verify

Should we choose to pursue this, it would entail modifying the implementations to exclude hashing. Otherwise, we would end up with double hashing. Another option would be to introduce new functions that do not perform hashing, which could then be called from within signature.c.

tmael avatar Oct 04 '23 21:10 tmael

Just got this running on Microchip's hardware, RSA is not working and has a compile time error even when NO_RSA is defined, Aes is also not working and I think it's because we need to use the specific talib aes functions

jpbland1 avatar Oct 10 '23 19:10 jpbland1

Just got this running on Microchip's hardware, RSA is not working and has a compile time error even when NO_RSA is defined, Aes is also not working and I think it's because we need to use the specific talib aes functions

How are you configuring and building the wolfSSL library? Here's mine:

./configure CFLAGS="-DHAVE_PK_CALLBACKS -DWOLFSSL_ATECC508A_NOIDLE -DECC_USER_CURVES -DWOLFSSL_ATECC_NO_ECDH_ENC -DWOLFSSL_ATECC_DEBUG -O0" --enable-cmac --enable-microchip=100 --with-cryptoauthlib --enable-debug --disable-shared --enable-keygen && make

tmael avatar Oct 10 '23 20:10 tmael

@jpbland1, Please don't hesitate to submit any relevant changes to this pull request.

tmael avatar Oct 13 '23 16:10 tmael

@tmael or @jpbland1 please rebase to resolve merge conflicts. Please assign over to wolfssl-bot to review when ready.

dgarske avatar May 15 '24 18:05 dgarske

Closing lingering PR.

tmael avatar Jun 20 '24 17:06 tmael