Skip to content

Commit 74486d3

Browse files
author
Emma Stensland
committed
harden forcezero and compiler barriers
1 parent 5bd7348 commit 74486d3

7 files changed

Lines changed: 83 additions & 11 deletions

File tree

tests/api.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27309,6 +27309,7 @@ static int test_wolfSSL_dup_CA_list(void)
2730927309
return res;
2731027310
}
2731127311

27312+
#ifndef WOLFSSL_NO_FORCE_ZERO
2731227313
static int test_ForceZero(void)
2731327314
{
2731427315
EXPECT_DECLS;
@@ -27339,6 +27340,7 @@ static int test_ForceZero(void)
2733927340

2734027341
return EXPECT_RESULT();
2734127342
}
27343+
#endif /* !WOLFSSL_NO_FORCE_ZERO */
2734227344

2734327345
#ifndef NO_BIO
2734427346

@@ -36864,7 +36866,9 @@ TEST_CASE testCases[] = {
3686436866
* wolfcrypt
3686536867
*********************************/
3686636868

36869+
#ifndef WOLFSSL_NO_FORCE_ZERO
3686736870
TEST_DECL(test_ForceZero),
36871+
#endif
3686836872

3686936873
TEST_DECL(test_wolfCrypt_Init),
3687036874

tests/api/test_aes.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10583,7 +10583,9 @@ static int test_CryptoCb_Aes_Cb(int devId, wc_CryptoInfo* info, void* ctx)
1058310583

1058410584
if (aes != NULL && aes->devCtx == cryptoCbAesMockHandle) {
1058510585
/* "Delete" key from simulated SE */
10586+
#ifndef WOLFSSL_NO_FORCE_ZERO
1058610587
ForceZero(&mockSeKey, sizeof(mockSeKey));
10588+
#endif
1058710589
cryptoCbAesFreeCalled++;
1058810590
}
1058910591

@@ -10963,7 +10965,9 @@ static int test_CryptoCb_AesGcm_Offload_Cb(int devId, wc_CryptoInfo* info, void*
1096310965

1096410966
if (aes != NULL && aes->devCtx == cryptoCbAesGcmMockHandle) {
1096510967
/* "Delete" key from simulated SE */
10968+
#ifndef WOLFSSL_NO_FORCE_ZERO
1096610969
ForceZero(&mockSeKeyOffload, sizeof(mockSeKeyOffload));
10970+
#endif
1096710971
cryptoCbAesGcmFreeCalled++;
1096810972
}
1096910973

@@ -11619,8 +11623,11 @@ static int test_Tls13Zero_CryptoCb(int devId, wc_CryptoInfo* info, void* ctx)
1161911623

1162011624
Aes* aes = (Aes*)info->free.obj;
1162111625
if (aes != NULL && aes->devCtx != NULL) {
11626+
#ifndef WOLFSSL_NO_FORCE_ZERO
1162211627
Tls13ZeroKeySlot* slot = (Tls13ZeroKeySlot*)aes->devCtx;
11628+
/* slot is static; zero, don't free. */
1162311629
ForceZero(slot, sizeof(*slot));
11630+
#endif
1162411631
aes->devCtx = NULL;
1162511632
}
1162611633
return 0;

tests/api/test_tls13.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7111,12 +7111,16 @@ int test_tls13_fragmented_session_ticket(void)
71117111
if (EXPECT_SUCCESS() && ssl_c->arrays != NULL) {
71127112
/* Zero before freeing so WOLFSSL_CHECK_MEM_ZERO builds don't abort. */
71137113
if (ssl_c->arrays->preMasterSecret != NULL) {
7114+
#ifndef WOLFSSL_NO_FORCE_ZERO
71147115
ForceZero(ssl_c->arrays->preMasterSecret, ENCRYPT_LEN);
7116+
#endif
71157117
XFREE(ssl_c->arrays->preMasterSecret, ssl_c->heap,
71167118
DYNAMIC_TYPE_SECRET);
71177119
ssl_c->arrays->preMasterSecret = NULL;
71187120
}
7121+
#ifndef WOLFSSL_NO_FORCE_ZERO
71197122
ForceZero(ssl_c->arrays, sizeof(Arrays));
7123+
#endif
71207124
XFREE(ssl_c->arrays, ssl_c->heap, DYNAMIC_TYPE_ARRAYS);
71217125
ssl_c->arrays = NULL;
71227126
}

wolfcrypt/src/misc.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -693,10 +693,12 @@ WC_MISC_STATIC WC_INLINE void xorbuf(void* buf, const void* mask, word32 count)
693693
with zeros. It ensures compiler optimization doesn't skip it. */
694694
WC_MISC_STATIC WC_INLINE void ForceZero(void* mem, size_t len)
695695
{
696-
byte *zb = (byte *)mem;
697-
unsigned long *zl;
696+
/* Volatile pointers prevent dead-store elimination.
697+
* WC_BARRIER() prevents compiler reordering. */
698+
volatile byte *zb = (volatile byte *)mem;
699+
volatile unsigned long *zl;
698700

699-
XFENCE();
701+
WC_BARRIER();
700702

701703
while ((wc_ptr_t)zb & (wc_ptr_t)(sizeof(unsigned long) - 1U)) {
702704
if (len == 0)
@@ -705,21 +707,21 @@ WC_MISC_STATIC WC_INLINE void ForceZero(void* mem, size_t len)
705707
--len;
706708
}
707709

708-
zl = (unsigned long *)zb;
710+
zl = (volatile unsigned long *)zb;
709711

710712
while (len >= sizeof(unsigned long)) {
711713
*zl++ = 0;
712714
len -= sizeof(unsigned long);
713715
}
714716

715-
zb = (byte *)zl;
717+
zb = (volatile byte *)zl;
716718

717719
while (len) {
718720
*zb++ = 0;
719721
--len;
720722
}
721723

722-
XFENCE();
724+
WC_BARRIER();
723725
}
724726
#endif
725727

wolfcrypt/test/test.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,9 @@ typedef struct testVector {
772772
#ifndef WC_TEST_EXPORT_SUBTESTS
773773

774774
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t macro_test(void);
775+
#ifndef WOLFSSL_NO_FORCE_ZERO
776+
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t forcezero_test(void);
777+
#endif
775778
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t error_test(void);
776779
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t base64_test(void);
777780
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t base16_test(void);
@@ -2409,6 +2412,13 @@ options: [-s max_relative_stack_bytes] [-m max_relative_heap_memory_bytes]\n\
24092412
else
24102413
TEST_PASS("macro test passed!\n");
24112414

2415+
#ifndef WOLFSSL_NO_FORCE_ZERO
2416+
if ( (ret = forcezero_test()) != 0)
2417+
TEST_FAIL("forcezero test failed!\n", ret);
2418+
else
2419+
TEST_PASS("forcezero test passed!\n");
2420+
#endif
2421+
24122422
if ( (ret = error_test()) != 0)
24132423
TEST_FAIL("error test failed!\n", ret);
24142424
else
@@ -4097,6 +4107,45 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t macro_test(void)
40974107
return ret;
40984108
}
40994109

4110+
#ifndef WOLFSSL_NO_FORCE_ZERO
4111+
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t forcezero_test(void)
4112+
{
4113+
/* Test unaligned offsets and lengths. Oversized buffer prevents OOB writes. */
4114+
byte buf[64];
4115+
static const size_t offsets[] = { 0, 1, 2, 3, 7 };
4116+
static const size_t lens[] = { 0, 1, 3, 7, 8, 9, 16, 31 };
4117+
size_t oi, li;
4118+
ForceZero(NULL, 0);
4119+
4120+
for (oi = 0; oi < XELEM_CNT(offsets); oi++) {
4121+
for (li = 0; li < XELEM_CNT(lens); li++) {
4122+
size_t off = offsets[oi];
4123+
size_t len = lens[li];
4124+
size_t i;
4125+
4126+
XMEMSET(buf, 0xA5, sizeof(buf));
4127+
ForceZero(buf + off, len);
4128+
4129+
for (i = 0; i < len; i++) {
4130+
if (buf[off + i] != 0x00)
4131+
return WC_TEST_RET_ENC_NC;
4132+
}
4133+
/* bytes outside [off, off+len) must be untouched */
4134+
for (i = 0; i < off; i++) {
4135+
if (buf[i] != (byte)0xA5)
4136+
return WC_TEST_RET_ENC_NC;
4137+
}
4138+
for (i = off + len; i < sizeof(buf); i++) {
4139+
if (buf[i] != (byte)0xA5)
4140+
return WC_TEST_RET_ENC_NC;
4141+
}
4142+
}
4143+
}
4144+
4145+
return 0;
4146+
}
4147+
#endif
4148+
41004149
WOLFSSL_TEST_SUBROUTINE wc_test_ret_t error_test(void)
41014150
{
41024151
const char* errStr;

wolfcrypt/test/test.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ wc_static_assert(-(long)MIN_CODE_E < 0x7ffL);
115115
#endif
116116

117117
extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t macro_test(void);
118+
#ifndef WOLFSSL_NO_FORCE_ZERO
119+
extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t forcezero_test(void);
120+
#endif
118121
extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t error_test(void);
119122
extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t base64_test(void);
120123
extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t base16_test(void);

wolfssl/wolfcrypt/wc_port.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,14 +1919,17 @@ WOLFSSL_ABI WOLFSSL_API int wolfCrypt_Cleanup(void);
19191919
#endif
19201920

19211921
#ifdef WOLF_C99
1922-
/* use alternate keyword for compatibility with -std=c99 */
1923-
#define XASM_VOLATILE(a) __asm__ volatile(a)
1922+
/* -std=c99 compat; "memory" clobber creates a compiler barrier. */
1923+
#define XASM_VOLATILE(a) __asm__ volatile(a ::: "memory") /* NOLINT(bugprone-macro-parentheses) */
19241924
#elif defined(__IAR_SYSTEMS_ICC__)
1925-
#define XASM_VOLATILE(a) asm volatile(a)
1925+
#define XASM_VOLATILE(a) asm volatile(a) /* NOLINT(bugprone-macro-parentheses) */
19261926
#elif defined(__KEIL__)
1927-
#define XASM_VOLATILE(a) __asm volatile(a)
1927+
/* No "memory" clobber: ARM Compiler 5 inline asm rejects GCC clobber
1928+
* syntax. */
1929+
#define XASM_VOLATILE(a) __asm volatile(a) /* NOLINT(bugprone-macro-parentheses) */
19281930
#else
1929-
#define XASM_VOLATILE(a) __asm__ __volatile__(a)
1931+
/* "memory" clobber strengthens XFENCE() into a full compiler barrier. */
1932+
#define XASM_VOLATILE(a) __asm__ __volatile__(a ::: "memory") /* NOLINT(bugprone-macro-parentheses) */
19301933
#endif
19311934

19321935
#ifndef WOLFSSL_NO_FENCE

0 commit comments

Comments
 (0)