Skip to content

Commit 7f266a4

Browse files
committed
[TA-100] Address review feedback (Copilot + Fenrir)
Copilot fixes: - atmel.c: ATCA_ENABLE_DEPRECATED I2C path now uses ATECC_I2C_ADDR instead of slave_address=1 (matches the non-deprecated path). - atmel.c: capture and propagate atmel_createHandles() return value; abort init via WC_HW_E if handle creation fails. - atmel.h: include calib_aes_gcm.h with the same <calib/...> form used for calib_command.h so a single -I (.../include or .../include/cryptoauthlib) resolves both. - configure.ac: drop the duplicated AM_CONDITIONAL([BUILD_CRYPTOAUTHLIB]) (kept only in the consolidated section near the end). - settings.h: remove leftover commented-out '#ifdef WOLFSSL_ATECC508A'. - benchmark.c: drop the broken TA100 wc_RsaSSL_Verify branch (it passed message/enc as if they were sig/out). - test.c: stop calling atmel_ecc_free() with the slot-TYPE enum constants; wc_ecc_free(userA/userB) already releases the allocated slots. - ecc.c (microchip_curve_id_for_key): switch on key->dp->id, not size, so SECP256K1 / BRAINPOOLP256R1 are not silently mapped to SECP256R1. Helper is now defined for ATECC508A/608A as well, fixing the TA100-only gating that broke ATECC builds. - ecc.c (_ecc_make_key_ex): keep ATECC508A/608A's curve check at SECP256R1-only (hardware does not support the wider curve set); TA100 retains the multi-curve list. Fenrir fixes: - ecc.c (wc_ecc_init_ex): under TA100 + ALT_ECC_SIZE the pubkey x/y/z pointers must be aimed at key->pubkey.xyz[] (with alt_fp_init) before mp_init_multi - otherwise mp_init_multi dereferenced NULL. - atmel.c (atmel_get_rev_info): check atcab_wakeup return and bail out via atmel_ecc_translate_err before calling atcab_info. - atmel.c (atmel_ecc_create_pms, TA100+ECDH_ENC): pass MAP_TO_HANDLE(slotId) (the ephemeral private-key handle) into talib_ecdh_compat instead of MAP_TO_HANDLE(slotIdEnc). - atmel.c (wc_Microchip_rsa_create_key): on any failure after the first talib_create_element succeeds, delete the previously created handle(s) and clear rKeyH/uKeyH so device elements are not leaked. - aes.c (wc_AesGcmEncrypt / wc_AesGcmDecrypt TA100 fast paths): replace '(authInSz + sz) <= MAX' with bounds on each operand individually so word32 wraparound cannot bypass the 996-byte hardware limit. - rsa.c (RsaPrivateDecryptEx): drop the TA100 RSA_PUBLIC_DECRYPT short-circuit. wc_Microchip_rsa_verify expects (digest, digestLen, sig, sigLen, ...) and the verified flag must be honored; the proper TA100 fast-path already lives in wc_RsaPSS_CheckPadding_ex2.
1 parent 20821c9 commit 7f266a4

9 files changed

Lines changed: 69 additions & 57 deletions

File tree

configure.ac

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3128,8 +3128,6 @@ AS_IF([test "x$with_cryptoauthlib" != "xno"], [
31283128
])
31293129
])
31303130

3131-
AM_CONDITIONAL([BUILD_CRYPTOAUTHLIB], [test "x$ENABLED_CRYPTOAUTHLIB" = "xyes"])
3132-
31333131
# TropicSquare TROPIC01
31343132
# Example: "./configure --with-tropic01=/home/pi/libtropic"
31353133
ENABLED_TROPIC01="no"

