Abstraction for architecture dependent secure memset.

In ecc_dh.c it is widely used to zero out the memory used for holding
sensitive information, such as private keys. This memory is cleared by
a memset, but as this is done right at function return, this memset
will most likely be optimized out by the compiler, as the memory will
no longer be used, hence no need to set it. To prevent the memset from
being optimized out, it is necessary to place a memory barrier at the
memset, but such a barrier is compiler depended.

To solve the above, a _set_secure function has been introduced. This
defaults to an inlined function calling memset and, in cases where the
compiler defines __GNUC__, adds a memory barrier which ensures that
the memset is not optimized out. For compilers, not defining the
__GNUC__, it may be necessary to define TINYCRYPT_ARCH_HAS_SET_SECURE
which then declares the _set_secure function extern, thus meaning that
it will require target implementation, which in turn allows for
architecture/compiler dependent implementation.

This also fixes the memset's in uECC_make_key and uECC_make_key_with_d
which actually was missing the memory barrier.

Fixes #32

Signed-off-by: Danny Oerndrup <[email protected]>

Author: daor-oti

lib/include/tinycrypt/utils.h

@@ -41,6 +41,7 @@
 
 #include <stdint.h>
 #include <stddef.h>
+#include <string.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -69,6 +70,31 @@ unsigned int _copy(uint8_t *to, unsigned int to_len,
  */
 void _set(void *to, uint8_t val, unsigned int len);
 
+/**
+ * @brief Set the value 'val' into the buffer 'to', 'len' times, in a way
+ *         which does not risk getting optimized out by the compiler
+ *        In cases where the compiler does not set __GNUC__ and where the
+ *         optimization level removes the memset, it may be necessary to
+ *         implement a _set_secure function and define the
+ *         TINYCRYPT_ARCH_HAS_SET_SECURE, which then can ensure that the
+ *         memset does not get optimized out.
+ *
+ * @param to OUT -- destination buffer
+ * @param val IN -- value to be set in 'to'
+ * @param len IN -- number of times the value will be copied
+ */
+#ifdef TINYCRYPT_ARCH_HAS_SET_SECURE
+extern void _set_secure(void *to, uint8_t val, unsigned int len);
+#else /* ! TINYCRYPT_ARCH_HAS_SET_SECURE */
+static inline void _set_secure(void *to, uint8_t val, unsigned int len)
+{
+  (void) memset(to, val, len);
+#ifdef __GNUC__
+  __asm__ __volatile__("" :: "g"(to) : "memory");
+#endif /* __GNUC__ */
+}
+#endif /* TINYCRYPT_ARCH_HAS_SET_SECURE */
+
 /*
  * @brief AES specific doubling function, which utilizes
  * the finite field used by AES.

lib/source/ecc_dh.c

@@ -57,6 +57,7 @@
 #include <tinycrypt/constants.h>
 #include <tinycrypt/ecc.h>
 #include <tinycrypt/ecc_dh.h>
+#include <tinycrypt/utils.h>
 #include <string.h>
 
 #if default_RNG_defined
@@ -92,7 +93,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key,
 				       _public + curve->num_words);
 
 		/* erasing temporary buffer used to store secret: */
-		memset(_private, 0, NUM_ECC_BYTES);
+		_set_secure(_private, 0, NUM_ECC_BYTES);
 
 		return 1;
 	}
@@ -133,7 +134,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve)
 					       _public + curve->num_words);
 
 			/* erasing temporary buffer that stored secret: */
-			memset(_private, 0, NUM_ECC_BYTES);
+			_set_secure(_private, 0, NUM_ECC_BYTES);
 
       			return 1;
     		}
@@ -189,12 +190,9 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key,
 
 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");
+	_set_secure(p2, 0, sizeof(p2));
+	_set_secure(tmp, 0, sizeof(tmp));
+	_set_secure(_private, 0, sizeof(_private));
 
 	return r;
 }