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

Permit unencrypted key exports from CNG #109119

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Permit unencrypted key exports from CNG #109119

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4d22ce
Permit unencrypted key exports from CNG.
vcsjones Oct 22, 2024
4172fdd
Skip explicit curve tests on platforms that do not support explicit c…
vcsjones Oct 22, 2024
669e094
Fix property name to make sense to people other than me
vcsjones Oct 22, 2024
efaab36
Apply fix to ECDH and DSA
vcsjones Oct 22, 2024
a1a5be2
Fix tests
vcsjones Oct 22, 2024
3811dde
Code review feedback
vcsjones Oct 22, 2024
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
89 changes: 80 additions & 9 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
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using Internal.NativeCrypto;

namespace System.Security.Cryptography
Expand Down Expand Up @@ -87,10 +88,17 @@ public override void ImportParameters(ECParameters parameters)
/// <returns>The key and explicit curve parameters used by the ECC object.</returns>
public override ECParameters ExportExplicitParameters(bool includePrivateParameters)
{
byte[] blob = ExportFullKeyBlob(includePrivateParameters);
ECParameters ecparams = default;
ECCng.ExportPrimeCurveParameters(ref ecparams, blob, includePrivateParameters);
return ecparams;
if (includePrivateParameters)
{
return ExportPrivateExplicitParameters();
}
else
{
byte[] blob = ExportFullKeyBlob(includePrivateParameters: false);
ECParameters ecparams = default;
ECCng.ExportPrimeCurveParameters(ref ecparams, blob, includePrivateParameters: false);
return ecparams;
}
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand All @@ -105,18 +113,38 @@ public override ECParameters ExportParameters(bool includePrivateParameters)
{
ECParameters ecparams = default;

const string TemporaryExportPassword = "DotnetExportPhrase";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to have a problem with CredScan after we merge this? We should try pushing this change to the internal mirror and make sure it doesn't trip CredScan before we merge this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was back-channeling with @GrabYourPitchforks on this - we (well, Levi) is going to run it - especially since this might be a thing we try to fix for 9.0.

string? curveName = GetCurveName(out string? oidValue);

if (string.IsNullOrEmpty(curveName))
{
byte[] fullKeyBlob = ExportFullKeyBlob(includePrivateParameters);
ECCng.ExportPrimeCurveParameters(ref ecparams, fullKeyBlob, includePrivateParameters);
if (includePrivateParameters)
{
ecparams = ExportPrivateExplicitParameters();
}
else
{
byte[] fullKeyBlob = ExportFullKeyBlob(includePrivateParameters: false);
ECCng.ExportPrimeCurveParameters(ref ecparams, fullKeyBlob, includePrivateParameters: false);
}
}
else
{
byte[] keyBlob = ExportKeyBlob(includePrivateParameters);
ECCng.ExportNamedCurveParameters(ref ecparams, keyBlob, includePrivateParameters);
ecparams.Curve = ECCurve.CreateFromOid(new Oid(oidValue, curveName));
if (includePrivateParameters && PlaintextOnlyExport)
{
byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
EccKeyFormatHelper.ReadEncryptedPkcs8(
exported,
TemporaryExportPassword,
out _,
out ecparams);
}
else
{
byte[] keyBlob = ExportKeyBlob(includePrivateParameters);
ECCng.ExportNamedCurveParameters(ref ecparams, keyBlob, includePrivateParameters);
ecparams.Curve = ECCurve.CreateFromOid(new Oid(oidValue, curveName));
}
}

return ecparams;
Expand Down Expand Up @@ -264,5 +292,48 @@ public override bool TryExportEncryptedPkcs8PrivateKey(
destination,
out bytesWritten);
}

private bool PlaintextOnlyExport
vcsjones marked this conversation as resolved.
Show resolved Hide resolved
{
get
{
const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
return (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
}
}

private ECParameters ExportPrivateExplicitParameters()
{
ECParameters ecparams = default;

if (PlaintextOnlyExport)
{
// We can't ask CNG for the explicit parameters when performing a PKCS#8 export. Instead,
// we ask CNG for the explicit parameters for the public part only, since the parameters are public.
// Then we ask CNG by encrypted PKCS#8 for the private parameters (D) and combine the explicit public
// key along with the private key.
const string TemporaryExportPassword = "DotnetExportPhrase";
byte[] publicKeyBlob = ExportFullKeyBlob(includePrivateParameters: false);
ECCng.ExportPrimeCurveParameters(ref ecparams, publicKeyBlob, includePrivateParameters: false);

byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
EccKeyFormatHelper.ReadEncryptedPkcs8(
exported,
TemporaryExportPassword,
out _,
out ECParameters localParameters);

Debug.Assert(ecparams.Q.X.AsSpan().SequenceEqual(localParameters.Q.X));
Debug.Assert(ecparams.Q.Y.AsSpan().SequenceEqual(localParameters.Q.Y));
ecparams.D = localParameters.D;
}
else
{
byte[] blob = ExportFullKeyBlob(includePrivateParameters: true);
ECCng.ExportPrimeCurveParameters(ref ecparams, blob, includePrivateParameters: true);
}

return ecparams;
}
}
}
21 changes: 21 additions & 0 deletions src/libraries/Common/src/System/Security/Cryptography/RSACng.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,31 @@ public override bool TryExportEncryptedPkcs8PrivateKey(
/// </summary>
public override RSAParameters ExportParameters(bool includePrivateParameters)
{
if (includePrivateParameters && PlaintextOnlyExport)
{
const string TemporaryExportPassword = "DotnetExportPhrase";
byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
RSAKeyFormatHelper.ReadEncryptedPkcs8(
exported,
TemporaryExportPassword,
out _,
out RSAParameters rsaParameters);
return rsaParameters;
}

byte[] rsaBlob = ExportKeyBlob(includePrivateParameters);
RSAParameters rsaParams = default;
rsaParams.FromBCryptBlob(rsaBlob, includePrivateParameters);
return rsaParams;
}

private bool PlaintextOnlyExport
{
get
{
const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
return (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
}
}
}
}
12 changes: 12 additions & 0 deletions ...ries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ private byte[] ExportKeyBlob(bool includePrivateParameters)

