Skip to content

Commit

Permalink
prevent optimization in algorithms
Browse files Browse the repository at this point in the history
Signed-off-by: Harshil Jani <[email protected]>

Add secure_erase function to clear secrets

Signed-off-by: Harshil Jani <[email protected]>

Update the function with good practices

Signed-off-by: Harshil Jani <[email protected]>

Renaming random.h to examples_util.h

Signed-off-by: Harshil Jani <[email protected]>
  • Loading branch information
Harshil-Jani committed Mar 2, 2023
1 parent 1b21aa5 commit 5660c13
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 17 deletions.
2 changes: 1 addition & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ noinst_HEADERS += contrib/lax_der_parsing.h
noinst_HEADERS += contrib/lax_der_parsing.c
noinst_HEADERS += contrib/lax_der_privatekey_parsing.h
noinst_HEADERS += contrib/lax_der_privatekey_parsing.c
noinst_HEADERS += examples/random.h
noinst_HEADERS += examples/examples_util.h

PRECOMPUTED_LIB = libsecp256k1_precomputed.la
noinst_LTLIBRARIES = $(PRECOMPUTED_LIB)
Expand Down
13 changes: 6 additions & 7 deletions examples/ecdh.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
#include <secp256k1.h>
#include <secp256k1_ecdh.h>

#include "random.h"

#include "examples_util.h"

int main(void) {
unsigned char seckey1[32];
Expand Down Expand Up @@ -112,12 +111,12 @@ int main(void) {
* example through "out of bounds" array access (see Heartbleed), Or the OS
* swapping them to disk. Hence, we overwrite the secret key buffer with zeros.
*
* TODO: Prevent these writes from being optimized out, as any good compiler
* Here we are preventing these writes from being optimized out, as any good compiler
* will remove any writes that aren't used. */
memset(seckey1, 0, sizeof(seckey1));
memset(seckey2, 0, sizeof(seckey2));
memset(shared_secret1, 0, sizeof(shared_secret1));
memset(shared_secret2, 0, sizeof(shared_secret2));
secure_erase(seckey1, sizeof(seckey1));
secure_erase(seckey2, sizeof(seckey2));
secure_erase(shared_secret1, sizeof(shared_secret1));
secure_erase(shared_secret2, sizeof(shared_secret2));

return 0;
}
8 changes: 3 additions & 5 deletions examples/ecdsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@

#include <secp256k1.h>

#include "random.h"


#include "examples_util.h"

int main(void) {
/* Instead of signing the message directly, we must sign a 32-byte hash.
Expand Down Expand Up @@ -125,9 +123,9 @@ int main(void) {
* example through "out of bounds" array access (see Heartbleed), Or the OS
* swapping them to disk. Hence, we overwrite the secret key buffer with zeros.
*
* TODO: Prevent these writes from being optimized out, as any good compiler
* Here we are preventing these writes from being optimized out, as any good compiler
* will remove any writes that aren't used. */
memset(seckey, 0, sizeof(seckey));
secure_erase(seckey, sizeof(seckey));

return 0;
}
29 changes: 29 additions & 0 deletions examples/random.h → examples/examples_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,32 @@ static void print_hex(unsigned char* data, size_t size) {
}
printf("\n");
}

#if defined(_MSC_VER)
// For SecureZeroMemory
#include <Windows.h>
#endif
/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */
static SECP256K1_INLINE void secure_erase(void *ptr, size_t len) {
#if defined(_MSC_VER)
/* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */
SecureZeroMemory(ptr, len);
#elif defined(__GNUC__)
/* We use a memory barrier that scares the compiler away from optimizing out the memset.
*
* Quoting Adam Langley <[email protected]> in commit ad1907fe73334d6c696c8539646c21b11178f20f
* in BoringSSL (ISC License):
* As best as we can tell, this is sufficient to break any optimisations that
* might try to eliminate "superfluous" memsets.
* This method used in memzero_explicit() the Linux kernel, too. Its advantage is that it is
* pretty efficient, because the compiler can still implement the memset() efficently,
* just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by
* Yang et al. (USENIX Security 2017) for more background.
*/
memset(ptr, 0, len);
__asm__ __volatile__("" : : "r"(ptr) : "memory");
#else
void *(*volatile const volatile_memset)(void *, int, size_t) = memset;
volatile_memset(ptr, 0, len);
#endif
}
7 changes: 3 additions & 4 deletions examples/schnorr.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <secp256k1_extrakeys.h>
#include <secp256k1_schnorrsig.h>

#include "random.h"
#include "examples_util.h"

int main(void) {
unsigned char msg[12] = "Hello World!";
Expand Down Expand Up @@ -140,9 +140,8 @@ int main(void) {
* example through "out of bounds" array access (see Heartbleed), Or the OS
* swapping them to disk. Hence, we overwrite the secret key buffer with zeros.
*
* TODO: Prevent these writes from being optimized out, as any good compiler
* Here we are preventing these writes from being optimized out, as any good compiler
* will remove any writes that aren't used. */
memset(seckey, 0, sizeof(seckey));

secure_erase(seckey, sizeof(seckey));
return 0;
}

0 comments on commit 5660c13

Please sign in to comment.