Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One Shot ECB #52510

Merged
merged 40 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4e831fd
ECB decrypt skeleton
vcsjones May 6, 2021
0192aeb
Add AES ECB decrypt one-shot
vcsjones May 6, 2021
3648777
Simple test
vcsjones May 6, 2021
ba0391e
Add ECB decrypt tests
vcsjones May 6, 2021
21e3760
Decrypt in to buffer unless no padding
vcsjones May 6, 2021
7505b6a
Implement ECB encryption
vcsjones May 7, 2021
ef12105
Add more ECB encrypt tests
vcsjones May 7, 2021
7a808fd
Add tests for zero length buffers
vcsjones May 7, 2021
835cf76
Fixup ECB tests
vcsjones May 7, 2021
7a72745
Add 3DES and tests.
vcsjones May 7, 2021
425f6c8
Implement DES ECB
vcsjones May 8, 2021
5fd05ca
Implement RC2 one shot
vcsjones May 8, 2021
b205986
Fix overlapping buffers in CNG
vcsjones May 8, 2021
0768841
Allow-list one shot ECB for not overridding
vcsjones May 9, 2021
bf32394
Permit overlapping buffers on MacOS
vcsjones May 9, 2021
5993181
Get non-persisted keys working for CNG one shots
vcsjones May 13, 2021
7d0ea8d
Fix padding calculation for CNG types.
vcsjones May 13, 2021
9dbd999
Fix overlapping buffer check on CNG.
vcsjones May 13, 2021
a853ccd
AES/3DES persisted CNG key implementation and tests
vcsjones May 14, 2021
1df32b7
Fix indentation
vcsjones May 14, 2021
720018b
Fix white space
vcsjones May 14, 2021
b32fd8a
Merge remote-tracking branch 'ms/main' into 2406-one-shot-ecb
vcsjones Jun 7, 2021
b1c6ea3
Some code review feedback and WIP on documentation.
vcsjones Jun 8, 2021
02d68cb
Merge remote-tracking branch 'upstream/main' into 2406-one-shot-ecb
vcsjones Jun 14, 2021
ccdecda
Fix build
vcsjones Jun 14, 2021
500b829
Fix compilation, again
vcsjones Jun 14, 2021
2d43ec7
Remove reset. This does not hold over any cipher data
vcsjones Jun 14, 2021
3f0c165
Use pool for decrypt buffer
vcsjones Jun 14, 2021
16ff27f
Add missing empty line.
vcsjones Jun 16, 2021
b315f93
Fix whitespace
vcsjones Jun 23, 2021
03b5564
Finish XML documentation
vcsjones Jun 23, 2021
1ebb11c
Fixup exceptions
vcsjones Jun 23, 2021
23fe2a6
Fix handling core implementation that indicates it wrote more bytes
vcsjones Jun 24, 2021
1c65eec
Test and fix for overflow
vcsjones Jun 24, 2021
f20f7aa
Merge remote-tracking branch 'ms/main' into 2406-one-shot-ecb
vcsjones Jun 24, 2021
0d76e80
Fix grammar and punctuation
vcsjones Jun 25, 2021
33c649d
Remove unnecessary early return.
vcsjones Jun 25, 2021
0e3b568
Let Array.Resize do the work of creating a new array
vcsjones Jun 25, 2021
d017f9d
Simplify throwing exceptions for incorrect encryption length
vcsjones Jun 25, 2021
18f2478
Apply feedback consistently
vcsjones Jun 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,42 @@ public override int Transform(ReadOnlySpan<byte> input, Span<byte> output)
Debug.Assert(input.Length > 0);
Debug.Assert((input.Length % PaddingSizeInBytes) == 0);

int numBytesWritten;
int numBytesWritten = 0;

if (_encrypting)
// BCryptEncrypt and BCryptDecrypt can do in place encryption, but if the buffers overlap
// the offset must be zero. In that case, we need to copy to a temporary location.
if (input.Overlaps(output, out int offset) && offset != 0)
{
numBytesWritten = Interop.BCrypt.BCryptEncrypt(_hKey, input, _currentIv, output);
byte[] rented = CryptoPool.Rent(output.Length);

try
{
if (_encrypting)
{
numBytesWritten = Interop.BCrypt.BCryptEncrypt(_hKey, input, _currentIv, rented);
}
else
{
numBytesWritten = Interop.BCrypt.BCryptDecrypt(_hKey, input, _currentIv, rented);
}
vcsjones marked this conversation as resolved.
Show resolved Hide resolved

rented.AsSpan(0, numBytesWritten).CopyTo(output);
}
finally
{
CryptoPool.Return(rented, clearSize: numBytesWritten);
}
}
else
{
numBytesWritten = Interop.BCrypt.BCryptDecrypt(_hKey, input, _currentIv, output);
if (_encrypting)
{
numBytesWritten = Interop.BCrypt.BCryptEncrypt(_hKey, input, _currentIv, output);
}
else
{
numBytesWritten = Interop.BCrypt.BCryptDecrypt(_hKey, input, _currentIv, output);
}
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
}

