Skip to content

Commit 9abc3fa

Browse files
committed
otp_crypto: harden PSA security and error handling
- Reset key attributes after psa_import_key in one-shot cipher path to match all other PSA import sites - Use secure_free for all crypto-adjacent buffers (sign/verify data, signature buffers, MAC data, AEAD AAD and combined buffers) to prevent sensitive data from lingering in freed memory - Reject AEAD decryption without a tag early with a clear error instead of letting it fail deep in PSA - Add finalized flag to MAC state so repeated mac_final/mac_update calls after finalization raise a clear error instead of a generic PSA failure - Document that ssl:nif_conf_rng is a no-op on mbedtls 4.x where PSA handles randomness internally Signed-off-by: Peter M <petermm@gmail.com>
1 parent 85ecdc1 commit 9abc3fa

2 files changed

Lines changed: 32 additions & 10 deletions

File tree

src/libAtomVM/otp_crypto.c

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ static term nif_crypto_crypto_one_time(Context *ctx, int argc, term argv[])
745745
psa_set_key_bits(&attributes, key_bits);
746746

747747
psa_status_t status = psa_import_key(&attributes, key_data, key_len, &key_id);
748+
psa_reset_key_attributes(&attributes);
748749
if (UNLIKELY(status != PSA_SUCCESS)) {
749750
char err_msg[48];
750751
snprintf(err_msg, sizeof(err_msg), "key import err %d", (int) status);
@@ -1782,9 +1783,9 @@ static term nif_crypto_sign(Context *ctx, int argc, term argv[])
17821783
cleanup:
17831784
psa_destroy_key(key_id);
17841785

1785-
free(maybe_allocated_data);
1786-
free(sig_raw);
1787-
free(sig_der);
1786+
secure_free(maybe_allocated_data, data_len);
1787+
secure_free(sig_raw, sig_raw_size);
1788+
secure_free(sig_der, sig_der_size);
17881789

17891790
if (UNLIKELY(!success)) {
17901791
RAISE_ERROR(result);
@@ -1951,8 +1952,8 @@ static term nif_crypto_verify(Context *ctx, int argc, term argv[])
19511952
cleanup:
19521953
psa_destroy_key(key_id);
19531954

1954-
free(maybe_allocated_data);
1955-
free(sig_raw);
1955+
secure_free(maybe_allocated_data, data_len);
1956+
secure_free(sig_raw, sig_raw_size);
19561957

19571958
if (UNLIKELY(!success)) {
19581959
RAISE_ERROR(result);
@@ -2112,7 +2113,7 @@ static term nif_crypto_mac(Context *ctx, int argc, term argv[])
21122113
psa_destroy_key(key_id);
21132114
secure_free(mac_out, mac_out_size);
21142115
secure_free(maybe_allocated_key, key_len);
2115-
free(maybe_allocated_data);
2116+
secure_free(maybe_allocated_data, data_len);
21162117

21172118
if (UNLIKELY(!success)) {
21182119
RAISE_ERROR(result);
@@ -2147,6 +2148,7 @@ struct MacState
21472148
psa_algorithm_t psa_algo;
21482149
psa_key_type_t psa_key_type;
21492150
size_t key_bit_size;
2151+
bool finalized;
21502152
#ifndef AVM_NO_SMP
21512153
Mutex *mutex;
21522154
#endif
@@ -2317,13 +2319,17 @@ static term nif_crypto_mac_update(Context *ctx, int argc, term argv[])
23172319
}
23182320
struct MacState *mac_state = (struct MacState *) psa_mac_obj_ptr;
23192321

2322+
if (UNLIKELY(mac_state->finalized)) {
2323+
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "MAC already finalized", ctx));
2324+
}
2325+
23202326
void *maybe_allocated_data = NULL;
23212327
size_t data_len;
23222328
term data_term = argv[1];
23232329
const void *data;
23242330
term iodata_handle_result = handle_iodata(data_term, &data, &data_len, &maybe_allocated_data);
23252331
if (UNLIKELY(iodata_handle_result != OK_ATOM)) {
2326-
free(maybe_allocated_data);
2332+
secure_free(maybe_allocated_data, data_len);
23272333
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "Bad text", ctx));
23282334
}
23292335

@@ -2333,6 +2339,7 @@ static term nif_crypto_mac_update(Context *ctx, int argc, term argv[])
23332339
psa_mac_abort(&mac_state->psa_op);
23342340
psa_destroy_key(mac_state->key_id);
23352341
mac_state->key_id = 0;
2342+
mac_state->finalized = true;
23362343
SMP_MUTEX_UNLOCK(mac_state->mutex);
23372344
secure_free(maybe_allocated_data, data_len);
23382345
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "Unexpected error", ctx));
@@ -2358,6 +2365,10 @@ static term nif_crypto_mac_final(Context *ctx, int argc, term argv[])
23582365
}
23592366
struct MacState *mac_state = (struct MacState *) psa_mac_obj_ptr;
23602367

