Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ECParameters that contain only D (Win, Linux). #33874

Merged
merged 25 commits into from
Apr 5, 2020
Merged

Support ECParameters that contain only D (Win, Linux). #33874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
05613ae
Support ECParameters that contain only D.
vcsjones Mar 20, 2020
02581c7
Support missing Q on Windows.
vcsjones Mar 26, 2020
7144a5e
Avoid complex logic.
vcsjones Mar 27, 2020
f387e9d
Add PEM/PKCS8 encoded tests.
vcsjones Mar 27, 2020
20c61ae
Fix tests for macOS
vcsjones Mar 27, 2020
99dbb01
Fix build break.
vcsjones Mar 27, 2020
57a477f
Only support named curves.
vcsjones Mar 28, 2020
d60707a
Restore length check.
vcsjones Mar 28, 2020
b892e17
Remove tests from the CNG types.
vcsjones Mar 28, 2020
d89cd9d
I think?
vcsjones Mar 29, 2020
e6987d2
Actually use the property.
vcsjones Mar 29, 2020
059e087
Implement for CNG.
vcsjones Mar 29, 2020
c8328f2
Encode to pooled buffer.
vcsjones Mar 30, 2020
de3ff4a
Fix syntax I will probably be told to change.
vcsjones Mar 30, 2020
9a7f119
Add tests for ImportParameters.
vcsjones Mar 30, 2020
f72e75e
Use blobs instead of PKCS8 for CNG limited private imports.
vcsjones Mar 31, 2020
2700348
Support explicit curves lacking pre-computed public keys.
vcsjones Apr 1, 2020
e485093
Merge remote-tracking branch 'ms/master' into 33744-fix
vcsjones Apr 1, 2020
e4ec783
Bulk up on explicit curve tests.
vcsjones Apr 1, 2020
b071145
Add ECPrivateKey to CNG.
vcsjones Mar 29, 2020
497e233
Merge remote-tracking branch 'ms/master' into 33744-fix
vcsjones Apr 4, 2020
7e71707
CNG limited private explicit curve support on ECParameters.
vcsjones Apr 1, 2020
e47356b
Use a zero value public key in PKCS8 where required
vcsjones Apr 4, 2020
629b43f
Zero out D
vcsjones Apr 4, 2020
457cb2b
Add test for keyUsage.
vcsjones Apr 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .config/CredScanSuppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"/src/libraries/Common/tests/System/Net/Http/PostScenarioTest.cs",
"/src/libraries/Common/tests/System/Net/Prerequisites/Deployment/setup_certificates.ps1",
"/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/ECKeyFileTests.cs",
"/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/ECKeyFileTests.LimitedPrivate.cs",
"/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAKeyFileTests.cs",
"/src/libraries/System.Data.Common/tests/System/Data/Common/DbConnectionStringBuilderTest.cs",
"/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs",
Expand Down
8 changes: 4 additions & 4 deletions ...Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ internal static partial class Crypto
private static extern int EcKeyCreateByKeyParameters(
out SafeEcKeyHandle key,
string oid,
byte[] qx, int qxLength,
byte[] qy, int qyLength,
byte[]? qx, int qxLength,
byte[]? qy, int qyLength,
byte[]? d, int dLength);

internal static SafeEcKeyHandle EcKeyCreateByKeyParameters(
string oid,
byte[] qx, int qxLength,
byte[] qy, int qyLength,
byte[]? qx, int qxLength,
byte[]? qy, int qyLength,
byte[]? d, int dLength)
{
SafeEcKeyHandle key;
Expand Down
101 changes: 98 additions & 3 deletions src/libraries/Common/src/System/Security/Cryptography/CngPkcs8.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,23 @@ internal static unsafe Pkcs8Response ImportPkcs8PrivateKey(ReadOnlySpan<byte> so
}

bytesRead = len;
return ImportPkcs8(source.Slice(0, len));
ReadOnlySpan<byte> pkcs8Source = source.Slice(0, len);

try
{
return ImportPkcs8(pkcs8Source);
}
catch (CryptographicException)
{
AsnWriter? pkcs8ZeroPublicKey = RewritePkcs8ECPrivateKeyWithZeroPublicKey(pkcs8Source);

if (pkcs8ZeroPublicKey == null)
{
throw;
}

return ImportPkcs8(pkcs8ZeroPublicKey.EncodeAsSpan());
}
}

internal static unsafe Pkcs8Response ImportEncryptedPkcs8PrivateKey(
Expand All @@ -147,7 +163,21 @@ internal static unsafe Pkcs8Response ImportEncryptedPkcs8PrivateKey(
}
catch (CryptographicException e)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
AsnWriter? pkcs8ZeroPublicKey = RewritePkcs8ECPrivateKeyWithZeroPublicKey(decryptedSpan);

if (pkcs8ZeroPublicKey == null)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
}