if (numBytesWritten != input.Length)
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Internal/Cryptography/Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ internal static partial class Helpers
return (byte[])(src.Clone());
}

public static int GetPaddingSize(this SymmetricAlgorithm algorithm)
public static int GetPaddingSize(this SymmetricAlgorithm algorithm, CipherMode mode, int feedbackSizeBits)
{
// CFB8 does not require any padding at all
// otherwise, it is always required to pad for block size
if (algorithm.Mode == CipherMode.CFB && algorithm.FeedbackSize == 8)
if (mode == CipherMode.CFB && feedbackSizeBits == 8)
return 1;

return algorithm.BlockSize / 8;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to take the block size parameter separately and get rid of the algorithm/this parameter? Then nothing could sneak its way in.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably; I wanted to clean this all up a bit when I get to the CFB implementation. There's a bit of weirdness of so many things being passed around... some of it is in bits, some of it is in bytes...

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,70 @@ protected sealed override void Dispose(bool disposing)
base.Dispose(disposing);
}

public override unsafe bool TransformOneShot(ReadOnlySpan<byte> input, Span<byte> output, out int bytesWritten)
{
if (input.Length % PaddingSizeBytes != 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 (!DepaddingRequired)
{
if (output.Length >= input.Length)
{
bytesWritten = BasicSymmetricCipher.TransformFinal(input, output);
return true;
}

// If no padding is going to be removed, we know the buffer is too small and we can bail out.
bytesWritten = 0;
return false;
}

#if NET5_0_OR_GREATER
Span<byte> buffer = GC.AllocateUninitializedArray<byte>(input.Length);
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
#else
Span<byte> buffer = new byte[input.Length];
#endif
vcsjones marked this conversation as resolved.
Show resolved Hide resolved

fixed (byte* pBuffer = buffer)
{
int written = BasicSymmetricCipher.TransformFinal(input, buffer);

try
{
Span<byte> decrypted = buffer.Slice(0, written);
int unpaddedLength = GetPaddingLength(decrypted); // validates padding

if (unpaddedLength == 0)
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
{
bytesWritten = 0;
return true;
}
if (unpaddedLength > output.Length)
{
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
bytesWritten = 0;
return false;
}

decrypted.Slice(0, unpaddedLength).CopyTo(output);
bytesWritten = unpaddedLength;
return true;
}
finally
{
CryptographicOperations.ZeroMemory(buffer);
Reset();
}
}
}

private void Reset()
{
if (_heldoverCipher != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,33 @@ protected override byte[] UncheckedTransformFinalBlock(byte[] inputBuffer, int i
return buffer;
}

public override bool TransformOneShot(ReadOnlySpan<byte> input, Span<byte> output, out int bytesWritten)
{
int ciphertextLength = GetCiphertextLength(input.Length);

if (output.Length < ciphertextLength)
{
bytesWritten = 0;
return false;
}

// Copy the input to the output, and apply padding if required. This will not throw since the
// output length has already been checked, and PadBlock will not copy from input to output
// until it has checked that it will be able to apply padding correctly.
int padWritten = PadBlock(input, output);

// Do an in-place encrypt. All of our implementations support this, either natively
// or making a temporary buffer themselves if in-place is not supported by the native
// implementation.
Span<byte> paddedOutput = output.Slice(0, padWritten);
bytesWritten = BasicSymmetricCipher.TransformFinal(paddedOutput, paddedOutput);

// After padding, we should have an even number of blocks, and the same applies
// to the transform.
Debug.Assert(padWritten == bytesWritten);
return true;
}

private int GetCiphertextLength(int plaintextLength)
{
Debug.Assert(plaintextLength >= 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Internal.Cryptography
//
internal abstract class UniversalCryptoTransform : ICryptoTransform
{
public static ICryptoTransform Create(
public static UniversalCryptoTransform Create(
PaddingMode paddingMode,
BasicSymmetricCipher cipher,
bool encrypting)
Expand Down Expand Up @@ -108,6 +108,8 @@ public byte[] TransformFinalBlock(byte[] inputBuffer, int inputOffset, int input
return output;
}

public abstract bool TransformOneShot(ReadOnlySpan<byte> input, Span<byte> output, out int bytesWritten);

protected virtual void Dispose(bool disposing)
{
if (disposing)
Expand Down
Loading