Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent dead-store elimination when clearing secrets in examples #1212

Merged
merged 1 commit into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}