diff --git a/SymCryptProvider/src/ciphers/p_scossl_aes.c b/SymCryptProvider/src/ciphers/p_scossl_aes.c index bfcf606d..2b35af73 100644 --- a/SymCryptProvider/src/ciphers/p_scossl_aes.c +++ b/SymCryptProvider/src/ciphers/p_scossl_aes.c @@ -155,19 +155,18 @@ static SCOSSL_STATUS p_scossl_aes_generic_decrypt_init(_Inout_ SCOSSL_AES_CTX *c // This function is adapted from ssl3_cbc_copy_mac in ssl/record/tls_pad.c // and runs in constant time. In case of bad padding, a random MAC is assigned instead static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx, - _In_reads_bytes_(buflen) unsigned char *record, _Inout_ SIZE_T *recordLen, + _In_reads_bytes_(buflen) unsigned char *record, + _Inout_ SIZE_T *recordLen, SIZE_T recordLenPadded, SCOSSL_STATUS paddingStatus) { // MAC rotation is performed in place BYTE rotatedMacBuf[64 + EVP_MAX_MD_SIZE]; PBYTE rotatedMac; - BYTE aux1, aux2, aux3, mask; BYTE paddingStatusByte = (BYTE) (paddingStatus & 0xff); - // Random mac set in case padding removal failed BYTE randMac[EVP_MAX_MD_SIZE]; - UINT32 macEnd = *record; + UINT32 macEnd = (UINT32)(*recordLen); // mac end index UINT32 macStart = macEnd - ctx->tlsMacSize; UINT32 inMac = 0; @@ -186,8 +185,7 @@ static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx, } *recordLen -= ctx->tlsMacSize; - - // Generate random bytes in case of bad padding + if (RAND_bytes_ex(ctx->libctx, randMac, ctx->tlsMacSize, 0) <= 0) { return SCOSSL_FAILURE; @@ -200,7 +198,7 @@ static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx, } rotatedMac = rotatedMacBuf + ((0 - (SIZE_T)rotatedMacBuf) & 0x3f); - + // Public info, safe to branch if (recordLenPadded > ctx->tlsMacSize + 255 + 1) { @@ -222,23 +220,74 @@ static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx, } // MAC rotation - for (i = 0, j = 0; i < ctx->tlsMacSize; i++) + for (i = 0; i < ctx->tlsMacSize; i++) { - // in case cache-line is 32 bytes, load from both lines and select appropriately - aux1 = rotatedMac[rotateOffset & ~0x20]; - aux2 = rotatedMac[rotateOffset | 0x20]; - mask = (BYTE) SYMCRYPT_MASK32_EQ(rotateOffset & !0x20, rotateOffset); - aux3 = SYMCRYPT_OPENSSL_MASK8_SELECT(mask, aux1, aux2); + BYTE macByte = 0; + for (j = 0; j < ctx->tlsMacSize; j++) { + UINT32 match = SYMCRYPT_MASK32_EQ(j, (rotateOffset + i) % ctx->tlsMacSize); + macByte |= rotatedMac[j] & match; + } + ctx->tlsMac[i] = SYMCRYPT_OPENSSL_MASK8_SELECT(paddingStatusByte, macByte, randMac[i]); + } + return SCOSSL_SUCCESS; +} + +SCOSSL_STATUS scossl_tls_remove_padding(PBYTE buf, SIZE_T *len) +{ + UINT32 cbPadVal; + UINT32 start; + UINT32 mPaddingError = 0; + BYTE last_block[SYMCRYPT_AES_BLOCK_SIZE]; + SCOSSL_STATUS ret = SCOSSL_SUCCESS; + + if (*len == 0 || (*len % SYMCRYPT_AES_BLOCK_SIZE) != 0) + return SCOSSL_FAILURE; + + cbPadVal = buf[*len - 1] + 1; + mPaddingError |= SymCryptMask32IsZeroU31(cbPadVal); + + // If cbPadVal is greater than cbBlockSize, + // we have to limit cbPadVal to be at most equal to cbBlockSize. + cbPadVal = 1 + ((cbPadVal - 1) & (SYMCRYPT_AES_BLOCK_SIZE - 1)); + // Copy last block to local buffer to ensure data-invariant access + start = *len - SYMCRYPT_AES_BLOCK_SIZE; + for (UINT32 i = 0; i < SYMCRYPT_AES_BLOCK_SIZE; i++) + last_block[i] = buf[start + i]; + + // Validate padding in constant time + for (UINT32 i = 0; i < SYMCRYPT_AES_BLOCK_SIZE; i++) { + UINT32 mask = (i >= SYMCRYPT_AES_BLOCK_SIZE - cbPadVal); + UINT32 diff = last_block[i] ^ cbPadVal; + mPaddingError |= (diff & -mask); + } + + ret ^= mPaddingError & (ret ^ SYMCRYPT_INVALID_ARGUMENT); + + *len -= cbPadVal; + return SCOSSL_SUCCESS; +} +static +SCOSSL_STATUS scossl_tls_add_padding(PBYTE out, SIZE_T outsize, PCBYTE in, SIZE_T inl, SIZE_T *outlen) +{ + UINT32 cbPadVal = SYMCRYPT_AES_BLOCK_SIZE - (inl % SYMCRYPT_AES_BLOCK_SIZE); + SIZE_T cbTotalLen = inl + cbPadVal; - ctx->tlsMac[j++] = SYMCRYPT_OPENSSL_MASK8_SELECT(paddingStatusByte, aux3, randMac[i]); + // Check if output buffer is large enough (non-secret, so safe to branch) + if (cbTotalLen > outsize) + return SCOSSL_FAILURE; - rotateOffset++; - rotateOffset = (rotateOffset & SYMCRYPT_MASK32_LT(rotateOffset, ctx->tlsMacSize)); + // Copy input to output + for (UINT32 i = 0; i < inl; i++) + out[i] = in[i]; + + // Write padding in constant time + for (UINT32 i = 0; i < SYMCRYPT_AES_BLOCK_SIZE; i++) { + UINT32 mask = (UINT32)(i < cbPadVal); // mask = 1 if i < padval, else 0 + out[inl + i] = (BYTE)((out[inl + i] & ~mask) | ((cbPadVal - 1) & mask)); } - // If we failed, we still succeed, but the MAC is set to some - // random value. It's up to the caller to check the MAC. + *outlen = cbTotalLen; return SCOSSL_SUCCESS; } @@ -268,11 +317,11 @@ static SCOSSL_STATUS p_scossl_aes_generic_block_update(_Inout_ SCOSSL_AES_CTX *c if (ctx->encrypt) { - // in == out - SymCryptPaddingPkcs7Add( - SYMCRYPT_AES_BLOCK_SIZE, - in, inl, - out, outsize, &inl); + if (!scossl_tls_add_padding(out, outsize, in, inl, &inl)) + { + ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL); + return SCOSSL_FAILURE; + } } if (inl % SYMCRYPT_AES_BLOCK_SIZE != 0 || @@ -310,12 +359,11 @@ static SCOSSL_STATUS p_scossl_aes_generic_block_update(_Inout_ SCOSSL_AES_CTX *c ERR_raise(ERR_LIB_PROV, PROV_R_CIPHER_OPERATION_FAILED); return SCOSSL_FAILURE; } - scError = SymCryptPaddingPkcs7Remove( - SYMCRYPT_AES_BLOCK_SIZE, - out, *outl, - out, *outl, - outl); - + if (!scossl_tls_remove_padding(out, outl)) + { + ERR_raise(ERR_LIB_PROV, PROV_R_CIPHER_OPERATION_FAILED); + return SCOSSL_FAILURE; + } return p_scossl_aes_copy_mac(ctx, out, outl, outlPadded,