From 29d2cc6d5b4ac72b9550e1fe73de2ce1f70c6d84 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Thu, 22 Apr 2021 17:08:35 -0500 Subject: [PATCH] Improve MbedTLS implementation of AES-CBC --- src/AudioStream.c | 2 +- src/InputStream.c | 7 ++--- src/PlatformCrypto.c | 61 ++++++++++++++++++++++++++++++++++++-------- src/PlatformCrypto.h | 5 ++-- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/AudioStream.c b/src/AudioStream.c index d4841bc..fd8f604 100644 --- a/src/AudioStream.c +++ b/src/AudioStream.c @@ -150,7 +150,7 @@ static void decodeInputData(PQUEUED_AUDIO_PACKET packet) { if (AudioEncryptionEnabled) { // We must have room for the AES padding which may be written to the buffer - unsigned char decryptedOpusData[MAX_PACKET_SIZE+16]; + unsigned char decryptedOpusData[ROUND_TO_PKCS7_PADDED_LEN(MAX_PACKET_SIZE)]; unsigned char iv[16] = { 0 }; int dataLength = packet->size - sizeof(*rtp); diff --git a/src/InputStream.c b/src/InputStream.c index cec07a0..1adf9c5 100644 --- a/src/InputStream.c +++ b/src/InputStream.c @@ -79,12 +79,13 @@ static int encryptData(unsigned char* plaintext, int plaintextLen, else { // PKCS7 padding may need to be added in-place, so we must copy this into a buffer // that can safely be modified. - unsigned char paddedData[MAX_INPUT_PACKET_SIZE]; + unsigned char paddedData[ROUND_TO_PKCS7_PADDED_LEN(MAX_INPUT_PACKET_SIZE)]; memcpy(paddedData, plaintext, plaintextLen); - // Prior to Gen 7, 128-bit AES CBC is used for encryption - return PltEncryptMessage(cryptoContext, ALGORITHM_AES_CBC, 0, + // Prior to Gen 7, 128-bit AES CBC is used for encryption with each message padded + // to the block size to ensure messages are not delayed within the cipher. + return PltEncryptMessage(cryptoContext, ALGORITHM_AES_CBC, CIPHER_FLAG_PAD_TO_BLOCK_SIZE, (unsigned char*)StreamConfig.remoteInputAesKey, sizeof(StreamConfig.remoteInputAesKey), currentAesIv, sizeof(currentAesIv), NULL, 0, diff --git a/src/PlatformCrypto.c b/src/PlatformCrypto.c index d535419..98b7ce1 100644 --- a/src/PlatformCrypto.c +++ b/src/PlatformCrypto.c @@ -10,6 +10,7 @@ bool RandomStateInitialized = false; #else #include #include +#endif static int addPkcs7PaddingInPlace(unsigned char* plaintext, int plaintextLen) { int paddedLength = ROUND_TO_PKCS7_PADDED_LEN(plaintextLen); @@ -19,9 +20,10 @@ static int addPkcs7PaddingInPlace(unsigned char* plaintext, int plaintextLen) { return paddedLength; } -#endif -// For CBC modes, inputData buffer must be allocated with length rounded up to next multiple of 16 and inputData buffer may be modified! +// When CIPHER_FLAG_PAD_TO_BLOCK_SIZE is used, inputData buffer must be allocated such that +// the buffer length is at least ROUND_TO_PKCS7_PADDED_LEN(inputDataLength) and inputData +// buffer may be modified! bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, unsigned char* key, int keyLength, unsigned char* iv, int ivLength, @@ -34,8 +36,6 @@ bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, switch (algorithm) { case ALGORITHM_AES_CBC: - LC_ASSERT(flags & CIPHER_FLAG_RESET_IV); - LC_ASSERT(flags & CIPHER_FLAG_FINISH); LC_ASSERT(tag == NULL); LC_ASSERT(tagLength == 0); cipherMode = MBEDTLS_MODE_CBC; @@ -70,9 +70,31 @@ bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, } } else { - if (mbedtls_cipher_crypt(&ctx->ctx, iv, ivLength, inputData, inputDataLength, outputData, &outLength) != 0) { + if (flags & CIPHER_FLAG_RESET_IV) { + if (mbedtls_cipher_set_iv(&ctx->ctx, iv, ivLength) != 0) { + return false; + } + + mbedtls_cipher_reset(&ctx->ctx); + } + + if (flags & CIPHER_FLAG_PAD_TO_BLOCK_SIZE) { + inputDataLength = addPkcs7PaddingInPlace(inputData, inputDataLength); + } + + if (mbedtls_cipher_update(&ctx->ctx, inputData, inputDataLength, outputData, &outLength) != 0) { return false; } + + if (flags & CIPHER_FLAG_FINISH) { + size_t finishLength; + + if (mbedtls_cipher_finish(&ctx->ctx, &outputData[outLength], &finishLength) != 0) { + return false; + } + + outLength += finishLength; + } } *outputDataLength = outLength; @@ -120,7 +142,9 @@ bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, ctx->initialized = true; } - inputDataLength = addPkcs7PaddingInPlace(inputData, inputDataLength); + if (flags & CIPHER_FLAG_PAD_TO_BLOCK_SIZE) { + inputDataLength = addPkcs7PaddingInPlace(inputData, inputDataLength); + } } if (EVP_EncryptUpdate(ctx->ctx, outputData, outputDataLength, inputData, inputDataLength) != 1) { @@ -154,7 +178,8 @@ bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, #endif } -// For CBC modes, outputData buffer must be allocated with length rounded up to next multiple of 16! +// When CBC is used, outputData buffer must be allocated such that the buffer length is +// at least ROUND_TO_PKCS7_PADDED_LEN(inputDataLength) to allow room for PKCS7 padding. bool PltDecryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, unsigned char* key, int keyLength, unsigned char* iv, int ivLength, @@ -167,8 +192,6 @@ bool PltDecryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, switch (algorithm) { case ALGORITHM_AES_CBC: - LC_ASSERT(flags & CIPHER_FLAG_RESET_IV); - LC_ASSERT(flags & CIPHER_FLAG_FINISH); LC_ASSERT(tag == NULL); LC_ASSERT(tagLength == 0); cipherMode = MBEDTLS_MODE_CBC; @@ -203,9 +226,27 @@ bool PltDecryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, } } else { - if (mbedtls_cipher_crypt(&ctx->ctx, iv, ivLength, inputData, inputDataLength, outputData, &outLength) != 0) { + if (flags & CIPHER_FLAG_RESET_IV) { + if (mbedtls_cipher_set_iv(&ctx->ctx, iv, ivLength) != 0) { + return false; + } + + mbedtls_cipher_reset(&ctx->ctx); + } + + if (mbedtls_cipher_update(&ctx->ctx, inputData, inputDataLength, outputData, &outLength) != 0) { return false; } + + if (flags & CIPHER_FLAG_FINISH) { + size_t finishLength; + + if (mbedtls_cipher_finish(&ctx->ctx, &outputData[outLength], &finishLength) != 0) { + return false; + } + + outLength += finishLength; + } } *outputDataLength = outLength; diff --git a/src/PlatformCrypto.h b/src/PlatformCrypto.h index 3f65833..2e9b353 100644 --- a/src/PlatformCrypto.h +++ b/src/PlatformCrypto.h @@ -27,8 +27,9 @@ void PltDestroyCryptoContext(PPLT_CRYPTO_CONTEXT ctx); #define ALGORITHM_AES_CBC 1 #define ALGORITHM_AES_GCM 2 -#define CIPHER_FLAG_RESET_IV 0x01 -#define CIPHER_FLAG_FINISH 0x02 +#define CIPHER_FLAG_RESET_IV 0x01 +#define CIPHER_FLAG_FINISH 0x02 +#define CIPHER_FLAG_PAD_TO_BLOCK_SIZE 0x04 bool PltEncryptMessage(PPLT_CRYPTO_CONTEXT ctx, int algorithm, int flags, unsigned char* key, int keyLength,