public override bool TryExportPkcs8PrivateKey(Span<byte> destination, out int bytesWritten)
{
if (PlaintextOnlyExport)
{
const string TemporaryExportPassword = "DotnetExportPhrase";
byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
RSAKeyFormatHelper.ReadEncryptedPkcs8(
exported,
TemporaryExportPassword,
out _,
out RSAParameters rsaParameters);
return RSAKeyFormatHelper.WritePkcs8PrivateKey(rsaParameters).TryEncode(destination, out bytesWritten);
}

return Key.TryExportKeyBlob(
Interop.NCrypt.NCRYPT_PKCS8_PRIVATE_KEY_BLOB,
destination,
Expand Down
132 changes: 132 additions & 0 deletions src/libraries/System.Security.Cryptography/tests/X509Certificates/ExportTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Linq;
using System.Security.Cryptography.EcDsa.Tests;
using System.Security.Cryptography.Pkcs;
using Xunit;

namespace System.Security.Cryptography.X509Certificates.Tests
Expand Down Expand Up @@ -348,5 +350,135 @@ public static void ExportCertificatePem()
Assert.Equal(TestData.CertRfc7468Wrapped, pem);
}
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "The PKCS#12 Exportable flag is not supported on iOS/MacCatalyst/tvOS")]
public static void RSA_Export_DefaultKeyStorePermitsUnencryptedExports_ExportParameters()
{
(byte[] pkcs12, RSA rsa) = CreateSimplePkcs12<RSA>();

using (rsa)
{
using X509Certificate2 cert = new X509Certificate2(pkcs12, "", X509KeyStorageFlags.Exportable);
using RSA key = cert.GetRSAPrivateKey();
RSAParameters expected = rsa.ExportParameters(true);
RSAParameters actual = key.ExportParameters(true);

Assert.Equal(expected.Modulus, actual.Modulus);
Assert.Equal(expected.D, actual.D);
}
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "The PKCS#12 Exportable flag is not supported on iOS/MacCatalyst/tvOS")]
public static void RSA_Export_DefaultKeyStorePermitsUnencryptedExports_Pkcs8PrivateKey()
{
(byte[] pkcs12, RSA rsa) = CreateSimplePkcs12<RSA>();

using (rsa)
{
using X509Certificate2 cert = new X509Certificate2(pkcs12, "", X509KeyStorageFlags.Exportable);
using RSA key = cert.GetRSAPrivateKey();
byte[] exported = rsa.ExportPkcs8PrivateKey();

using RSA imported = RSA.Create();
imported.ImportPkcs8PrivateKey(exported, out _);
RSAParameters actual = imported.ExportParameters(true);
RSAParameters expected = rsa.ExportParameters(true);

Assert.Equal(expected.Modulus, actual.Modulus);
Assert.Equal(expected.D, actual.D);
}
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "The PKCS#12 Exportable flag is not supported on iOS/MacCatalyst/tvOS")]
public static void ECDsa_Export_DefaultKeyStorePermitsUnencryptedExports_Pkcs8PrivateKey()
{
(byte[] pkcs12, ECDsa ecdsa) = CreateSimplePkcs12<ECDsa>();

using (ecdsa)
{
using X509Certificate2 cert = new X509Certificate2(pkcs12, "", X509KeyStorageFlags.Exportable);
using ECDsa key = cert.GetECDsaPrivateKey();
byte[] exported = ecdsa.ExportPkcs8PrivateKey();

using ECDsa imported = ECDsa.Create();
imported.ImportPkcs8PrivateKey(exported, out _);
ECParameters actual = imported.ExportParameters(true);
ECParameters expected = ecdsa.ExportParameters(true);

Assert.Equal(expected.D, actual.D);
Assert.Equal(expected.Q.X, actual.Q.X);
Assert.Equal(expected.Q.Y, actual.Q.Y);
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "The PKCS#12 Exportable flag is not supported on iOS/MacCatalyst/tvOS")]
public static void ECDsa_Export_DefaultKeyStorePermitsUnencryptedExports_ExportParameters(bool explicitParameters)
{
if (explicitParameters && !ECDsaFactory.ExplicitCurvesSupported)
{
return;
}

(byte[] pkcs12, ECDsa ecdsa) = CreateSimplePkcs12<ECDsa>();

using (ecdsa)
{
using X509Certificate2 cert = new X509Certificate2(pkcs12, "", X509KeyStorageFlags.Exportable);
using ECDsa key = cert.GetECDsaPrivateKey();

ECParameters actual = explicitParameters ? key.ExportExplicitParameters(true) : key.ExportParameters(true);
ECParameters expected = explicitParameters ? ecdsa.ExportExplicitParameters(true) : ecdsa.ExportParameters(true);

Assert.Equal(expected.D, actual.D);
Assert.Equal(expected.Q.X, actual.Q.X);
Assert.Equal(expected.Q.Y, actual.Q.Y);
}
}

