Skip to content

Commit 49a3a95

Browse files
brentschmaltzHP712
brentschmaltz
and
HP712
authored
Do not dispose SignatureProvider when AsymmetricAdapter faults. (#2682)
Move compacted SignatureProviders to new cache to ensure dispose is called. Add delegate to check if signature provider should be removed from cache. Separate expired from compaction Dispose SignatureProvider if it was never cached. Co-authored-by: id4s <[email protected]>
1 parent ecc2b08 commit 49a3a95

File tree

9 files changed

+290
-193
lines changed

9 files changed

+290
-193
lines changed

‎buildpackLocal.bat

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
dotnet clean Product.proj > clean.log
2+
dotnet build /r Product.proj
3+
dotnet pack --no-restore -o c:\localpackages --no-build Product.proj

‎src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,13 @@ public override bool Sign(ReadOnlySpan<byte> input, Span<byte> signature, out in
208208
catch
209209
{
210210
CryptoProviderCache?.TryRemove(this);
211-
Dispose(true);
212211
throw;
213212
}
214213
finally
215214
{
216-
if (!_disposed)
215+
if (asym != null)
217216
_asymmetricAdapterObjectPool.Free(asym);
218217
}
219-
220218
}
221219
#endif
222220

@@ -248,12 +246,11 @@ public override byte[] Sign(byte[] input)
248246
catch
249247
{
250248
CryptoProviderCache?.TryRemove(this);
251-
Dispose(true);
252249
throw;
253250
}
254251
finally
255252
{
256-
if (!_disposed)
253+
if (asym != null)
257254
_asymmetricAdapterObjectPool.Free(asym);
258255
}
259256
}
@@ -279,12 +276,11 @@ public override byte[] Sign(byte[] input, int offset, int count)
279276
catch
280277
{
281278
CryptoProviderCache?.TryRemove(this);
282-
Dispose(true);
283279
throw;
284280
}
285281
finally
286282
{
287-
if (!_disposed)
283+
if (asym != null)
288284
_asymmetricAdapterObjectPool.Free(asym);
289285
}
290286
}
@@ -380,12 +376,11 @@ public override bool Verify(byte[] input, byte[] signature)
380376
catch
381377
{
382378
CryptoProviderCache?.TryRemove(this);
383-
Dispose(true);
384379
throw;
385380
}
386381
finally
387382
{
388-
if (!_disposed)
383+
if (asym != null)
389384
_asymmetricAdapterObjectPool.Free(asym);
390385
}
391386
}
@@ -474,15 +469,14 @@ public override bool Verify(byte[] input, int inputOffset, int inputLength, byte
474469
}
475470
catch
476471
{
477-
Dispose(true);
472+
CryptoProviderCache?.TryRemove(this);
478473
throw;
479474
}
480475
finally
481476
{
482-
if (!_disposed)
477+
if (asym != null)
483478
_asymmetricAdapterObjectPool.Free(asym);
484479
}
485-
486480
}
487481

488482
/// <summary>

‎src/Microsoft.IdentityModel.Tokens/CryptoProviderFactory.cs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ public class CryptoProviderFactory
1818
private static readonly ConcurrentDictionary<string, string> _typeToAlgorithmMap = new ConcurrentDictionary<string, string>();
1919
private static readonly object _cacheLock = new object();
2020
private static int _defaultSignatureProviderObjectPoolCacheSize = Environment.ProcessorCount * 4;
21+
private static string _typeofAsymmetricSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
22+
private static string _typeofSymmetricSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
2123
private int _signatureProviderObjectPoolCacheSize = _defaultSignatureProviderObjectPoolCacheSize;
2224

2325
/// <summary>
@@ -513,7 +515,13 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
513515
{
514516
signatureProvider = CustomCryptoProvider.Create(algorithm, key, willCreateSignatures) as SignatureProvider;
515517
if (signatureProvider == null)
516-
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(LogMessages.IDX10646, LogHelper.MarkAsNonPII(algorithm), key, LogHelper.MarkAsNonPII(typeof(SignatureProvider)))));
518+
throw LogHelper.LogExceptionMessage(
519+
new InvalidOperationException(
520+
LogHelper.FormatInvariant(
521+
LogMessages.IDX10646,
522+
LogHelper.MarkAsNonPII(algorithm),
523+
key,
524+
LogHelper.MarkAsNonPII(typeof(SignatureProvider)))));
517525

518526
return signatureProvider;
519527
}
@@ -523,7 +531,7 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
523531
bool createAsymmetric = true;
524532
if (key is AsymmetricSecurityKey)
525533
{
526-
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
534+
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
527535
}
528536
else if (key is JsonWebKey jsonWebKey)
529537
{
@@ -533,22 +541,22 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
533541
{
534542
if (convertedSecurityKey is AsymmetricSecurityKey)
535543
{
536-
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
544+
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
537545
}
538546
else if (convertedSecurityKey is SymmetricSecurityKey)
539547
{
540-
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
548+
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
541549
createAsymmetric = false;
542550
}
543551
}
544552
// this code is simply to maintain the same exception thrown
545553
else
546554
{
547555
if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.RSA || jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.EllipticCurve)
548-
typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
556+
typeofSignatureProvider = _typeofAsymmetricSignatureProvider;
549557
else if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.Octet)
550558
{
551-
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
559+
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
552560
createAsymmetric = false;
553561
}
554562
}
@@ -560,12 +568,20 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
560568
}
561569
else if (key is SymmetricSecurityKey)
562570
{
563-
typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
571+
typeofSignatureProvider = _typeofSymmetricSignatureProvider;
564572
createAsymmetric = false;
565573
}
566574

567575
if (typeofSignatureProvider == null)
568-
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()))));
576+
throw LogHelper.LogExceptionMessage(
577+
new NotSupportedException(
578+
LogHelper.FormatInvariant(
579+
LogMessages.IDX10621,
580+
LogHelper.MarkAsNonPII(typeof(SymmetricSignatureProvider)),
581+
LogHelper.MarkAsNonPII(typeof(SecurityKey)),
582+
LogHelper.MarkAsNonPII(typeof(AsymmetricSecurityKey)),
583+
LogHelper.MarkAsNonPII(typeof(SymmetricSecurityKey)),
584+
LogHelper.MarkAsNonPII(key.GetType()))));
569585

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

594610
if (ShouldCacheSignatureProvider(signatureProvider))
595-
CryptoProviderCache.TryAdd(signatureProvider);
611+
signatureProvider.IsCached = CryptoProviderCache.TryAdd(signatureProvider);
596612
}
597613
}
598614
else
@@ -737,7 +753,7 @@ public virtual void ReleaseSignatureProvider(SignatureProvider signatureProvider
737753
signatureProvider.Release();
738754
if (CustomCryptoProvider != null && CustomCryptoProvider.IsSupportedAlgorithm(signatureProvider.Algorithm))
739755
CustomCryptoProvider.Release(signatureProvider);
740-
else if (signatureProvider.CryptoProviderCache == null && signatureProvider.RefCount == 0)
756+
else if (signatureProvider.CryptoProviderCache == null && signatureProvider.RefCount == 0 && !signatureProvider.IsCached)
741757
signatureProvider.Dispose();
742758
}
743759
}

0 commit comments

Comments
 (0)