Skip to content

Commit 1844d19

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

7 files changed

Lines changed: 86 additions & 18 deletions

File tree

tests/api.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27316,15 +27316,15 @@ static int test_ForceZero(void)
2731627316
unsigned int i, j, len;
2731727317

2731827318
/* Test case with 0 length */
27319-
ForceZero(data, 0);
27319+
wc_ForceZero(data, 0);
2732027320

2732127321
/* Test ForceZero */
2732227322
for (i = 0; i < sizeof(data); i++) {
2732327323
for (len = 1; len < sizeof(data) - i; len++) {
2732427324
for (j = 0; j < sizeof(data); j++)
2732527325
data[j] = ((unsigned char)j + 1);
2732627326

27327-
ForceZero(data + i, len);
27327+
wc_ForceZero(data + i, len);
2732827328

2732927329
for (j = 0; j < sizeof(data); j++) {
2733027330
if (j < i || j >= i + len) {

tests/api/test_aes.c

Lines changed: 10 additions & 3 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-
ForceZero(&mockSeKey, sizeof(mockSeKey));
10586+
#ifndef WOLFSSL_NO_FORCE_ZERO
10587+
wc_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 */
10966-
ForceZero(&mockSeKeyOffload, sizeof(mockSeKeyOffload));
10968+
#ifndef WOLFSSL_NO_FORCE_ZERO
10969+
wc_ForceZero(&mockSeKeyOffload, sizeof(mockSeKeyOffload));
10970+
#endif
1096710971
cryptoCbAesGcmFreeCalled++;
1096810972
}
1096910973

@@ -11620,7 +11624,10 @@ static int test_Tls13Zero_CryptoCb(int devId, wc_CryptoInfo* info, void* ctx)
1162011624
Aes* aes = (Aes*)info->free.obj;
1162111625
if (aes != NULL && aes->devCtx != NULL) {
1162211626
Tls13ZeroKeySlot* slot = (Tls13ZeroKeySlot*)aes->devCtx;
11623-
ForceZero(slot, sizeof(*slot));
11627+
if (slot != NULL) {
11628+
wc_ForceZero(slot, sizeof(*slot));
11629+
XFREE(slot, NULL, DYNAMIC_TYPE_TMP_BUFFER);
11630+
}
1162411631
aes->devCtx = NULL;
1162511632
}
1162611633
return 0;

tests/api/test_tls13.c

Lines changed: 6 additions & 2 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-
ForceZero(ssl_c->arrays->preMasterSecret, ENCRYPT_LEN);
7114+
#ifndef WOLFSSL_NO_FORCE_ZERO
7115+
wc_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
}
7119-
ForceZero(ssl_c->arrays, sizeof(Arrays));
7121+
#ifndef WOLFSSL_NO_FORCE_ZERO
7122+
wc_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+
wc_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+
wc_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)