wolfcrypt/benchmark/benchmark.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10575,13 +10575,8 @@ static void bench_rsa_helper(int useDeviceID,
1057510575
1, &times, ntimes, &pending)) {
1057610576
#if !defined(WOLFSSL_RSA_VERIFY_INLINE) && \
1057710577
!defined(WOLFSSL_RSA_PUBLIC_ONLY)
10578-
#if defined(WOLFSSL_MICROCHIP_TA100)
10579-
ret = wc_RsaSSL_Verify(message, len,
10580-
enc[i], rsaKeySz/8, rsaKey[i]);
10581-
#else
1058210578
ret = wc_RsaSSL_Verify(enc[i], idx, out[i],
1058310579
rsaKeySz/8, rsaKey[i]);
10584-
#endif
1058510580
#elif defined(USE_CERT_BUFFERS_2048)
1058610581
XMEMCPY(enc[i], rsa_2048_sig, sizeof(rsa_2048_sig));
1058710582
idx = sizeof(rsa_2048_sig);

wolfcrypt/src/aes.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10161,7 +10161,8 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1016110161
aes->keylen == TA_KEY_TYPE_AES128_SIZE &&
1016210162
ivSz == TA_AES_GCM_IV_LENGTH &&
1016310163
authTagSz == TA_AES_GCM_TAG_LENGTH &&
10164-
(authInSz + sz) <= TA_AES_GCM_MAX_DATA_SIZE) {
10164+
sz <= TA_AES_GCM_MAX_DATA_SIZE &&
10165+
authInSz <= (word32)(TA_AES_GCM_MAX_DATA_SIZE - sz)) {
1016510166
return wc_Microchip_AesGcmEncrypt(
1016610167
aes, out, in, sz,
1016710168
iv, ivSz,
@@ -10905,7 +10906,8 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1090510906
aes->keylen == TA_KEY_TYPE_AES128_SIZE &&
1090610907
ivSz == TA_AES_GCM_IV_LENGTH &&
1090710908
authTagSz == TA_AES_GCM_TAG_LENGTH &&
10908-
(authInSz + sz) <= TA_AES_GCM_MAX_DATA_SIZE) {
10909+
sz <= TA_AES_GCM_MAX_DATA_SIZE &&
10910+
authInSz <= (word32)(TA_AES_GCM_MAX_DATA_SIZE - sz)) {
1090910911
return wc_Microchip_AesGcmDecrypt(
1091010912
aes, out, in, sz, iv, ivSz,
1091110913
authTag, authTagSz, authIn, authInSz);

wolfcrypt/src/ecc.c

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5660,16 +5660,15 @@ int wc_ecc_make_pub_ex(ecc_key* key, ecc_point* pubOut, WC_RNG* rng)
56605660
return err;
56615661
}
56625662

5663-
#if defined(WOLFSSL_MICROCHIP_TA100)
5664-
static WC_INLINE int ta100_curve_id_for_key(const ecc_key* key)
5663+
#if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \
5664+
defined(WOLFSSL_MICROCHIP_TA100)
5665+
/* Resolve the curve id to pass to the Microchip backend. Keep the curve
5666+
* distinction by id (not size): SECP256R1, SECP256K1 and BRAINPOOLP256R1 are
5667+
* all 32 bytes and must NOT be collapsed onto SECP256R1. */
5668+
static WC_INLINE int microchip_curve_id_for_key(const ecc_key* key)
56655669
{
56665670
if (key != NULL && key->dp != NULL) {
5667-
switch (key->dp->size) {
5668-
case 28: return ECC_SECP224R1;
5669-
case 32: return ECC_SECP256R1;
5670-
case 48: return ECC_SECP384R1;
5671-
default: return key->dp->id;
5672-
}
5671+
return key->dp->id;
56735672
}
56745673
return ECC_CURVE_DEF;
56755674
}
@@ -5764,24 +5763,21 @@ static int _ecc_make_key_ex(WC_RNG* rng, int keysize, ecc_key* key,
57645763

57655764
#if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \
57665765
defined(WOLFSSL_MICROCHIP_TA100)
5767-
#if !defined(WOLFSSL_MICROCHIP_TA100)
5766+
#if defined(WOLFSSL_MICROCHIP_TA100)
5767+
/* TA100 supports multiple curves natively. */
57685768
if (key->dp->id == ECC_SECP256R1 ||
57695769
key->dp->id == ECC_SECP224R1 ||
57705770
key->dp->id == ECC_SECP384R1 ||
57715771
key->dp->id == ECC_SECP256K1 ||
57725772
key->dp->id == ECC_BRAINPOOLP256R1) {
5773-
/* supports more than ECC256R1 curve */
57745773
#else
5775-
if (key->dp->id == ECC_SECP256R1 ||
5776-
key->dp->id == ECC_SECP224R1 ||
5777-
key->dp->id == ECC_SECP384R1 ||
5778-
key->dp->id == ECC_SECP256K1 ||
5779-
key->dp->id == ECC_BRAINPOOLP256R1) {
5774+
/* ATECC508A/608A hardware only supports SECP256R1. */
5775+
if (key->dp->id == ECC_SECP256R1) {
57805776
#endif
57815777
key->type = ECC_PRIVATEKEY;
57825778
if (key->slot == ATECC_INVALID_SLOT)
57835779
key->slot = atmel_ecc_alloc(ATMEL_SLOT_ECDHE);
5784-
err = atmel_ecc_create_key(key->slot, ta100_curve_id_for_key(key),
5780+
err = atmel_ecc_create_key(key->slot, microchip_curve_id_for_key(key),
57855781
key->pubkey_raw);
57865782

57875783
/* populate key->pubkey */
@@ -6282,13 +6278,24 @@ int wc_ecc_init_ex(ecc_key* key, void* heap, int devId)
62826278
defined(WOLFSSL_MICROCHIP_TA100)
62836279
key->slot = ATECC_INVALID_SLOT;
62846280
#ifdef WOLFSSL_MICROCHIP_TA100
6285-
/* TA100 needs pubkey initialized to populate after genkey */
6281+
/* TA100 needs pubkey initialized to populate after genkey. With
6282+
* ALT_ECC_SIZE the x/y/z pointers must first be aimed at the inline
6283+
* xyz[] storage; mp_init_multi otherwise dereferences NULL. */
6284+
#ifdef ALT_ECC_SIZE
6285+
key->pubkey.x = (mp_int*)&key->pubkey.xyz[0];
6286+
key->pubkey.y = (mp_int*)&key->pubkey.xyz[1];
6287+
key->pubkey.z = (mp_int*)&key->pubkey.xyz[2];
6288+
alt_fp_init(key->pubkey.x);
6289+
alt_fp_init(key->pubkey.y);
6290+
alt_fp_init(key->pubkey.z);
6291+
#else
62866292
ret = mp_init_multi(key->pubkey.x, key->pubkey.y, key->pubkey.z,
62876293
NULL, NULL, NULL);
62886294
if (ret != MP_OKAY) {
62896295
return MEMORY_E;
62906296
}
62916297
#endif
6298+
#endif
62926299
#else
62936300
#if defined(WOLFSSL_KCAPI_ECC)
62946301
key->handle = NULL;
@@ -6531,14 +6538,14 @@ static int wc_ecc_sign_hash_hw(const byte* in, word32 inlen,
65316538
#if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \
65326539
defined(WOLFSSL_MICROCHIP_TA100)
65336540
#if defined(WOLFSSL_MICROCHIP_TA100)
6534-
if (ta100_curve_id_for_key(key) == ECC_SECP256R1) {
6541+
if (microchip_curve_id_for_key(key) == ECC_SECP256R1) {
65356542
(void)inlen;
65366543
/* Sign: Result is 32-bytes of R then 32-bytes of S */
65376544
err = atmel_ecc_sign(key->slot, in, out);
65386545
}
65396546
else {
65406547
/* Sign: Result is raw R||S */
6541-
err = atmel_ecc_sign_ex(key->slot, ta100_curve_id_for_key(key),
6548+
err = atmel_ecc_sign_ex(key->slot, microchip_curve_id_for_key(key),
65426549
in, inlen, out);
65436550
}
65446551
#else
@@ -9422,7 +9429,7 @@ int wc_ecc_verify_hash_ex(mp_int *r, mp_int *s, const byte* hash,
94229429
#endif /* WOLFSSL_SE050 */
94239430

94249431
#if defined(WOLFSSL_MICROCHIP_TA100)
9425-
if (ta100_curve_id_for_key(key) == ECC_SECP256R1) {
9432+
if (microchip_curve_id_for_key(key) == ECC_SECP256R1) {
94269433
err = atmel_ecc_verify(hash, sigRS, key->pubkey_raw, res);
94279434
if (err != 0) {
94289435
return err;
@@ -9431,7 +9438,7 @@ int wc_ecc_verify_hash_ex(mp_int *r, mp_int *s, const byte* hash,
94319438
}
94329439
else {
94339440
err = atmel_ecc_verify_ex(hash, hashlen, sigRS, key->pubkey_raw,
9434-
keySz * 2, ta100_curve_id_for_key(key), res);
9441+
keySz * 2, microchip_curve_id_for_key(key), res);
94359442
if (err != 0) {
94369443
return err;
94379444
}

wolfcrypt/src/port/atmel/atmel.c

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ static int ateccx08a_cfg_initialized = 0;
147147
.devtype = MICROCHIP_DEV_TYPE,
148148
.atcai2c = {
149149
#ifdef ATCA_ENABLE_DEPRECATED
150-
.slave_address = 1,
150+
.slave_address = ATECC_I2C_ADDR,
151151
#else
152152
.address = ATECC_I2C_ADDR,
153153
#endif
@@ -576,11 +576,12 @@ int atmel_get_rev_info(word32* revision)
576576
WOLFSSL_MSG("Waking device...");
577577
#endif
578578
ret = atcab_wakeup();
579+
if (ret != ATCA_SUCCESS) {
579580
#ifdef WOLFSSL_ATECC_DEBUG
580-
if (ret != 0) {
581581
WOLFSSL_MSG("atcab_wakeup failed");
582-
}
583582
#endif
583+
return atmel_ecc_translate_err(ret);
584+
}
584585
ret = atcab_info((uint8_t*)revision);
585586
ret = atmel_ecc_translate_err(ret);
586587
return ret;
@@ -614,8 +615,11 @@ int atmel_ecc_create_pms(int slotId, const uint8_t* peerKey, uint8_t* pms)
614615

615616
#ifdef WOLFSSL_ATECC_ECDH_ENC
616617
#ifdef WOLFSSL_MICROCHIP_TA100
617-
(void)slotId;
618-
ret = talib_ecdh_compat(atcab_get_device(), MAP_TO_HANDLE(slotIdEnc),
618+
/* TA100 ECDH uses the ephemeral private-key slot; the encryption
619+
* parent slot is allocated above only for parity with the
620+
* non-TA100 atcab_ecdh_enc path and is not consumed here. */
621+
(void)slotIdEnc;
622+
ret = talib_ecdh_compat(atcab_get_device(), MAP_TO_HANDLE(slotId),
619623
peerKey, pms);
620624
#else
621625
/* send the encrypted version of the ECDH command */
@@ -910,24 +914,34 @@ int wc_Microchip_rsa_create_key(struct RsaKey* key, int size, long e)
910914
ret = talib_handle_init_public_key(&uKeyA, WOLFSSL_TA_KEY_TYPE_RSA,
911915
TA_ALG_MODE_RSA_SSA_PSS, 0, 0);
912916
if (ret != ATCA_SUCCESS)
913-
return WC_HW_E;
917+
goto err_free_r;
914918

915919
ta100_fix_property_endian(&uKeyA);
916920

917921
ret = talib_create_element(atcab_get_device(), &uKeyA, &key->uKeyH);
918922
if (ret != ATCA_SUCCESS)
919-
return WC_HW_E;
923+
goto err_free_r;
920924

921925
ret = talib_genkey_base(atcab_get_device(), TA_KEYGEN_MODE_NEWKEY,
922926
(uint32_t)key->rKeyH, key->uKey, &uKey_len);
923927
if (ret != ATCA_SUCCESS)
924-
return WC_HW_E;
928+
goto err_free_ru;
925929

926930
/* Use talib_write_element, not talib_write_pub_key */
927931
ret = talib_write_element(atcab_get_device(), key->uKeyH,
928932
(uint16_t)uKey_len, key->uKey);
933+
if (ret != ATCA_SUCCESS)
934+
goto err_free_ru;
929935

930936
return atmel_ecc_translate_err(ret);
937+
938+
err_free_ru:
939+
(void)talib_delete_handle(atcab_get_device(), (uint32_t)key->uKeyH);
940+
key->uKeyH = 0;
941+
err_free_r:
942+
(void)talib_delete_handle(atcab_get_device(), (uint32_t)key->rKeyH);
943+
key->rKeyH = 0;
944+
return WC_HW_E;
931945
}
932946

933947
int wc_Microchip_rsa_encrypt(const byte* in, word32 inLen, byte* out,
@@ -1243,7 +1257,10 @@ int atmel_init(void)
12431257
}
12441258
#ifdef WOLFSSL_MICROCHIP_TA100
12451259
/* create handles for TA100 */
1246-
atmel_createHandles();
1260+
if (atmel_createHandles() != 0) {
1261+
WOLFSSL_MSG("atmel_createHandles failed");
1262+
return WC_HW_E;
1263+
}
12471264
#endif
12481265
}
12491266

wolfcrypt/src/rsa.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3841,18 +3841,11 @@ static int RsaPrivateDecryptEx(const byte* in, word32 inLen, byte* out,
38413841
}
38423842
return WC_HW_E;
38433843
}
3844-
else if (rsa_type == RSA_PUBLIC_DECRYPT &&
3845-
pad_value == RSA_BLOCK_TYPE_1) {
3846-
if (key->uKeyH != 0) {
3847-
int tmp;
3848-
if (pad_type != WC_RSA_PSS_PAD) {
3849-
return WC_HW_E;
3850-
}
3851-
return wc_Microchip_rsa_verify(in, inLen,
3852-
out, outLen, key, &tmp);
3853-
}
3854-
return WC_HW_E;
3855-
}
3844+
/* Note: RSA_PUBLIC_DECRYPT (verify) is intentionally not intercepted
3845+
* here. wc_Microchip_rsa_verify takes a digest as input, not a raw
3846+
* signature blob; the proper TA100 short-circuit lives in the
3847+
* wc_RsaPSS_CheckPadding / wc_RsaPSS_VerifyCheck path which has the
3848+
* digest available. */
38563849
#elif defined(WOLFSSL_SE050) && !defined(WOLFSSL_SE050_NO_RSA)
38573850
if (rsa_type == RSA_PRIVATE_DECRYPT && pad_value == RSA_BLOCK_TYPE_2) {
38583851
ret = se050_rsa_private_decrypt(in, inLen, out, outLen, key,

wolfcrypt/test/test.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36963,10 +36963,8 @@ static wc_test_ret_t ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerif
3696336963
WC_FREE_VAR(sig, HEAP_HINT);
3696436964
WC_FREE_VAR(digest, HEAP_HINT);
3696536965
#endif
36966-
#if defined(WOLFSSL_MICROCHIP_TA100)
36967-
atmel_ecc_free(ATMEL_SLOT_ECDHE_ALICE);
36968-
atmel_ecc_free(ATMEL_SLOT_ECDHE_BOB);
36969-
#endif
36966+
/* Slot cleanup happens via wc_ecc_free(userA/userB) above; do not call
36967+
* atmel_ecc_free with the slot-type enum constants. */
3697036968
(void)keySize;
3697136969
(void)curve_id;
3697236970
(void)rng;

wolfssl/wolfcrypt/port/atmel/atmel.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@
3939
#include <calib/calib_command.h>
4040
#endif
4141
#if defined(WOLFSSL_MICROCHIP_TA100) && defined(HAVE_AESGCM)
42-
#include <cryptoauthlib/calib/calib_aes_gcm.h>
42+
/* Use the same include style as <calib/calib_command.h> above so the
43+
* single -I added by configure.ac (either .../include or
44+
* .../include/cryptoauthlib) resolves both. */
45+
#include <calib/calib_aes_gcm.h>
4346
#endif
4447
#ifndef ATECC_MAX_SLOT
4548
#define ATECC_MAX_SLOT (0x8) /* Only use 0-7 */

wolfssl/wolfcrypt/settings.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,6 @@
12271227
#endif
12281228
#endif
12291229

1230-
/*#ifdef WOLFSSL_ATECC508A*/
12311230
#if defined(WOLFSSL_ATECC508A) || \
12321231
defined(WOLFSSL_MICROCHIP_TA100)
12331232
/* backwards compatibility */

0 commit comments

Comments
 (0)