try
{
return ImportPkcs8(pkcs8ZeroPublicKey.EncodeAsSpan());
}
catch (CryptographicException)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
}
}
finally
{
Expand Down Expand Up @@ -199,7 +229,22 @@ internal static unsafe Pkcs8Response ImportEncryptedPkcs8PrivateKey(
}
catch (CryptographicException e)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
AsnWriter? pkcs8ZeroPublicKey = RewritePkcs8ECPrivateKeyWithZeroPublicKey(decryptedSpan);

if (pkcs8ZeroPublicKey == null)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
}

try
{
bytesRead = len;
return ImportPkcs8(pkcs8ZeroPublicKey.EncodeAsSpan());
}
catch (CryptographicException)
{
throw new CryptographicException(SR.Cryptography_Pkcs8_EncryptedReadFailed, e);
}
}
finally
{
Expand Down Expand Up @@ -305,6 +350,56 @@ private static AsnWriter RewriteEncryptedPkcs8PrivateKey(
}
}

// CNG cannot import a PrivateKeyInfo with the following criteria:
// 1. Is a EC key with explicitly encoded parameters
// 2. Is missing the PublicKey from ECPrivateKey.
// CNG can import an explicit EC PrivateKeyInfo if the PublicKey
// is present. CNG will also re-compute the public key from the
// private key if they do not much. To help CNG be able to import
// these keys, we re-write the PKCS8 to contain a zeroed PublicKey.
//
// If the PKCS8 key does not meet the above criteria, null is returned,
// signaling the original exception should be thrown.
private static unsafe AsnWriter? RewritePkcs8ECPrivateKeyWithZeroPublicKey(ReadOnlySpan<byte> source)
{
fixed (byte* ptr = &MemoryMarshal.GetReference(source))
{
using (MemoryManager<byte> manager = new PointerMemoryManager<byte>(ptr, source.Length))
{
PrivateKeyInfoAsn privateKeyInfo = PrivateKeyInfoAsn.Decode(manager.Memory, AsnEncodingRules.BER);
AlgorithmIdentifierAsn privateAlgorithm = privateKeyInfo.PrivateKeyAlgorithm;

if (privateAlgorithm.Algorithm.Value != Oids.EcPublicKey)
bartonjs marked this conversation as resolved.
Show resolved Hide resolved
{
return null;
}

ECPrivateKey privateKey = ECPrivateKey.Decode(privateKeyInfo.PrivateKey, AsnEncodingRules.BER);
EccKeyFormatHelper.FromECPrivateKey(privateKey, privateAlgorithm, out ECParameters ecParameters);

fixed (byte* pD = ecParameters.D)
{
try
{
if (!ecParameters.Curve.IsExplicit || ecParameters.Q.X != null || ecParameters.Q.Y != null)
{
return null;
}

byte[] zero = new byte[ecParameters.D!.Length];
ecParameters.Q.Y = zero;
ecParameters.Q.X = zero;
return EccKeyFormatHelper.WritePkcs8PrivateKey(ecParameters, privateKeyInfo.Attributes);
}
finally
{
Array.Clear(ecParameters.D!, 0, ecParameters.D!.Length);
}
}
}
}
}

private static void FillRandomAsciiString(Span<char> destination)
{
Debug.Assert(destination.Length < 128);
Expand Down
35 changes: 30 additions & 5 deletions src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,25 @@ public override void ImportParameters(ECParameters parameters)
ThrowIfDisposed();

ECCurve curve = parameters.Curve;
bool includePrivateParamerters = (parameters.D != null);
bool includePrivateParameters = parameters.D != null;
bool hasPublicParameters = parameters.Q.X != null && parameters.Q.Y != null;

if (curve.IsPrime)
{
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref parameters, ecdh: true);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParamerters);
if (!hasPublicParameters && includePrivateParameters)
{
byte[] zero = new byte[parameters.D!.Length];
ECParameters ecParamsCopy = parameters;
ecParamsCopy.Q.X = zero;
ecParamsCopy.Q.Y = zero;
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref ecParamsCopy, ecdh: true);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParameters: true);
}
else
{
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref parameters, ecdh: true);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParameters);
}
}
else if (curve.IsNamed)
{
Expand All @@ -35,8 +48,20 @@ public override void ImportParameters(ECParameters parameters)
SR.Format(SR.Cryptography_InvalidCurveOid, curve.Oid.Value));
}

byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref parameters, ecdh: true);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParamerters);
if (!hasPublicParameters && includePrivateParameters)
{
byte[] zero = new byte[parameters.D!.Length];
bartonjs marked this conversation as resolved.
Show resolved Hide resolved
ECParameters ecParamsCopy = parameters;
ecParamsCopy.Q.X = zero;
ecParamsCopy.Q.Y = zero;
byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref ecParamsCopy, ecdh: true);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParameters: true);
}
else
{
byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref parameters, ecdh: true);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParameters);
}
}
else
{
Expand Down
35 changes: 30 additions & 5 deletions src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,46 @@ public override void ImportParameters(ECParameters parameters)
ThrowIfDisposed();

ECCurve curve = parameters.Curve;
bool includePrivateParameters = (parameters.D != null);
bool includePrivateParameters = parameters.D != null;
bool hasPublicParameters = parameters.Q.X != null && parameters.Q.Y != null;

if (curve.IsPrime)
{
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref parameters, ecdh: false);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParameters);
if (!hasPublicParameters && includePrivateParameters)
{
byte[] zero = new byte[parameters.D!.Length];
ECParameters ecParamsCopy = parameters;
ecParamsCopy.Q.X = zero;
ecParamsCopy.Q.Y = zero;
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref ecParamsCopy, ecdh: false);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParameters: true);
}
else
{
byte[] ecExplicitBlob = ECCng.GetPrimeCurveBlob(ref parameters, ecdh: false);
ImportFullKeyBlob(ecExplicitBlob, includePrivateParameters);
}
}
else if (curve.IsNamed)
{
// FriendlyName is required; an attempt was already made to default it in ECCurve
if (string.IsNullOrEmpty(curve.Oid.FriendlyName))
throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_InvalidCurveOid, curve.Oid.Value!.ToString()));

byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref parameters, ecdh: false);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParameters);
if (!hasPublicParameters && includePrivateParameters)
{
byte[] zero = new byte[parameters.D!.Length];
ECParameters ecParamsCopy = parameters;
ecParamsCopy.Q.X = zero;
ecParamsCopy.Q.Y = zero;
byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref ecParamsCopy, ecdh: false);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParameters: true);
}
else
{
byte[] ecNamedCurveBlob = ECCng.GetNamedCurveBlob(ref parameters, ecdh: false);
ImportKeyBlob(ecNamedCurveBlob, curve.Oid.FriendlyName, includePrivateParameters);
}
}
else
{
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ private static SafeEcKeyHandle ImportNamedCurveParameters(ECParameters parameter

SafeEcKeyHandle key = Interop.Crypto.EcKeyCreateByKeyParameters(
oid,
parameters.Q.X!, parameters.Q.X!.Length,
parameters.Q.Y!, parameters.Q.Y!.Length,
parameters.Q.X, parameters.Q.X?.Length ?? 0,
parameters.Q.Y, parameters.Q.Y?.Length ?? 0,
parameters.D, parameters.D == null ? 0 : parameters.D.Length);

return key;
Expand All @@ -126,8 +126,8 @@ private static SafeEcKeyHandle ImportPrimeCurveParameters(ECParameters parameter
Debug.Assert(parameters.Curve.IsPrime);
SafeEcKeyHandle key = Interop.Crypto.EcKeyCreateByExplicitParameters(
parameters.Curve.CurveType,
parameters.Q.X, parameters.Q.X!.Length,
parameters.Q.Y, parameters.Q.Y!.Length,
parameters.Q.X, parameters.Q.X?.Length ?? 0,
parameters.Q.Y, parameters.Q.Y?.Length ?? 0,
parameters.D, parameters.D == null ? 0 : parameters.D.Length,
parameters.Curve.Prime!, parameters.Curve.Prime!.Length,
parameters.Curve.A!, parameters.Curve.A!.Length,
Expand All @@ -146,8 +146,8 @@ private static SafeEcKeyHandle ImportCharacteristic2CurveParameters(ECParameters
Debug.Assert(parameters.Curve.IsCharacteristic2);
SafeEcKeyHandle key = Interop.Crypto.EcKeyCreateByExplicitParameters(
parameters.Curve.CurveType,
parameters.Q.X, parameters.Q.X!.Length,
parameters.Q.Y, parameters.Q.Y!.Length,
parameters.Q.X, parameters.Q.X?.Length ?? 0,
parameters.Q.Y, parameters.Q.Y?.Length ?? 0,
parameters.D, parameters.D == null ? 0 : parameters.D.Length,
parameters.Curve.Polynomial!, parameters.Curve.Polynomial!.Length,
parameters.Curve.A!, parameters.Curve.A!.Length,
Expand Down
Loading