2368+
if (UNLIKELY(mac_state->finalized)) {
2369+
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "MAC already finalized", ctx));
2370+
}
2371+
23612372
bool success = false;
23622373
term result = ERROR_ATOM;
23632374

@@ -2377,6 +2388,7 @@ static term nif_crypto_mac_final(Context *ctx, int argc, term argv[])
23772388
psa_status_t status = psa_mac_sign_finish(&mac_state->psa_op, mac_buf, mac_size, &mac_len);
23782389
psa_destroy_key(mac_state->key_id);
23792390
mac_state->key_id = 0;
2391+
mac_state->finalized = true;
23802392
SMP_MUTEX_UNLOCK(mac_state->mutex);
23812393
if (UNLIKELY(status != PSA_SUCCESS)) {
23822394
result = make_crypto_error(__FILE__, __LINE__, "Unexpected error", ctx);
@@ -2409,6 +2421,10 @@ static term nif_crypto_mac_finalN(Context *ctx, int argc, term argv[])
24092421
}
24102422
struct MacState *mac_state = (struct MacState *) psa_mac_obj_ptr;
24112423

2424+
if (UNLIKELY(mac_state->finalized)) {
2425+
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "MAC already finalized", ctx));
2426+
}
2427+
24122428
avm_int_t requested_len;
24132429
if (UNLIKELY(!term_is_integer(argv[1]))) {
24142430
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "Bad length", ctx));
@@ -2438,6 +2454,7 @@ static term nif_crypto_mac_finalN(Context *ctx, int argc, term argv[])
24382454
psa_status_t status = psa_mac_sign_finish(&mac_state->psa_op, mac_buf, mac_size, &mac_len);
24392455
psa_destroy_key(mac_state->key_id);
24402456
mac_state->key_id = 0;
2457+
mac_state->finalized = true;
24412458
SMP_MUTEX_UNLOCK(mac_state->mutex);
24422459
if (UNLIKELY(status != PSA_SUCCESS)) {
24432460
result = make_crypto_error(__FILE__, __LINE__, "Unexpected error", ctx);
@@ -3214,6 +3231,10 @@ static term nif_crypto_crypto_one_time_aead(Context *ctx, int argc, term argv[])
32143231
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "EncFlag must be a boolean", ctx));
32153232
}
32163233

3234+
if (UNLIKELY(!encrypting && argc == 6)) {
3235+
RAISE_ERROR(make_crypto_error(__FILE__, __LINE__, "Tag is required for AEAD decryption", ctx));
3236+
}
3237+
32173238
size_t tag_len = aead_params->default_tag_len;
32183239
const void *tag_data = NULL;
32193240
size_t tag_data_len = 0;
@@ -3369,15 +3390,15 @@ static term nif_crypto_crypto_one_time_aead(Context *ctx, int argc, term argv[])
33693390
out_buf_size = pt_size + 1;
33703391
out_buf = malloc(out_buf_size); // +1 to ensure valid malloc even for 0
33713392
if (IS_NULL_PTR(out_buf)) {
3372-
free(combined_buf);
3393+
secure_free(combined_buf, combined_len);
33733394
result = OUT_OF_MEMORY_ATOM;
33743395
goto cleanup;
33753396
}
33763397

33773398
size_t pt_len = 0;
33783399
status = psa_aead_decrypt(key_id, psa_algo, iv_data, iv_len, aad_data, aad_len,
33793400
combined_buf, combined_len, out_buf, pt_size, &pt_len);
3380-
free(combined_buf);
3401+
secure_free(combined_buf, combined_len);
33813402

33823403
switch (status) {
33833404
case PSA_SUCCESS:
@@ -3411,7 +3432,7 @@ static term nif_crypto_crypto_one_time_aead(Context *ctx, int argc, term argv[])
34113432
cleanup:
34123433
psa_destroy_key(key_id);
34133434
secure_free(maybe_allocated_intext, intext_len);
3414-
free(maybe_allocated_aad);
3435+
secure_free(maybe_allocated_aad, aad_len);
34153436
secure_free(out_buf, out_buf_size);
34163437

34173438
if (UNLIKELY(!success)) {

src/libAtomVM/otp_ssl.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@ static term nif_ssl_conf_rng(Context *ctx, int argc, term argv[])
521521

522522
mbedtls_ssl_conf_rng(&conf_obj->config, mbedtls_ctr_drbg_random, &ctr_drbg_obj->context);
523523
#else
524+
// mbedtls 4.x uses PSA for randomness; no explicit RNG configuration needed.
524525
UNUSED(conf_obj);
525526
UNUSED(ctr_drbg_obj);
526527
#endif

0 commit comments

Comments
 (0)