From aa87b3596489a63e36285d7f917673e7e3e6970a Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Fri, 29 Aug 2025 08:43:15 +1000 Subject: [PATCH] Mark variables as volatile Ensures compiler optimizers don't stop code from being constant time. --- src/internal.c | 50 +++++++++++++++++++++-------------------- src/tls.c | 8 +++++-- wolfcrypt/src/aes.c | 4 ++-- wolfcrypt/src/dh.c | 2 +- wolfcrypt/src/ecc.c | 8 +++---- wolfcrypt/src/misc.c | 4 ++-- wolfcrypt/src/rsa.c | 18 +++++++++------ wolfcrypt/src/sp_int.c | 51 ++++++++++++++++++++++-------------------- 8 files changed, 79 insertions(+), 66 deletions(-) diff --git a/src/internal.c b/src/internal.c index 947ac2491ad..7c55d9fe187 100644 --- a/src/internal.c +++ b/src/internal.c @@ -20994,7 +20994,7 @@ static byte MaskPadding(const byte* data, int sz, int macSz) checkSz = TLS_MAX_PAD_SZ; for (i = 0; i < checkSz; i++) { - byte mask = ctMaskLTE(i, paddingSz); + volatile byte mask = ctMaskLTE(i, paddingSz); good |= mask & (data[sz - 1 - i] ^ paddingSz); } @@ -21014,16 +21014,21 @@ static byte MaskPadding(const byte* data, int sz, int macSz) static byte MaskMac(const byte* data, int sz, int macSz, byte* expMac) { int i, j; - unsigned char mac[WC_MAX_DIGEST_SIZE]; - int scanStart = sz - 1 - TLS_MAX_PAD_SZ - macSz; - int macEnd = sz - 1 - data[sz - 1]; - int macStart = macEnd - macSz; int r = 0; - unsigned char started, notEnded; + unsigned char mac[WC_MAX_DIGEST_SIZE]; + volatile int scanStart = sz - 1 - TLS_MAX_PAD_SZ - macSz; + volatile int macEnd = sz - 1 - data[sz - 1]; + volatile int macStart = macEnd - macSz; + volatile int maskScanStart; + volatile int maskMacStart; + volatile unsigned char started; + volatile unsigned char notEnded; unsigned char good = 0; - scanStart &= ctMaskIntGTE(scanStart, 0); - macStart &= ctMaskIntGTE(macStart, 0); + maskScanStart = ctMaskIntGTE(scanStart, 0); + maskMacStart = ctMaskIntGTE(macStart, 0); + scanStart &= maskScanStart; + macStart &= maskMacStart; /* Div on Intel has different speeds depending on value. * Use a bitwise AND or mod a specific value (converted to mul). */ @@ -21993,22 +21998,19 @@ static int DoDecrypt(WOLFSSL *ssl) /* Last of padding bytes - indicates length. */ ssl->keys.padSz = in->buffer[off]; - /* Constant time checking of padding - don't leak - * the length of the data. - */ + /* Constant time checking of padding - don't leak the length of + * the data. */ /* Compare max pad bytes or at most data + pad. */ for (i = 1; i < MAX_PAD_SIZE && off >= i; i++) { - /* Mask on indicates this is expected to be a - * padding byte. - */ - padding &= ctMaskLTE((int)i, - (int)ssl->keys.padSz); - /* When this is a padding byte and not equal - * to length then mask is set. - */ - invalid |= padding & - ctMaskNotEq(in->buffer[off - i], - (int)ssl->keys.padSz); + /* Mask on indicates this is expected to be a padding byte. + */ + volatile byte maskPadByte = ctMaskLTE((int)i, + (int)ssl->keys.padSz); + padding &= maskPadByte; + /* When this is a padding byte and not equal to length then + * mask is set. */ + invalid |= padding & ctMaskNotEq(in->buffer[off - i], + (int)ssl->keys.padSz); } /* If mask is set then there was an error. */ if (invalid) { @@ -41655,7 +41657,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], case rsa_kea: { RsaKey* key = (RsaKey*)ssl->hsKey; - int lenErrMask; + volatile int lenErrMask; ret = RsaDec(ssl, input + args->idx, @@ -41869,7 +41871,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], case rsa_kea: { byte *tmpRsa; - byte mask; + volatile byte mask; /* Add the signature length to idx */ args->idx += args->length; diff --git a/src/tls.c b/src/tls.c index 7ce76aacc56..23a5752ef56 100644 --- a/src/tls.c +++ b/src/tls.c @@ -946,7 +946,10 @@ static int Hmac_UpdateFinal_CT(Hmac* hmac, byte* digest, const byte* in, unsigned int k; int blockBits, blockMask; int lastBlockLen, extraLen, eocIndex; - int blocks, safeBlocks, lenBlock, eocBlock; + int blocks; + int safeBlocks; + int lenBlock; + int eocBlock; word32 maxLen; int blockSz, padSz; int ret; @@ -1056,7 +1059,8 @@ static int Hmac_UpdateFinal_CT(Hmac* hmac, byte* digest, const byte* in, for (j = 0; j < blockSz; j++) { unsigned char atEoc = ctMaskEq(j, eocIndex) & isEocBlock; - unsigned char pastEoc = ctMaskGT(j, eocIndex) & isEocBlock; + volatile unsigned char maskPastEoc = ctMaskGT(j, eocIndex); + volatile unsigned char pastEoc = maskPastEoc & isEocBlock; unsigned char b = 0; if (k < headerSz) diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index e9be2c2c212..9d44dfd639b 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -6803,7 +6803,7 @@ static WC_INLINE void RIGHTSHIFTX(byte* x) { int i; int carryIn = 0; - byte borrow = (byte)((0x00U - (x[15] & 0x01U)) & 0xE1U); + volatile byte borrow = (byte)((0x00U - (x[15] & 0x01U)) & 0xE1U); for (i = 0; i < WC_AES_BLOCK_SIZE; i++) { int carryOut = (x[i] & 0x01) << 7; @@ -9346,7 +9346,7 @@ int WARN_UNUSED_RESULT AES_GCM_decrypt_C( ALIGN16 byte scratch[WC_AES_BLOCK_SIZE]; ALIGN16 byte Tprime[WC_AES_BLOCK_SIZE]; ALIGN16 byte EKY0[WC_AES_BLOCK_SIZE]; - sword32 res; + volatile sword32 res; if (ivSz == GCM_NONCE_MID_SZ) { /* Counter is IV with bottom 4 bytes set to: 0x00,0x00,0x00,0x01. */ diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index e41dd73f47a..afa5c8275a9 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -2114,7 +2114,7 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz, } if ((ret == 0) && ct) { - word16 mask = 0xff; + volatile word16 mask = 0xff; sword16 o = (sword16)(*agreeSz - 1); *agreeSz = (word32)(i + 1); diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index b90883029ef..b2479204cf8 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3166,7 +3166,7 @@ static int ecc_mulmod(const mp_int* k, ecc_point* P, ecc_point* Q, ecc_point** R, mp_int* a, mp_int* modulus, mp_digit mp, WC_RNG* rng) { int err = MP_OKAY; - int bytes = (mp_count_bits(modulus) + 7) / 8; + int bytes = (mp_count_bits(modulus) + 7) >> 3; int i; int j = 1; int cnt = DIGIT_BIT; @@ -3404,7 +3404,7 @@ static int ecc_mulmod(const mp_int* k, ecc_point* P, ecc_point* Q, ecc_point** R, mp_int* a, mp_int* modulus, mp_digit mp, WC_RNG* rng) { int err = MP_OKAY; - int bytes = (mp_count_bits(modulus) + 7) / 8; + int bytes = (mp_count_bits(modulus) + 7) >> 3; int i; int j = 1; int cnt; @@ -4452,7 +4452,7 @@ int wc_ecc_get_curve_id_from_params(int fieldSize, Gx == NULL || Gy == NULL) return BAD_FUNC_ARG; - curveSz = (fieldSize + 1) / 8; /* round up */ + curveSz = (fieldSize + 1) >> 3; /* round up */ for (idx = 0; ecc_sets[idx].size != 0; idx++) { if (curveSz == ecc_sets[idx].size) { @@ -11929,7 +11929,7 @@ int wc_ecc_sig_size(const ecc_key* key) keySz = key->dp->size; orderBits = wc_ecc_get_curve_order_bit_count(key->dp); if (orderBits > keySz * 8) { - keySz = (orderBits + 7) / 8; + keySz = (orderBits + 7) >> 3; } /* maximum possible signature header size is 7 bytes */ maxSigSz = (keySz * 2) + SIG_HEADER_SZ; diff --git a/wolfcrypt/src/misc.c b/wolfcrypt/src/misc.c index bf8373c7d10..655c1167267 100644 --- a/wolfcrypt/src/misc.c +++ b/wolfcrypt/src/misc.c @@ -774,7 +774,7 @@ WC_MISC_STATIC WC_INLINE void ctMaskCopy(byte mask, byte* dst, byte* src, { #if !defined(WOLFSSL_NO_CT_OPS) && !defined(WOLFSSL_NO_CT_MAX_MIN) && \ defined(WORD64_AVAILABLE) - word32 gte_mask = (word32)ctMaskWord32GTE(a, b); + volatile word32 gte_mask = (word32)ctMaskWord32GTE(a, b); return (a & ~gte_mask) | (b & gte_mask); #else /* WOLFSSL_NO_CT_OPS */ return a > b ? b : a; @@ -791,7 +791,7 @@ WC_MISC_STATIC WC_INLINE void ctMaskCopy(byte mask, byte* dst, byte* src, { #if !defined(WOLFSSL_NO_CT_OPS) && !defined(WOLFSSL_NO_CT_MAX_MIN) && \ defined(WORD64_AVAILABLE) - word32 gte_mask = (word32)ctMaskWord32GTE(a, b); + volatile word32 gte_mask = (word32)ctMaskWord32GTE(a, b); return (a & gte_mask) | (b & ~gte_mask); #else /* WOLFSSL_NO_CT_OPS */ return a > b ? a : b; diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index 7014e6dede8..b4a644dbbb6 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -1562,11 +1562,11 @@ static int RsaUnPad_OAEP(byte *pkcsBlock, unsigned int pkcsBlockLen, byte* optLabel, word32 labelLen, void* heap) { word32 hLen; - int ret; + volatile int ret; byte h[WC_MAX_DIGEST_SIZE]; /* max digest size */ word32 idx; word32 i; - word32 inc; + volatile word32 inc; #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) byte* tmp = NULL; @@ -1851,9 +1851,11 @@ static int RsaUnPad(const byte *pkcsBlock, unsigned int pkcsBlockLen, } #ifndef WOLFSSL_RSA_VERIFY_ONLY else { - unsigned int j; - word16 pastSep = 0; - byte invalid = 0; + unsigned int j; + volatile word16 pastSep = 0; + volatile byte invalid = 0; + volatile byte minPad; + volatile int invalidMask; i = 0; /* Decrypted with private key - unpad must be constant time. */ @@ -1865,7 +1867,8 @@ static int RsaUnPad(const byte *pkcsBlock, unsigned int pkcsBlockLen, } /* Minimum of 11 bytes of pre-message data - including leading 0x00. */ - invalid |= ctMaskLT(i, RSA_MIN_PAD_SZ); + minPad = ctMaskLT(i, RSA_MIN_PAD_SZ); + invalid |= minPad; /* Must have seen separator. */ invalid |= (byte)~pastSep; /* First byte must be 0x00. */ @@ -1874,7 +1877,8 @@ static int RsaUnPad(const byte *pkcsBlock, unsigned int pkcsBlockLen, invalid |= ctMaskNotEq(pkcsBlock[1], padValue); *output = (byte *)(pkcsBlock + i); - ret = ((int)-1 + (int)(invalid >> 7)) & ((int)pkcsBlockLen - i); + invalidMask = (int)-1 + (int)(invalid >> 7); + ret = invalidMask & ((int)pkcsBlockLen - i); } #endif diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index 5a8222a5226..c2799e24ca4 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -5495,7 +5495,7 @@ int sp_exch(sp_int* a, sp_int* b) int sp_cond_swap_ct_ex(sp_int* a, sp_int* b, int cnt, int swap, sp_int* t) { unsigned int i; - sp_int_digit mask = (sp_int_digit)0 - (sp_int_digit)swap; + volatile sp_int_digit mask = (sp_int_digit)0 - (sp_int_digit)swap; /* XOR other fields in sp_int into temp - mask set when swapping. */ t->used = (a->used ^ b->used) & (sp_size_t)mask; @@ -5765,7 +5765,7 @@ static int _sp_cmp_ct(const sp_int* a, const sp_int* b, unsigned int n) { int ret = MP_EQ; int i; - int mask = -1; + volatile int mask = -1; for (i = n - 1; i >= 0; i--) { sp_int_digit ad = a->dp[i] & ((sp_int_digit)0 - (i < (int)a->used)); @@ -7298,7 +7298,8 @@ static void _sp_div_2(const sp_int* a, sp_int* r) /* Shift down each word by 1 and include bottom bit of next at top. */ for (i = 0; i < (int)a->used - 1; i++) { - r->dp[i] = (a->dp[i] >> 1) | (a->dp[i+1] << (SP_WORD_SIZE - 1)); + r->dp[i] = a->dp[i] >> 1; + r->dp[i] |= a->dp[i+1] << (SP_WORD_SIZE - 1); } /* Last word only needs to be shifted down. */ r->dp[i] = a->dp[i] >> 1; @@ -7378,7 +7379,7 @@ int sp_div_2_mod_ct(const sp_int* a, const sp_int* m, sp_int* r) sp_int_digit t; #endif /* Mask to apply to modulus. */ - sp_int_digit mask = (sp_int_digit)0 - (a->dp[0] & 1); + volatile sp_int_digit mask = (sp_int_digit)0 - (a->dp[0] & 1); sp_size_t i; #if 0 @@ -7389,7 +7390,7 @@ int sp_div_2_mod_ct(const sp_int* a, const sp_int* m, sp_int* r) /* Add a to m, if a is odd, into r in constant time. */ for (i = 0; i < m->used; i++) { /* Mask to apply to a - set when used value at index. */ - sp_int_digit mask_a = (sp_int_digit)0 - (i < a->used); + volatile sp_int_digit mask_a = (sp_int_digit)0 - (i < a->used); #ifndef SQR_MUL_ASM /* Conditionally add modulus. */ @@ -8013,7 +8014,7 @@ static void sp_clamp_ct(sp_int* a) { int i; sp_size_t used = a->used; - sp_size_t mask = (sp_size_t)-1; + volatile sp_size_t mask = (sp_size_t)-1; for (i = (int)a->used - 1; i >= 0; i--) { #if ((SP_WORD_SIZE == 64) && \ @@ -8062,9 +8063,9 @@ int sp_addmod_ct(const sp_int* a, const sp_int* b, const sp_int* m, sp_int* r) sp_int_digit sh; sp_int_digit t; #endif - sp_int_digit mask; - sp_int_digit mask_a = (sp_int_digit)-1; - sp_int_digit mask_b = (sp_int_digit)-1; + volatile sp_int_digit mask; + volatile sp_int_digit mask_a = (sp_int_digit)-1; + volatile sp_int_digit mask_b = (sp_int_digit)-1; sp_size_t i; /* Check result is as big as modulus. */ @@ -8226,9 +8227,9 @@ static void _sp_submod_ct(const sp_int* a, const sp_int* b, const sp_int* m, sp_int_digit h; sp_int_digit t; #endif - sp_int_digit mask; - sp_int_digit mask_a = (sp_int_digit)-1; - sp_int_digit mask_b = (sp_int_digit)-1; + volatile sp_int_digit mask; + volatile sp_int_digit mask_a = (sp_int_digit)-1; + volatile sp_int_digit mask_b = (sp_int_digit)-1; unsigned int i; /* In constant time, subtract b from a putting result in r. */ @@ -17449,7 +17450,7 @@ static int _sp_mont_red(sp_int* a, const sp_int* m, sp_int_digit mp, int ct) /* 1. mask = (1 << (NumBits(m) % WORD_SIZE)) - 1 * Mask when last digit of modulus doesn't have highest bit set. */ - sp_int_digit mask = (sp_int_digit) + volatile sp_int_digit mask = (sp_int_digit) (((sp_int_digit)1 << (bits & (SP_WORD_SIZE - 1))) - 1); /* Overflow. */ sp_int_word o = 0; @@ -17530,7 +17531,7 @@ static int _sp_mont_red(sp_int* a, const sp_int* m, sp_int_digit mp, int ct) int bits; sp_int_digit mu; sp_int_digit o; - sp_int_digit mask; + volatile sp_int_digit mask; #if 0 sp_print(a, "a"); @@ -18032,7 +18033,7 @@ int sp_unsigned_bin_size(const sp_int* a) int cnt = 0; if (a != NULL) { - cnt = (sp_count_bits(a) + 7) / 8; + cnt = (sp_count_bits(a) + 7) >> 3; } return cnt; @@ -18256,20 +18257,22 @@ int sp_to_unsigned_bin_len_ct(const sp_int* a, byte* out, int outSz) /* Start at the end of the buffer - least significant byte. */ int j; unsigned int i; - sp_int_digit mask = (sp_int_digit)-1; + volatile sp_int_digit mask = (sp_int_digit)-1; sp_int_digit d; /* Put each digit in. */ i = 0; for (j = outSz - 1; j >= 0; ) { unsigned int b; + volatile unsigned int notFull = (i < (unsigned int)a->used - 1); + d = a->dp[i]; /* Place each byte of a digit into the buffer. */ for (b = 0; (j >= 0) && (b < SP_WORD_SIZEOF); b++) { out[j--] = (byte)(d & mask); d >>= 8; } - mask &= (sp_int_digit)0 - (i < (unsigned int)a->used - 1); + mask &= (sp_int_digit)0 - notFull; i += (unsigned int)(1 & mask); } } @@ -18280,7 +18283,7 @@ int sp_to_unsigned_bin_len_ct(const sp_int* a, byte* out, int outSz) if (err == MP_OKAY) { unsigned int i; int j; - sp_int_digit mask = (sp_int_digit)-1; + volatile sp_int_digit mask = (sp_int_digit)-1; i = 0; for (j = outSz - 1; j >= 0; j--) { @@ -18351,11 +18354,12 @@ static int _sp_read_radix_16(sp_int* a, const char* in) /* Step through string a character at a time starting at end - least * significant byte. */ for (i = (int)(XSTRLEN(in) - 1); i >= 0; i--) { + volatile char c = in[i]; /* Convert character from hex. */ - int ch = (int)HexCharToByte(in[i]); + int ch = (int)HexCharToByte(c); /* Check for invalid character. */ if (ch < 0) { - if (!eol_done && CharIsWhiteSpace(in[i])) + if (!eol_done && CharIsWhiteSpace(c)) continue; err = MP_VAL; break; @@ -18415,7 +18419,6 @@ static int _sp_read_radix_10(sp_int* a, const char* in) { int err = MP_OKAY; int i; - char ch; /* Start with a being zero. */ _sp_zero(a); @@ -18423,7 +18426,7 @@ static int _sp_read_radix_10(sp_int* a, const char* in) /* Process all characters. */ for (i = 0; in[i] != '\0'; i++) { /* Get character. */ - ch = in[i]; + volatile char ch = in[i]; /* Check character is valid. */ if ((ch >= '0') && (ch <= '9')) { /* Assume '0'..'9' are continuous values as characters. */ @@ -18785,7 +18788,7 @@ int sp_radix_size(const sp_int* a, int radix, int* size) } else { /* Count of nibbles. */ - int cnt = (sp_count_bits(a) + 3) / 4; + int cnt = (sp_count_bits(a) + 3) >> 2; #ifndef WC_DISABLE_RADIX_ZERO_PAD /* Must have even number of nibbles to have complete bytes. */ if (cnt & 1) { @@ -19393,7 +19396,7 @@ static int _sp_prime_random_trials(const sp_int* a, int trials, int* result, { int err = MP_OKAY; int bits = sp_count_bits(a); - word32 baseSz = ((word32)bits + 7) / 8; + word32 baseSz = ((word32)bits + 7) >> 3; DECL_SP_INT_ARRAY(ds, a->used + 1, 2); DECL_SP_INT_ARRAY(d, a->used * 2 + 1, 2);