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.

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

Dispose SignatureProviders after moving to staging cache. #2682

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Dispose SignatureProviders after moving to staging cache. #2682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions buildpackLocal.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
dotnet clean Product.proj > clean.log
brentschmaltz marked this conversation as resolved.
Show resolved Hide resolved
dotnet build /r Product.proj
dotnet pack --no-restore -o c:\localpackages --no-build Product.proj
18 changes: 6 additions & 12 deletions src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,13 @@ public override bool Sign(ReadOnlySpan<byte> input, Span<byte> signature, out in
catch
{
CryptoProviderCache?.TryRemove(this);
Dispose(true);
throw;
}
finally
{
if (!_disposed)
if (asym != null)
_asymmetricAdapterObjectPool.Free(asym);
}

}
#endif

Expand Down Expand Up @@ -248,12 +246,11 @@ public override byte[] Sign(byte[] input)
catch
{
CryptoProviderCache?.TryRemove(this);
Dispose(true);
throw;
}
finally
{
if (!_disposed)
if (asym != null)
_asymmetricAdapterObjectPool.Free(asym);
}
}
Expand All @@ -279,12 +276,11 @@ public override byte[] Sign(byte[] input, int offset, int count)
catch
{
CryptoProviderCache?.TryRemove(this);
Dispose(true);
throw;
}
finally
{
if (!_disposed)
if (asym != null)
_asymmetricAdapterObjectPool.Free(asym);
}
}
Expand Down Expand Up @@ -380,12 +376,11 @@ public override bool Verify(byte[] input, byte[] signature)
catch
{
CryptoProviderCache?.TryRemove(this);
Dispose(true);
throw;
}
finally
{
if (!_disposed)
if (asym != null)
_asymmetricAdapterObjectPool.Free(asym);
}
}
Expand Down Expand Up @@ -474,15 +469,14 @@ public override bool Verify(byte[] input, int inputOffset, int inputLength, byte
}
catch
{
Dispose(true);
CryptoProviderCache?.TryRemove(this);
throw;
}
finally
{
if (!_disposed)
if (asym != null)
_asymmetricAdapterObjectPool.Free(asym);
}

}

/// <summary>
Expand Down
36 changes: 26 additions & 10 deletions src/Microsoft.IdentityModel.Tokens/CryptoProviderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class CryptoProviderFactory
private static readonly ConcurrentDictionary<string, string> _typeToAlgorithmMap = new ConcurrentDictionary<string, string>();
private static readonly object _cacheLock = new object();
private static int _defaultSignatureProviderObjectPoolCacheSize = Environment.ProcessorCount * 4;
private static string _typeofAsymmetricSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
private static string _typeofSymmetricSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
private int _signatureProviderObjectPoolCacheSize = _defaultSignatureProviderObjectPoolCacheSize;

/// <summary>
Expand Down Expand Up @@ -513,7 +515,13 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
{
signatureProvider = CustomCryptoProvider.Create(algorithm, key, willCreateSignatures) as SignatureProvider;
if (signatureProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(LogMessages.IDX10646, LogHelper.MarkAsNonPII(algorithm), key, LogHelper.MarkAsNonPII(typeof(SignatureProvider)))));
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX10646,
LogHelper.MarkAsNonPII(algorithm),
key,
LogHelper.MarkAsNonPII(typeof(SignatureProvider)))));

return signatureProvider;
}
Expand All @@ -523,7 +531,7 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
bool createAsymmetric = true;
if (key is AsymmetricSecurityKey)
{
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
brentschmaltz marked this conversation as resolved.
Show resolved Hide resolved
}
else if (key is JsonWebKey jsonWebKey)
{
Expand All @@ -533,22 +541,22 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
{
if (convertedSecurityKey is AsymmetricSecurityKey)
{
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
}
else if (convertedSecurityKey is SymmetricSecurityKey)
{
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
createAsymmetric = false;
}
}
// this code is simply to maintain the same exception thrown
else
{
if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.RSA || jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.EllipticCurve)
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
else if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.Octet)
{
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
createAsymmetric = false;
}
}
Expand All @@ -560,12 +568,20 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
}
else if (key is SymmetricSecurityKey)
{
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
createAsymmetric = false;
}

if (typeofSignatureProvider == null)
throw LogHelper.LogExceptionMessage(new NotSupportedException(LogHelper.FormatInvariant(LogMessages.IDX10621, LogHelper.MarkAsNonPII(typeof(SymmetricSignatureProvider)), LogHelper.MarkAsNonPII(typeof(SecurityKey)), LogHelper.MarkAsNonPII(typeof(AsymmetricSecurityKey)), LogHelper.MarkAsNonPII(typeof(SymmetricSecurityKey)), LogHelper.MarkAsNonPII(key.GetType()))));
throw LogHelper.LogExceptionMessage(
new NotSupportedException(
LogHelper.FormatInvariant(
LogMessages.IDX10621,
LogHelper.MarkAsNonPII(typeof(SymmetricSignatureProvider)),
LogHelper.MarkAsNonPII(typeof(SecurityKey)),
LogHelper.MarkAsNonPII(typeof(AsymmetricSecurityKey)),
LogHelper.MarkAsNonPII(typeof(SymmetricSecurityKey)),
LogHelper.MarkAsNonPII(key.GetType()))));

if (CacheSignatureProviders && cacheProvider)
{
Expand All @@ -592,7 +608,7 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
signatureProvider = new SymmetricSignatureProvider(key, algorithm, willCreateSignatures);

if (ShouldCacheSignatureProvider(signatureProvider))
CryptoProviderCache.TryAdd(signatureProvider);
signatureProvider.IsCached = CryptoProviderCache.TryAdd(signatureProvider);
brentschmaltz marked this conversation as resolved.
Show resolved Hide resolved
}
}
else
Expand Down Expand Up @@ -737,7 +753,7 @@ public virtual void ReleaseSignatureProvider(SignatureProvider signatureProvider
signatureProvider.Release();
if (CustomCryptoProvider != null && CustomCryptoProvider.IsSupportedAlgorithm(signatureProvider.Algorithm))
CustomCryptoProvider.Release(signatureProvider);
else if (signatureProvider.CryptoProviderCache == null && signatureProvider.RefCount == 0)
else if (signatureProvider.CryptoProviderCache == null && signatureProvider.RefCount == 0 && !signatureProvider.IsCached)
signatureProvider.Dispose();
}
}
Expand Down
Loading