diff --git a/src/libraries/System.Security.Cryptography.Cng/tests/CngKeyTests.cs b/src/libraries/System.Security.Cryptography.Cng/tests/CngKeyTests.cs new file mode 100644 index 00000000000000..9acfbf663749f6 --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Cng/tests/CngKeyTests.cs @@ -0,0 +1,55 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Reflection; +using System.Runtime.InteropServices; +using Xunit; + +namespace System.Security.Cryptography.Tests +{ + public static class CngKeyTests + { + [Theory] + [MemberData(nameof(AllPublicProperties))] + public static void AllProperties_ThrowOnDisposal(string propertyName) + { + // Create a dummy key. Use a fast-ish algorithm. + // If we can't query the property even on a fresh key, exclude it from the tests. + + PropertyInfo propInfo = typeof(CngKey).GetProperty(propertyName); + CngKey theKey = CngKey.Create(CngAlgorithm.ECDsaP256); + + try + { + propInfo.GetValue(theKey); + } + catch + { + // Property getter threw an exception. It's nonsensical for us to query + // whether this same getter throws ObjectDisposedException once the object + // is disposed. So we'll just mark this test as success. + + return; + } + + // We've queried the property. Now dispose the object and query the property again. + // We should see an ObjectDisposedException. + + theKey.Dispose(); + Assert.ThrowsAny(() => + propInfo.GetValue(theKey, BindingFlags.DoNotWrapExceptions, null, null, null)); + } + + public static IEnumerable AllPublicProperties() + { + foreach (PropertyInfo pi in typeof(CngKey).GetProperties(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly)) + { + if (pi.GetMethod is not null && !typeof(SafeHandle).IsAssignableFrom(pi.PropertyType)) + { + yield return new[] { pi.Name }; + } + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj b/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj index e40ea90436f4ca..c4215a5c774a59 100644 --- a/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj @@ -82,6 +82,7 @@ Link="CommonTest\System\Security\Cryptography\AlgorithmImplementations\RSA\TestData.cs" /> + diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.StandardProperties.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.StandardProperties.cs index bf421ed398141b..d4d8f55e89c86d 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.StandardProperties.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.StandardProperties.cs @@ -20,6 +20,8 @@ public sealed partial class CngKey : IDisposable // Key properties // + private const int CachedKeySizeUninitializedSentinel = -1; + private int _cachedKeySize = CachedKeySizeUninitializedSentinel; /// /// Algorithm group this key can be used with @@ -170,52 +172,66 @@ public int KeySize { get { - int keySize = 0; - - // Attempt to use PublicKeyLength first as it returns the correct value for ECC keys - ErrorCode errorCode = Interop.NCrypt.NCryptGetIntProperty( - _keyHandle, - KeyPropertyName.PublicKeyLength, - ref keySize); + // Key size lookup is a common operation, and we don't want to incur the + // LRPC overhead to query lsass for the information. Since the key size + // cannot be changed on an existing key, we'll cache it. For consistency + // with other properties, we'll still throw if the underlying handle has + // been closed. + if (_cachedKeySize == CachedKeySizeUninitializedSentinel || _keyHandle.IsClosed) + { + _cachedKeySize = ComputeKeySize(); + } + return _cachedKeySize; - if (errorCode != ErrorCode.ERROR_SUCCESS) + int ComputeKeySize() { - // Fall back to Length (< Windows 10) - errorCode = Interop.NCrypt.NCryptGetIntProperty( + int keySize = 0; + + // Attempt to use PublicKeyLength first as it returns the correct value for ECC keys + ErrorCode errorCode = Interop.NCrypt.NCryptGetIntProperty( _keyHandle, - KeyPropertyName.Length, + KeyPropertyName.PublicKeyLength, ref keySize); - } - if (errorCode != ErrorCode.ERROR_SUCCESS) - { - throw errorCode.ToCryptographicException(); - } + if (errorCode != ErrorCode.ERROR_SUCCESS) + { + // Fall back to Length (< Windows 10) + errorCode = Interop.NCrypt.NCryptGetIntProperty( + _keyHandle, + KeyPropertyName.Length, + ref keySize); + } - // The platform crypto provider always returns "0" for EC keys when asked for a key size. This - // has been observed in Windows 10 and most recently observed in Windows 11 22H2. - // The Algorithm NCrypt Property only returns the Algorithm Group, so that doesn't work either. - // What does work is the ECCCurveName. - CngAlgorithmGroup? algorithmGroup = AlgorithmGroup; + if (errorCode != ErrorCode.ERROR_SUCCESS) + { + throw errorCode.ToCryptographicException(); + } - if (keySize == 0 && Provider == CngProvider.MicrosoftPlatformCryptoProvider && - (algorithmGroup == CngAlgorithmGroup.ECDiffieHellman || algorithmGroup == CngAlgorithmGroup.ECDsa)) - { - string? curve = _keyHandle.GetPropertyAsString(KeyPropertyName.ECCCurveName, CngPropertyOptions.None); + // The platform crypto provider always returns "0" for EC keys when asked for a key size. This + // has been observed in Windows 10 and most recently observed in Windows 11 22H2. + // The Algorithm NCrypt Property only returns the Algorithm Group, so that doesn't work either. + // What does work is the ECCCurveName. + CngAlgorithmGroup? algorithmGroup = AlgorithmGroup; - switch (curve) + if (keySize == 0 && Provider == CngProvider.MicrosoftPlatformCryptoProvider && + (algorithmGroup == CngAlgorithmGroup.ECDiffieHellman || algorithmGroup == CngAlgorithmGroup.ECDsa)) { - // nistP192 and nistP224 don't have named curve accelerators but we can handle them. - // These string values match the names in https://learn.microsoft.com/en-us/windows/win32/seccng/cng-named-elliptic-curves - case "nistP192": return 192; - case "nistP224": return 224; - case nameof(ECCurve.NamedCurves.nistP256): return 256; - case nameof(ECCurve.NamedCurves.nistP384): return 384; - case nameof(ECCurve.NamedCurves.nistP521): return 521; + string? curve = _keyHandle.GetPropertyAsString(KeyPropertyName.ECCCurveName, CngPropertyOptions.None); + + switch (curve) + { + // nistP192 and nistP224 don't have named curve accelerators but we can handle them. + // These string values match the names in https://learn.microsoft.com/en-us/windows/win32/seccng/cng-named-elliptic-curves + case "nistP192": return 192; + case "nistP224": return 224; + case nameof(ECCurve.NamedCurves.nistP256): return 256; + case nameof(ECCurve.NamedCurves.nistP384): return 384; + case nameof(ECCurve.NamedCurves.nistP521): return 521; + } } - } - return keySize; + return keySize; + } } }