[Perf] Cache key size in CngKey (#89599)

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 (file)
index 0000000..9acfbf6
--- /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<ObjectDisposedException>(() =>
+                propInfo.GetValue(theKey, BindingFlags.DoNotWrapExceptions, null, null, null));
+        }
+
+        public static IEnumerable<object[]> 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 e40ea90..c4215a5 100644 (file)
--- 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" />
     <Compile Include="$(CommonTestPath)System\IO\PositionValueStream.cs"
              Link="CommonTest\System\IO\PositionValueStream.cs" />
+    <Compile Include="CngKeyTests.cs" />
     <Compile Include="CngPkcs8Tests.cs" />
     <Compile Include="DSACngPkcs8Tests.cs" />
     <Compile Include="DSACngProvider.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 bf421ed..d4d8f55 100644 (file)
--- 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 @@ namespace System.Security.Cryptography
         // Key properties
         //
 
+        private const int CachedKeySizeUninitializedSentinel = -1;
+        private int _cachedKeySize = CachedKeySizeUninitializedSentinel;
 
         /// <summary>
         ///     Algorithm group this key can be used with
@@ -170,52 +172,66 @@ namespace System.Security.Cryptography
         {
             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;
+                }
             }
         }