Address review feedback

Author: embhorn

src/internal.c

@@ -41491,7 +41491,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
                     {
                         RsaKey* key = (RsaKey*)ssl->hsKey;
                         volatile int lenErrMask;
-                        int lenErrMaskCopy;
+                        int mask;
 
                         ret = RsaDec(ssl,
                             input + args->idx,
@@ -41518,9 +41518,11 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
                             goto exit_dcke;
 
                         lenErrMask = 0 - (SECRET_LEN != args->sigSz);
-                        lenErrMaskCopy = lenErrMask;
-                        args->lastErr = (ret & (~lenErrMaskCopy)) |
-                            (WC_NO_ERR_TRACE(RSA_PAD_E) & lenErrMaskCopy);
+                        /* Snapshot volatile to avoid multiple volatile
+                         * accesses per expression. */
+                        mask = lenErrMask;
+                        args->lastErr = (ret & (~mask)) |
+                            (WC_NO_ERR_TRACE(RSA_PAD_E) & mask);
                         ret = 0;
                         break;
                     } /* rsa_kea */

wolfcrypt/src/rsa.c

@@ -1890,10 +1890,8 @@ static int RsaUnPad(const byte *pkcsBlock, unsigned int pkcsBlockLen,
         volatile byte   invalid = 0;
         volatile byte   minPad;
         volatile int    invalidMask;
-        word16 pastSepCopy;
-        byte   invalidCopy;
-        byte   minPadCopy;
-        int    invalidMaskCopy;
+        byte inv;
+        word16 sep;
 
         i = 0;
         /* Decrypted with private key - unpad must be constant time. */
@@ -1904,27 +1902,25 @@ static int RsaUnPad(const byte *pkcsBlock, unsigned int pkcsBlockLen,
             pastSep |= ctMask16Eq(pkcsBlock[j], 0x00);
         }
 
+        /* Snapshot volatiles to avoid multiple volatile accesses per
+         * expression. */
+        inv = invalid;
+        sep = pastSep;
+
         /* Minimum of 11 bytes of pre-message data - including leading 0x00. */
         minPad = ctMaskLT(i, RSA_MIN_PAD_SZ);
-        minPadCopy = minPad;
-        invalidCopy = invalid;
-        invalid = invalidCopy | minPadCopy;
+        inv |= minPad;
         /* Must have seen separator. */
-        pastSepCopy = pastSep;
-        invalidCopy = invalid;
-        invalid = invalidCopy | (byte)~pastSepCopy;
+        inv |= (byte)~sep;
         /* First byte must be 0x00. */
-        invalidCopy = invalid;
-        invalid = invalidCopy | ctMaskNotEq(pkcsBlock[0], 0x00);
+        inv |= ctMaskNotEq(pkcsBlock[0], 0x00);
         /* Check against expected block type: padValue */
-        invalidCopy = invalid;
-        invalid = invalidCopy | ctMaskNotEq(pkcsBlock[1], padValue);
+        inv |= ctMaskNotEq(pkcsBlock[1], padValue);
 
+        invalid = inv;
         *output = (byte *)(pkcsBlock + i);
-        invalidCopy = invalid;
-        invalidMask = (int)-1 + (int)(invalidCopy >> 7);
-        invalidMaskCopy = invalidMask;
-        ret = invalidMaskCopy & ((int)pkcsBlockLen - i);
+        invalidMask = (int)-1 + (int)(inv >> 7);
+        ret = invalidMask & ((int)pkcsBlockLen - i);
     }
 #endif