private static (byte[] Pkcs12, TKey key) CreateSimplePkcs12<TKey>() where TKey : AsymmetricAlgorithm
{
CertificateRequest req;
TKey key;

if (typeof(TKey) == typeof(ECDsa))
{
ECDsa ecKey = ECDsa.Create(ECCurve.NamedCurves.nistP256);
req = new("CN=simple", ecKey, HashAlgorithmName.SHA256);
key = (TKey)(object)ecKey;
}
else if (typeof(TKey) == typeof(RSA))
{
RSA rsaKey = RSA.Create(2048);
req = new("CN=simple", rsaKey, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
key = (TKey)(object)rsaKey;
}
else
{
throw new InvalidOperationException();
}

using X509Certificate2 cert = req.CreateSelfSigned(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddDays(30));
Pkcs9LocalKeyId keyId = new([1]);
PbeParameters pbe = new(PbeEncryptionAlgorithm.TripleDes3KeyPkcs12, HashAlgorithmName.SHA1, 1);

Pkcs12Builder builder = new();
Pkcs12SafeContents certContainer = new();
Pkcs12SafeContents keyContainer = new();
Pkcs12SafeBag certBag = certContainer.AddCertificate(cert);
Pkcs12SafeBag keyBag = keyContainer.AddShroudedKey(key, "", pbe);
certBag.Attributes.Add(keyId);
keyBag.Attributes.Add(keyId);
builder.AddSafeContentsEncrypted(certContainer, "", pbe);
builder.AddSafeContentsUnencrypted(keyContainer);

builder.SealWithMac("", pbe.HashAlgorithm, pbe.IterationCount);
return (builder.Encode(), key);
}
}
}
Loading