Skip to content

Commit b1c5a11

Browse files
committed
Fix ForceZero and use constant time
Previous commit to use ForceZero() didn't work, it should be wc_ForceZero(). Also, the AES key wrap padding should be constant time. F-497
1 parent d43ba51 commit b1c5a11

2 files changed

Lines changed: 27 additions & 8 deletions

File tree

src/crypto.c

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3318,12 +3318,31 @@ CK_RV C_Decrypt(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pEncryptedData,
33183318
if (mechanism == CKM_AES_KEY_WRAP_PAD) {
33193319
int i;
33203320
byte padValue = pData[decDataLen - 1];
3321-
if (padValue > KEYWRAP_BLOCK_SIZE || padValue > decDataLen)
3322-
return CKR_ENCRYPTED_DATA_LEN_RANGE;
3323-
for (i = 0; i < padValue; i++) {
3324-
if (pData[decDataLen - 1 - i] != padValue)
3325-
return CKR_ENCRYPTED_DATA_INVALID;
3321+
unsigned int badPad = 0;
3322+
unsigned int inPad;
3323+
3324+
/* Constant-time range check: padValue must be 1..KEYWRAP_BLOCK_SIZE
3325+
* and must not exceed decDataLen. */
3326+
badPad |= ((unsigned int)padValue - 1) >> 31;
3327+
badPad |= ((unsigned int)KEYWRAP_BLOCK_SIZE -
3328+
(unsigned int)padValue) >> 31;
3329+
badPad |= ((unsigned int)decDataLen -
3330+
(unsigned int)padValue) >> 31;
3331+
3332+
/* Constant-time padding byte verification.
3333+
* Always iterate KEYWRAP_BLOCK_SIZE times to avoid leaking
3334+
* padValue through iteration count. */
3335+
for (i = 0; i < KEYWRAP_BLOCK_SIZE; i++) {
3336+
/* Full mask: all-ones when i < padValue, else 0 */
3337+
inPad = 0 - (((unsigned int)i -
3338+
(unsigned int)padValue) >> 31);
3339+
badPad |= inPad &
3340+
((unsigned int)pData[decDataLen - 1 - i] ^
3341+
(unsigned int)padValue);
33263342
}
3343+
3344+
if (badPad != 0)
3345+
return CKR_ENCRYPTED_DATA_INVALID;
33273346
decDataLen -= padValue;
33283347
}
33293348
*pulDataLen = decDataLen;

src/internal.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2739,7 +2739,7 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest)
27392739
}
27402740

27412741
if (derBuf != NULL) {
2742-
ForceZero(derBuf, derSz);
2742+
wc_ForceZero(derBuf, derSz);
27432743
XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
27442744
}
27452745

@@ -2814,7 +2814,7 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest)
28142814

28152815
/* Clean up */
28162816
if (derBuf != NULL) {
2817-
ForceZero(derBuf, derSz);
2817+
wc_ForceZero(derBuf, derSz);
28182818
XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
28192819
}
28202820

@@ -8288,7 +8288,7 @@ void WP11_Object_Free(WP11_Object* object)
82888288
#endif
82898289
if ((object->type == CKK_AES || object->type == CKK_GENERIC_SECRET ||
82908290
object->type == CKK_HKDF) && object->data.symmKey != NULL) {
8291-
ForceZero(object->data.symmKey->data, object->data.symmKey->len);
8291+
wc_ForceZero(object->data.symmKey->data, object->data.symmKey->len);
82928292
XFREE(object->data.symmKey, NULL, DYNAMIC_TYPE_AES);
82938293
object->data.symmKey = NULL;
82948294
}

0 commit comments

Comments
 (0)