Don't access AlgorithmGroup until necessary
authorKevin Jones <kevin@vcsjones.com>
Wed, 2 Aug 2023 04:35:18 +0000 (00:35 -0400)
committerGitHub <noreply@github.com>
Wed, 2 Aug 2023 04:35:18 +0000 (00:35 -0400)
Accessing CNG properties is relatively slow to a cryptographic operation itself. This avoids eagerly reading the AlgorithmGroup property.

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.StandardProperties.cs

index d4d8f55..67984d1 100644 (file)
@@ -207,26 +207,30 @@ namespace System.Security.Cryptography
                         throw errorCode.ToCryptographicException();
                     }
 
-                    // 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 (keySize == 0 && Provider == CngProvider.MicrosoftPlatformCryptoProvider &&
-                        (algorithmGroup == CngAlgorithmGroup.ECDiffieHellman || algorithmGroup == CngAlgorithmGroup.ECDsa))
+                    if (keySize == 0 && Provider == CngProvider.MicrosoftPlatformCryptoProvider)
                     {
-                        string? curve = _keyHandle.GetPropertyAsString(KeyPropertyName.ECCCurveName, CngPropertyOptions.None);
-
-                        switch (curve)
+                        // 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.
+                        // Accessing the AlgorithmGroup property is expensive which is why this is broken in to a separate
+                        // if block. We don't want to read from it unless we don't know the key size.
+                        CngAlgorithmGroup? algorithmGroup = AlgorithmGroup;
+
+                        if (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;
+                            }
                         }
                     }