From b3e380e3e64ab740fb2745f7ae234300578d021a Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 10 Mar 2022 19:17:07 -0500 Subject: [PATCH] One-shot decrypt in to user buffers This changes the one-shot decryption to allow decrypting directly in to user-supplied buffers, even in the presence of padding, so that a copy and CryptoPool rental can be avoided. If the buffer is large enough to accommodate the padding, then decrypt directly in to the supplied buffer, zero the padding, and indicate the de-padded amount in bytesWritten. If the buffer is possibly large enough, then decrypt all but the last block in to the buffer, and do the last block on the stack. If the de-padded amount in the stack is small enough, then copy the remaining. If it is too large, then zero everything in the user buffer. --- .../AES/AesCipherOneShotTests.cs | 66 ++++++++++ .../Symmetric/SymmetricOneShotBase.cs | 143 +++++++++++++++++++++ .../Cryptography/UniversalCryptoOneShot.cs | 140 ++++++++++++++++++-- 3 files changed, 337 insertions(+), 12 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherOneShotTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherOneShotTests.cs index bc109f9..c489db5 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherOneShotTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherOneShotTests.cs @@ -135,6 +135,40 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests yield return new object[] { // plaintext + ByteArrayFilledWith(0xCE, length: 144), + + // ciphertext + new byte[] + { + 0x16, 0xC3, 0x8D, 0x6B, 0x4F, 0x2C, 0x6E, 0xE7, + 0xC5, 0x17, 0xE2, 0xDB, 0x78, 0xD0, 0xFD, 0xF3, + 0xC4, 0xB2, 0xCC, 0x91, 0x9D, 0x7C, 0x66, 0xE0, + 0x9B, 0x24, 0xD7, 0x2C, 0xF4, 0x4D, 0x00, 0xA2, + 0x1A, 0xAB, 0x43, 0x6E, 0x1E, 0x5C, 0xDE, 0x03, + 0xDE, 0x79, 0x2D, 0x3F, 0x88, 0xEC, 0x88, 0x72, + 0xDB, 0x14, 0x98, 0x46, 0x04, 0x8E, 0x1A, 0x21, + 0x28, 0x35, 0x93, 0x57, 0x6C, 0x0C, 0x77, 0xAC, + 0x4B, 0x42, 0x29, 0x8E, 0xB9, 0x56, 0x4D, 0x0D, + 0x71, 0x9B, 0x80, 0xFE, 0x01, 0x6A, 0x90, 0x9B, + 0x35, 0x83, 0x8B, 0x0A, 0x41, 0xE2, 0x31, 0xC4, + 0xCF, 0xF5, 0x59, 0xA4, 0x50, 0xDC, 0x8E, 0xB6, + 0xA6, 0x53, 0x9F, 0xC5, 0xC3, 0x0C, 0x21, 0x87, + 0xD0, 0xC3, 0x29, 0xAF, 0xB8, 0xA0, 0xB8, 0x32, + 0x92, 0x6F, 0xA3, 0xAD, 0x4E, 0x91, 0xC3, 0x7E, + 0xD0, 0xF2, 0xEB, 0x78, 0x8A, 0xED, 0xD5, 0x9B, + 0x4F, 0xC0, 0xF6, 0x3F, 0xAD, 0x3F, 0x2C, 0xEC, + 0xCB, 0x59, 0x9D, 0xEC, 0x3F, 0xAE, 0xFD, 0xFE, + 0x10, 0xEC, 0x97, 0xDD, 0xF2, 0xC8, 0x83, 0xF5, + 0x01, 0x9C, 0x1D, 0x5A, 0x9D, 0x98, 0x5C, 0xAC, + }, + + PaddingMode.PKCS7, + CipherMode.CBC, + }; + + yield return new object[] + { + // plaintext new byte[] { 0x50, 0x68, 0x12, 0xA4, 0x5F, 0x08, 0xC8, 0x89, @@ -155,6 +189,38 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests yield return new object[] { // plaintext + ByteArrayFilledWith(0xCE, length: 144), + + // ciphertext + new byte[] + { + 0x16, 0xC3, 0x8D, 0x6B, 0x4F, 0x2C, 0x6E, 0xE7, + 0xC5, 0x17, 0xE2, 0xDB, 0x78, 0xD0, 0xFD, 0xF3, + 0xC4, 0xB2, 0xCC, 0x91, 0x9D, 0x7C, 0x66, 0xE0, + 0x9B, 0x24, 0xD7, 0x2C, 0xF4, 0x4D, 0x00, 0xA2, + 0x1A, 0xAB, 0x43, 0x6E, 0x1E, 0x5C, 0xDE, 0x03, + 0xDE, 0x79, 0x2D, 0x3F, 0x88, 0xEC, 0x88, 0x72, + 0xDB, 0x14, 0x98, 0x46, 0x04, 0x8E, 0x1A, 0x21, + 0x28, 0x35, 0x93, 0x57, 0x6C, 0x0C, 0x77, 0xAC, + 0x4B, 0x42, 0x29, 0x8E, 0xB9, 0x56, 0x4D, 0x0D, + 0x71, 0x9B, 0x80, 0xFE, 0x01, 0x6A, 0x90, 0x9B, + 0x35, 0x83, 0x8B, 0x0A, 0x41, 0xE2, 0x31, 0xC4, + 0xCF, 0xF5, 0x59, 0xA4, 0x50, 0xDC, 0x8E, 0xB6, + 0xA6, 0x53, 0x9F, 0xC5, 0xC3, 0x0C, 0x21, 0x87, + 0xD0, 0xC3, 0x29, 0xAF, 0xB8, 0xA0, 0xB8, 0x32, + 0x92, 0x6F, 0xA3, 0xAD, 0x4E, 0x91, 0xC3, 0x7E, + 0xD0, 0xF2, 0xEB, 0x78, 0x8A, 0xED, 0xD5, 0x9B, + 0x4F, 0xC0, 0xF6, 0x3F, 0xAD, 0x3F, 0x2C, 0xEC, + 0xCB, 0x59, 0x9D, 0xEC, 0x3F, 0xAE, 0xFD, 0xFE, + }, + + PaddingMode.None, + CipherMode.CBC, + }; + + yield return new object[] + { + // plaintext new byte[] { 0x50, 0x68, 0x12, 0xA4, 0x5F, 0x08, 0xC8, 0x89, diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/Symmetric/SymmetricOneShotBase.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/Symmetric/SymmetricOneShotBase.cs index d1059b0..7b56a70 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/Symmetric/SymmetricOneShotBase.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/Symmetric/SymmetricOneShotBase.cs @@ -436,6 +436,142 @@ namespace System.Security.Cryptography.Tests } } + [Theory] + [InlineData(PaddingMode.PKCS7, 0)] + [InlineData(PaddingMode.ANSIX923, 0)] + [InlineData(PaddingMode.ISO10126, 0)] + [InlineData(PaddingMode.PKCS7, 2048)] + [InlineData(PaddingMode.ANSIX923, 2048)] + [InlineData(PaddingMode.ISO10126, 2048)] + public void DecryptOneShot_Cbc_InvalidPadding_DoesNotContainPlaintext(PaddingMode paddingMode, int ciphertextSize) + { + using (SymmetricAlgorithm alg = CreateAlgorithm()) + { + alg.Key = Key; + int size = ciphertextSize > 0 ? ciphertextSize : alg.BlockSize / 8; + + byte plaintextByte = 0xFE; + byte[] invalidPadding = new byte[size]; + invalidPadding.AsSpan().Fill(plaintextByte); + byte[] destination = new byte[size]; + + // Use paddingMode: None since we manually padded our data + byte[] ciphertext = alg.EncryptCbc(invalidPadding, IV, paddingMode: PaddingMode.None); + Assert.Throws(() => alg.DecryptCbc(ciphertext, IV, destination, paddingMode: paddingMode)); + Assert.True(destination.AsSpan().IndexOf(plaintextByte) < 0, "does not contain plaintext data"); + } + } + + [Theory] + [InlineData(PaddingMode.PKCS7, 0)] + [InlineData(PaddingMode.ANSIX923, 0)] + [InlineData(PaddingMode.ISO10126, 0)] + [InlineData(PaddingMode.PKCS7, 2048)] + [InlineData(PaddingMode.ANSIX923, 2048)] + [InlineData(PaddingMode.ISO10126, 2048)] + public void DecryptOneShot_Ecb_InvalidPadding_DoesNotContainPlaintext(PaddingMode paddingMode, int ciphertextSize) + { + using (SymmetricAlgorithm alg = CreateAlgorithm()) + { + alg.Key = Key; + int size = ciphertextSize > 0 ? ciphertextSize : alg.BlockSize / 8; + + byte plaintextByte = 0xFE; + byte[] invalidPadding = new byte[size]; + invalidPadding.AsSpan().Fill(plaintextByte); + byte[] destination = new byte[size]; + + // Use paddingMode: None since we manually padded our data + byte[] ciphertext = alg.EncryptEcb(invalidPadding, paddingMode: PaddingMode.None); + Assert.Throws(() => alg.DecryptEcb(ciphertext, destination, paddingMode: paddingMode)); + Assert.True(destination.AsSpan().IndexOf(plaintextByte) < 0, "does not contain plaintext data"); + } + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [InlineData(PaddingMode.PKCS7, 0)] + [InlineData(PaddingMode.ANSIX923, 0)] + [InlineData(PaddingMode.ISO10126, 0)] + [InlineData(PaddingMode.PKCS7, 2048)] + [InlineData(PaddingMode.ANSIX923, 2048)] + [InlineData(PaddingMode.ISO10126, 2048)] + public void DecryptOneShot_Cfb_InvalidPadding_DoesNotContainPlaintext(PaddingMode paddingMode, int ciphertextSize) + { + using (SymmetricAlgorithm alg = CreateAlgorithm()) + { + if (alg is RC2) + { + throw new SkipTestException("RC2 does not support CFB."); + } + + alg.Key = Key; + int size = ciphertextSize > 0 ? ciphertextSize : alg.BlockSize / 8; + + byte plaintextByte = 0xFE; + byte[] invalidPadding = new byte[size]; + invalidPadding.AsSpan().Fill(plaintextByte); + byte[] destination = new byte[size]; + + // Use paddingMode: None since we manually padded our data + byte[] ciphertext = alg.EncryptCfb(invalidPadding, IV, paddingMode: PaddingMode.None); + Assert.Throws(() => alg.DecryptCfb(ciphertext, IV, destination, paddingMode: paddingMode)); + Assert.True(destination.AsSpan().IndexOf(plaintextByte) < 0, "does not contain plaintext data"); + } + } + + [Theory] + [InlineData(PaddingMode.PKCS7)] + [InlineData(PaddingMode.ANSIX923)] + [InlineData(PaddingMode.ISO10126)] + public void DecryptOneShot_Cbc_TooShortDoesNotContainPlaintext(PaddingMode paddingMode) + { + using (SymmetricAlgorithm alg = CreateAlgorithm()) + { + alg.Key = Key; + int size = 2048; + + byte plaintextByte = 0xFE; + Span plaintext = new byte[size + 1]; + plaintext.Fill(plaintextByte); + Span destination = new byte[size]; + byte[] ciphertext = alg.EncryptCbc(plaintext, IV, paddingMode: paddingMode); + + bool success = alg.TryDecryptCbc(ciphertext, IV, destination, out int bytesWritten, paddingMode: paddingMode); + Assert.False(success, nameof(alg.TryDecryptCbc)); + Assert.True(destination.IndexOf(plaintextByte) < 0, "does not contain plaintext data"); + Assert.Equal(0, bytesWritten); + } + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [InlineData(PaddingMode.PKCS7)] + [InlineData(PaddingMode.ANSIX923)] + [InlineData(PaddingMode.ISO10126)] + public void DecryptOneShot_Cfb8_TooShortDoesNotContainPlaintext(PaddingMode paddingMode) + { + using (SymmetricAlgorithm alg = CreateAlgorithm()) + { + if (alg is RC2) + { + throw new SkipTestException("RC2 does not support CFB."); + } + + alg.Key = Key; + int size = 2048; + + byte plaintextByte = 0xFE; + Span plaintext = new byte[size + 1]; + plaintext.Fill(0xFE); + Span destination = new byte[size]; + byte[] ciphertext = alg.EncryptCfb(plaintext, IV, paddingMode: paddingMode); + + bool success = alg.TryDecryptCfb(ciphertext, IV, destination, out int bytesWritten, paddingMode: paddingMode); + Assert.False(success, nameof(alg.TryDecryptCfb)); + Assert.True(destination.IndexOf(plaintextByte) < 0, "does not contain plaintext data"); + Assert.Equal(0, bytesWritten); + } + } + private static void AssertPlaintexts(ReadOnlySpan expected, ReadOnlySpan actual, PaddingMode padding) { if (padding == PaddingMode.Zeros) @@ -461,5 +597,12 @@ namespace System.Security.Cryptography.Tests AssertExtensions.SequenceEqual(expected, actual); } } + + protected static byte[] ByteArrayFilledWith(byte contents, int length) + { + byte[] ret = new byte[length]; + ret.AsSpan().Fill(contents); + return ret; + } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/UniversalCryptoOneShot.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/UniversalCryptoOneShot.cs index 7f2e5a9..50ced69 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/UniversalCryptoOneShot.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/UniversalCryptoOneShot.cs @@ -20,31 +20,147 @@ namespace System.Security.Cryptography if (input.Length % cipher.PaddingSizeInBytes != 0) throw new CryptographicException(SR.Cryptography_PartialBlock); - // If there is no padding that needs to be removed, and the output buffer is large enough to hold - // the resulting plaintext, we can decrypt directly in to the output buffer. - // We do not do this for modes that require padding removal. - // - // This is not done for padded ciphertexts because we don't know if the padding is valid - // until it's been decrypted. We don't want to decrypt in to a user-supplied buffer and then throw - // a padding exception after we've already filled the user buffer with plaintext. We should only - // release the plaintext to the caller once we know the padding is valid. - if (!SymmetricPadding.DepaddingRequired(paddingMode)) + // The internal implementation of the one-shots are never expected to create + // a plaintext larger than the ciphertext. If the buffer supplied is large enough + // to do the transform, use it directly. + // This does mean that the TransformFinal will write more than what is reported in + // bytesWritten when padding needs to be removed. The padding is "removed" simply + // by reporting less of the amount written, then zeroing out the extra padding. + if (output.Length >= input.Length) { - if (output.Length >= input.Length) + int bytesTransformed = cipher.TransformFinal(input, output); + Span transformBuffer = output.Slice(0, bytesTransformed); + + try { - bytesWritten = cipher.TransformFinal(input, output); + // validates padding + // This intentionally passes in BlockSizeInBytes instead of PaddingSizeInBytes. This is so that + // "extra padded" CFB data can still be decrypted. The .NET Framework always padded CFB8 to the + // block size, not the feedback size. We want the one-shot to be able to continue to decrypt + // those ciphertexts, so for CFB8 we are more lenient on the number of allowed padding bytes. + bytesWritten = SymmetricPadding.GetPaddingLength(transformBuffer, paddingMode, cipher.BlockSizeInBytes); + + // Zero out the padding so that the buffer does not contain the padding data "after" the bytesWritten. + CryptographicOperations.ZeroMemory(transformBuffer.Slice(bytesWritten)); return true; } + catch (CryptographicException) + { + // The padding is invalid, but don't leave the plaintext in the buffer. + CryptographicOperations.ZeroMemory(transformBuffer); + throw; + } + } - // If no padding is going to be removed, we know the buffer is too small and we can bail out. + // If no padding is going to removed, then we already know the buffer is too small + // since that requires a buffer at-least the size of the ciphertext. Bail out early. + // The second condition is where the output length is short by more than a whole block. + // All valid padding is at most one complete block. If the difference between the + // output and the input is more than a whole block then we know the output is too small. + if (!SymmetricPadding.DepaddingRequired(paddingMode) || + input.Length - cipher.BlockSizeInBytes > output.Length) + { bytesWritten = 0; return false; } + // At this point the destination might be big enough but we don't know until we've + // transformed the last block, and input is within one block of being the right + // size. + // For sufficiently small ciphertexts, do them on the stack. + // This buffer needs to be at least twice as big as the largest block size + // we support, which is 16 bytes for AES. + const int MaxInStackDecryptionBuffer = 128; + Span stackBuffer = stackalloc byte[MaxInStackDecryptionBuffer]; + + if (input.Length <= MaxInStackDecryptionBuffer) + { + int stackTransformFinal = cipher.TransformFinal(input, stackBuffer); + int depaddedLength = SymmetricPadding.GetPaddingLength( + stackBuffer.Slice(0, stackTransformFinal), + paddingMode, + cipher.BlockSizeInBytes); + Span writtenDepadded = stackBuffer.Slice(0, depaddedLength); + + if (output.Length < depaddedLength) + { + CryptographicOperations.ZeroMemory(writtenDepadded); + bytesWritten = 0; + return false; + } + + writtenDepadded.CopyTo(output); + CryptographicOperations.ZeroMemory(writtenDepadded); + bytesWritten = depaddedLength; + return true; + } + + // If the source and destination do not overlap, we can decrypt directly in to the user buffer. + if (!input.Overlaps(output, out int overlap) || overlap == 0) + { + // At this point we know that we have multiple blocks that need to be decrypted. + // So we decrypt all but the last block directly in to the buffer. The final + // block we decrypt in to a stack buffer, and if it fits, copy the last block to + // the output. + + // We should only get here if we have multiple blocks to transform. The single + // block case should have happened on the stack. + Debug.Assert(input.Length > cipher.BlockSizeInBytes); + + int writtenToOutput = 0; + int finalTransformWritten = 0; + + // CFB8 means this may not be an exact multiple of the block size. + // If the an AES CFB8 ciphertext length is 129 with PKCS7 padding, then + // we'll have 113 bytes in the unpaddedBlocks and 16 in the paddedBlock. + // We still need to do this on block size, not padding size. The CFB8 + // padding byte might be block size. We don't want unpaddedBlocks to + // contain removable padding, so split on block size. + ReadOnlySpan unpaddedBlocks = input[..^cipher.BlockSizeInBytes]; + ReadOnlySpan paddedBlock = input[^cipher.BlockSizeInBytes..]; + Debug.Assert(paddedBlock.Length % cipher.BlockSizeInBytes == 0); + Debug.Assert(paddedBlock.Length <= MaxInStackDecryptionBuffer); + + try + { + writtenToOutput = cipher.Transform(unpaddedBlocks, output); + finalTransformWritten = cipher.TransformFinal(paddedBlock, stackBuffer); + + // This will throw on invalid padding. + int depaddedLength = SymmetricPadding.GetPaddingLength( + stackBuffer.Slice(0, finalTransformWritten), + paddingMode, + cipher.BlockSizeInBytes); + Span depaddedFinalTransform = stackBuffer.Slice(0, depaddedLength); + + if (output.Length - writtenToOutput < depaddedLength) + { + CryptographicOperations.ZeroMemory(depaddedFinalTransform); + CryptographicOperations.ZeroMemory(output.Slice(0, writtenToOutput)); + bytesWritten = 0; + return false; + } + + depaddedFinalTransform.CopyTo(output.Slice(writtenToOutput)); + CryptographicOperations.ZeroMemory(depaddedFinalTransform); + bytesWritten = writtenToOutput + depaddedLength; + return true; + } + catch (CryptographicException) + { + CryptographicOperations.ZeroMemory(output.Slice(0, writtenToOutput)); + CryptographicOperations.ZeroMemory(stackBuffer.Slice(0, finalTransformWritten)); + throw; + } + } + + // If we get here, then we have multiple blocks with overlapping buffers that don't fit in the stack. + // We need to rent and copy for this. byte[] rentedBuffer = CryptoPool.Rent(input.Length); Span buffer = rentedBuffer.AsSpan(0, input.Length); Span decryptedBuffer = default; + // Keep our buffer fixed so the GC doesn't move it around before we clear and return to the pool. fixed (byte* pBuffer = buffer) { try -- 2.7.4