Skip to content

Cache AES one-shot cipher handles #129996

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.

Sign up for GitHub

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

Open
vcsjones wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Cache AES one-shot cipher handles #129996

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2a2fde
Cache AES one-shot cipher handles
vcsjones Jun 29, 2026
b0fcfea
Zero key when in stack buffer
vcsjones Jun 29, 2026
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
211 changes: 134 additions & 77 deletions ...raries/System.Security.Cryptography/src/System/Security/Cryptography/AesImplementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,19 @@ namespace System.Security.Cryptography
internal sealed partial class AesImplementation : Aes
{
private FixedMemoryKeyBox? _keyBox;
private ILiteSymmetricCipher? _encryptEcbCipher;
private ILiteSymmetricCipher? _decryptEcbCipher;
private ILiteSymmetricCipher? _encryptCbcCipher;
private ILiteSymmetricCipher? _decryptCbcCipher;
private ConcurrencyBlock _block;

@vcsjones vcsjones Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We use a single ConcurrencyBlock for everything. In theory mix-and-matching concurrent calls to EncryptCbc and DecryptCbc would "work" if we had a per-handle block but that is just making the intentions less clear. I want to avoid "Sometimes concurrency is okay, sometimes it isn't" and just go for "it isn't".


private FixedMemoryKeyBox GetKey()
{
if (_keyBox is null)
{
GenerateKey();
Span<byte> key = stackalloc byte[KeySize / BitsPerByte];

@vcsjones vcsjones Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can't use GenerateKey() here because ConcurrencyBlock is not re-entrant. GetKey can be called while a ConcurrencyBlock is already being held and GenerateKey is also under a ConcurrencyBlock. We could have a GenerateKeyUnchecked or similar but given that's is 2 lines of code seemed not worth it.

RandomNumberGenerator.Fill(key);
SetKeyCoreUnchecked(key);
Comment thread
vcsjones marked this conversation as resolved.
Outdated
Comment thread
vcsjones marked this conversation as resolved.
Outdated
Debug.Assert(_keyBox is not null);
}

Expand All @@ -32,9 +39,13 @@ public override int KeySize
get => base.KeySize;
set
{
base.KeySize = value;
_keyBox?.Dispose();
_keyBox = null;
using (ConcurrencyBlock.Enter(ref _block))
{
base.KeySize = value;
ClearCachedCiphers();
_keyBox?.Dispose();
_keyBox = null;
}
}
}

Expand Down Expand Up @@ -78,6 +89,7 @@ protected sealed override void Dispose(bool disposing)
{
if (disposing)
{
ClearCachedCiphers();
_keyBox?.Dispose();
_keyBox = null;
}
Expand All @@ -87,9 +99,10 @@ protected sealed override void Dispose(bool disposing)

protected override void SetKeyCore(ReadOnlySpan<byte> key)
{
KeySizeValue = checked(BitsPerByte * key.Length);
_keyBox?.Dispose();
_keyBox = new FixedMemoryKeyBox(key);
using (ConcurrencyBlock.Enter(ref _block))
{
SetKeyCoreUnchecked(key);
}
}

protected override bool TryDecryptEcbCore(
Expand All @@ -98,19 +111,14 @@ protected override bool TryDecryptEcbCore(
PaddingMode paddingMode,
out int bytesWritten)
{
ILiteSymmetricCipher cipher = GetKey().UseKey(
BlockSize / BitsPerByte,
static (blockSizeBytes, key) => CreateLiteCipher(
using (ConcurrencyBlock.Enter(ref _block))
{
ILiteSymmetricCipher cipher = GetOrCreateCachedLiteCipher(
ref _decryptEcbCipher,
CipherMode.ECB,
key,
iv: default,
blockSize: blockSizeBytes,
paddingSize: blockSizeBytes,
0, /*feedback size */
encrypting: false));
encrypting: false);

using (cipher)
{
return UniversalCryptoOneShot.OneShotDecrypt(cipher, paddingMode, ciphertext, destination, out bytesWritten);
}
}
Expand All @@ -121,19 +129,14 @@ protected override bool TryEncryptEcbCore(
PaddingMode paddingMode,
out int bytesWritten)
{
ILiteSymmetricCipher cipher = GetKey().UseKey(
BlockSize / BitsPerByte,
static (blockSizeBytes, key) => CreateLiteCipher(
using (ConcurrencyBlock.Enter(ref _block))
{
ILiteSymmetricCipher cipher = GetOrCreateCachedLiteCipher(
ref _encryptEcbCipher,
CipherMode.ECB,
key,
iv: default,
blockSize: blockSizeBytes,
paddingSize: blockSizeBytes,
0, /*feedback size */
encrypting: true));
encrypting: true);

using (cipher)
{
return UniversalCryptoOneShot.OneShotEncrypt(cipher, paddingMode, plaintext, destination, out bytesWritten);
}
}
Expand All @@ -145,20 +148,14 @@ protected override bool TryEncryptCbcCore(
PaddingMode paddingMode,
out int bytesWritten)
{
ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
BlockSize / BitsPerByte,
static (iv, blockSizeBytes, key) => CreateLiteCipher(
using (ConcurrencyBlock.Enter(ref _block))
{
ILiteSymmetricCipher cipher = GetOrCreateCachedLiteCipher(
ref _encryptCbcCipher,
CipherMode.CBC,
key,
iv,
blockSize: blockSizeBytes,
paddingSize: blockSizeBytes,
0, /*feedback size */
encrypting: true));
encrypting: true);

using (cipher)
{
return UniversalCryptoOneShot.OneShotEncrypt(cipher, paddingMode, plaintext, destination, out bytesWritten);
}
}
Expand All @@ -170,20 +167,14 @@ protected override bool TryDecryptCbcCore(
PaddingMode paddingMode,
out int bytesWritten)
{
ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
BlockSize / BitsPerByte,
static (iv, blockSizeBytes, key) => CreateLiteCipher(
using (ConcurrencyBlock.Enter(ref _block))
{
ILiteSymmetricCipher cipher = GetOrCreateCachedLiteCipher(
ref _decryptCbcCipher,
CipherMode.CBC,
key,
iv,
blockSize: blockSizeBytes,
paddingSize: blockSizeBytes,
0, /*feedback size */
encrypting: false));
encrypting: false);

using (cipher)
{
return UniversalCryptoOneShot.OneShotDecrypt(cipher, paddingMode, ciphertext, destination, out bytesWritten);
}
}
Expand All @@ -198,21 +189,24 @@ protected override bool TryDecryptCfbCore(
{
ValidateCFBFeedbackSize(feedbackSizeInBits);

ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
(BlockSizeBytes: BlockSize / BitsPerByte, FeedbackSizeBytes: feedbackSizeInBits / BitsPerByte),
static (iv, state, key) => CreateLiteCipher(
CipherMode.CFB,
key,
iv: iv,
blockSize: state.BlockSizeBytes,
paddingSize: state.FeedbackSizeBytes,
state.FeedbackSizeBytes,
encrypting: false));

using (cipher)
using (ConcurrencyBlock.Enter(ref _block))

@vcsjones vcsjones Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Even though CFB is not cached it felt odd to leave it out of the ConcurrencyBlock. We can take it out but I want to more or less make it official: don't call these things concurrently.

{
return UniversalCryptoOneShot.OneShotDecrypt(cipher, paddingMode, ciphertext, destination, out bytesWritten);
ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
(BlockSizeBytes: BlockSize / BitsPerByte, FeedbackSizeBytes: feedbackSizeInBits / BitsPerByte),
static (iv, state, key) => CreateLiteCipher(
CipherMode.CFB,
key,
iv: iv,
blockSize: state.BlockSizeBytes,
paddingSize: state.FeedbackSizeBytes,
state.FeedbackSizeBytes,
encrypting: false));

using (cipher)
{
return UniversalCryptoOneShot.OneShotDecrypt(cipher, paddingMode, ciphertext, destination, out bytesWritten);
}
}
}

Expand All @@ -226,21 +220,24 @@ protected override bool TryEncryptCfbCore(
{
ValidateCFBFeedbackSize(feedbackSizeInBits);

ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
(BlockSizeBytes: BlockSize / BitsPerByte, FeedbackSizeBytes: feedbackSizeInBits / BitsPerByte),
static (iv, state, key) => CreateLiteCipher(
CipherMode.CFB,
key,
iv,
blockSize: state.BlockSizeBytes,
paddingSize: state.FeedbackSizeBytes,
state.FeedbackSizeBytes,
encrypting: true));

using (cipher)
using (ConcurrencyBlock.Enter(ref _block))
{
return UniversalCryptoOneShot.OneShotEncrypt(cipher, paddingMode, plaintext, destination, out bytesWritten);
ILiteSymmetricCipher cipher = GetKey().UseKey(
iv,
(BlockSizeBytes: BlockSize / BitsPerByte, FeedbackSizeBytes: feedbackSizeInBits / BitsPerByte),
static (iv, state, key) => CreateLiteCipher(
CipherMode.CFB,
key,
iv,
blockSize: state.BlockSizeBytes,
paddingSize: state.FeedbackSizeBytes,
state.FeedbackSizeBytes,
encrypting: true));

using (cipher)
{
return UniversalCryptoOneShot.OneShotEncrypt(cipher, paddingMode, plaintext, destination, out bytesWritten);
}
}
}

Expand Down Expand Up @@ -291,6 +288,66 @@ private static void ValidateCFBFeedbackSize(int feedback)
}
}

private ILiteSymmetricCipher GetOrCreateCachedLiteCipher(
ref ILiteSymmetricCipher? cipher,
CipherMode cipherMode,
ReadOnlySpan<byte> iv,
bool encrypting)
{
Debug.Assert(cipherMode is CipherMode.ECB or CipherMode.CBC);

if (cipher is not null)
{
try
{
cipher.Reset(iv);
return cipher;
}
catch
{
cipher.Dispose();
cipher = null; // Null-out the cipher field passed by reference.
throw;
}
}

int blockSizeBytes = BlockSize / BitsPerByte;
cipher = GetKey().UseKey(
iv,
(BlockSizeBytes: blockSizeBytes, CipherMode: cipherMode, Encrypting: encrypting),
static (iv, state, key) => CreateLiteCipher(
state.CipherMode,
key,
iv,
blockSize: state.BlockSizeBytes,
paddingSize: state.BlockSizeBytes,
0, /* feedback size */
encrypting: state.Encrypting));

return cipher;
}

private void SetKeyCoreUnchecked(ReadOnlySpan<byte> key)
{
KeySizeValue = checked(BitsPerByte * key.Length);
FixedMemoryKeyBox keyBox = new FixedMemoryKeyBox(key);
ClearCachedCiphers();
_keyBox?.Dispose();
_keyBox = keyBox;
}

private void ClearCachedCiphers()
{
_encryptEcbCipher?.Dispose();
_encryptEcbCipher = null;
_decryptEcbCipher?.Dispose();
_decryptEcbCipher = null;
_encryptCbcCipher?.Dispose();
_encryptCbcCipher = null;
_decryptCbcCipher?.Dispose();
_decryptCbcCipher = null;
}

private const int BitsPerByte = 8;
}
}
Loading