bartoszek/zephyr
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tinycrypt: ecc_dh: Properly clear out temporary secret buffers

Browse Source 
Zeroing out 2*NUM_ECC_WORDS bytes starting from the `p2` pointer would
not only write 16 bytes to an 8-byte array allocated on the stack, but
also not clear out important arrays such as `_private` and `tmp`.
Moreover, no memory was cleared out before returning from the function,
and there are two exit points.

Properly memset() all private data and use an empty assembly block
referencing the memory region to avoid the memset() calls to be elided
by the compiler.

Ideally, in the future, all stack-allocated variables that contains
sensitive information should be marked with __attribute__((cleanup)), a
GCC extension that calls a function when the variable exits the scope.
This will not only reduce code size, but for other functions with
multiple exit points, also ensure that sensitive data is always
cleared.

Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>
This commit is contained in:
Leandro Pereira 2017-08-18 10:41:31 -07:00 committed by Anas Nashif
parent cf17f4870d
commit eb0dccdb94
1 changed files with 15 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
ext/lib/crypto/tinycrypt/source/ecc_dh.c
View File

@ -154,6 +154,7 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key,
uECC_word_t carry;
wordcount_t num_words = curve->num_words;
wordcount_t num_bytes = curve->num_bytes;
int r;
/* Converting buffers to correct bit order: */
uECC_vli_bytesToNative(_private,
@ -174,7 +175,8 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key,
* improve protection against side-channel attacks. */
if (g_rng_function) {
if (!uECC_generate_random_int(p2[carry], curve->p, num_words)) {
return 0;
r = 0;
goto clear_and_out;
}
initial_Z = p2[carry];
}
@ -182,9 +184,17 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key,
EccPoint_mult(_public, _public, p2[!carry], initial_Z, curve->num_n_bits + 1,
curve);
/* erasing temporary buffer used to store secret: */
memset (p2, 0, 2*NUM_ECC_WORDS);
uECC_vli_nativeToBytes(secret, num_bytes, _public);
return !EccPoint_isZero(_public, curve);
r = !EccPoint_isZero(_public, curve);
clear_and_out:
/* erasing temporary buffer used to store secret: */
memset(p2, 0, sizeof(p2));
__asm__ __volatile__("" :: "g"(p2) : "memory");
memset(tmp, 0, sizeof(tmp));
__asm__ __volatile__("" :: "g"(tmp) : "memory");
memset(_private, 0, sizeof(_private));
__asm__ __volatile__("" :: "g"(_private) : "memory");
return r;
}