Skip to content

Commit 5660c13

Browse files
committed
prevent optimization in algorithms
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com> Add secure_erase function to clear secrets Signed-off-by: Harshil Jani <harshiljani2002@gmail.com> Update the function with good practices Signed-off-by: Harshil Jani <harshiljani2002@gmail.com> Renaming random.h to examples_util.h Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
1 parent 1b21aa5 commit 5660c13

File tree

5 files changed

+42
-17
lines changed

5 files changed

+42
-17
lines changed

Makefile.am

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ noinst_HEADERS += contrib/lax_der_parsing.h
6969
noinst_HEADERS += contrib/lax_der_parsing.c
7070
noinst_HEADERS += contrib/lax_der_privatekey_parsing.h
7171
noinst_HEADERS += contrib/lax_der_privatekey_parsing.c
72-
noinst_HEADERS += examples/random.h
72+
noinst_HEADERS += examples/examples_util.h
7373

7474
PRECOMPUTED_LIB = libsecp256k1_precomputed.la
7575
noinst_LTLIBRARIES = $(PRECOMPUTED_LIB)

examples/ecdh.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
#include <secp256k1.h>
1515
#include <secp256k1_ecdh.h>
1616

17-
#include "random.h"
18-
17+
#include "examples_util.h"
1918

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

122121
return 0;
123122
}

examples/ecdsa.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@
1313

1414
#include <secp256k1.h>
1515

16-
#include "random.h"
17-
18-
16+
#include "examples_util.h"
1917

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

132130
return 0;
133131
}

examples/random.h renamed to examples/examples_util.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,32 @@ static void print_hex(unsigned char* data, size_t size) {
7171
}
7272
printf("\n");
7373
}
74+
75+
#if defined(_MSC_VER)
76+
// For SecureZeroMemory
77+
#include <Windows.h>
78+
#endif
79+
/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */
80+
static SECP256K1_INLINE void secure_erase(void *ptr, size_t len) {
81+
#if defined(_MSC_VER)
82+
/* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */
83+
SecureZeroMemory(ptr, len);
84+
#elif defined(__GNUC__)
85+
/* We use a memory barrier that scares the compiler away from optimizing out the memset.
86+
*
87+
* Quoting Adam Langley <agl@google.com> in commit ad1907fe73334d6c696c8539646c21b11178f20f
88+
* in BoringSSL (ISC License):
89+
* As best as we can tell, this is sufficient to break any optimisations that
90+
* might try to eliminate "superfluous" memsets.
91+
* This method used in memzero_explicit() the Linux kernel, too. Its advantage is that it is
92+
* pretty efficient, because the compiler can still implement the memset() efficently,
93+
* just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by
94+
* Yang et al. (USENIX Security 2017) for more background.
95+
*/
96+
memset(ptr, 0, len);
97+
__asm__ __volatile__("" : : "r"(ptr) : "memory");
98+
#else
99+
void *(*volatile const volatile_memset)(void *, int, size_t) = memset;
100+
volatile_memset(ptr, 0, len);
101+
#endif
102+
}

examples/schnorr.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
#include <secp256k1_extrakeys.h>
1616
#include <secp256k1_schnorrsig.h>
1717

18-
#include "random.h"
18+
#include "examples_util.h"
1919

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

0 commit comments

Comments
 (0)