Skip to content

Commit

Permalink
Don't access AlgorithmGroup until necessary
Browse files Browse the repository at this point in the history
Accessing CNG properties is relatively slow to a cryptographic operation itself. This avoids eagerly reading the AlgorithmGroup property.
  • Loading branch information
vcsjones authored Aug 2, 2023
1 parent e2383d1 commit 1686311
Showing 1 changed file with 22 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,26 +207,30 @@ int ComputeKeySize()
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;
}
}
}

Expand Down

0 comments on commit 1686311

Please sign in to comment.