From b54070c13759763e7ed4251f31545ad07482bdcd Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Mon, 26 Aug 2024 17:29:22 -0700 Subject: [PATCH 1/2] New cert loader should load into CNG by default When no provider attribute is present on a key, Windows loads the key into the CAPI Base provider unless PKCS12_PREFER_CNG_KSP is set. So, set that flag. On .NET Framework (or .NET Standard running on .NET Framework) we don't have the power to set that flag (without completely redefining how the PFX load loads), so inject a synthetic attribute to force keys into the CNG KSP when PreserveStorageProvider isn't set. Technically these two approaches differ when the incoming PFX has no name and PreserveStorageProvider is set (CoreFX: CNG, NetFX: CAPI Base), but that's unlikely, and consistent with .NET Framework imports. --- .../X509CertificateLoader.Pkcs12.cs | 30 +++++++++ .../Cryptography/X509Certificates/TestData.cs | 53 ++++++++++++++++ ...9CertificateLoaderPkcs12CollectionTests.cs | 22 +++++++ .../X509CertificateLoaderPkcs12Tests.cs | 62 +++++++++++++++++++ .../Microsoft.Bcl.Cryptography.Tests.csproj | 8 +-- .../StorePal.Windows.Import.cs | 20 ------ .../X509CertificateLoader.Windows.cs | 6 +- 7 files changed, 176 insertions(+), 25 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs b/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs index 0a216a3a191058..1e6b5974c6e667 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs @@ -22,6 +22,15 @@ public static partial class X509CertificateLoader private const int NTE_FAIL = unchecked((int)0x80090020); #endif +#pragma warning disable CA1805 + private static readonly AttributeAsn? s_syntheticKspAttribute = +#if NET + null; +#else + BuildSyntheticKspAttribute(); +#endif +#pragma warning restore CA1805 + static partial void LoadPkcs12NoLimits( ReadOnlyMemory data, ReadOnlySpan password, @@ -366,6 +375,13 @@ private static void ProcessSafeContents( }); } + if (!loaderLimits.PreserveStorageProvider && s_syntheticKspAttribute.HasValue) + { + int newCount = (bag.BagAttributes?.Length).GetValueOrDefault(0) + 1; + Array.Resize(ref bag.BagAttributes, newCount); + bag.BagAttributes[newCount - 1] = s_syntheticKspAttribute.GetValueOrDefault(); + } + bagState.AddKey(bag); } } @@ -552,6 +568,20 @@ static int GetRawKdfCount(in AlgorithmIdentifierAsn algorithmIdentifier) } } +#if !NET + private static AttributeAsn? BuildSyntheticKspAttribute() + { + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + writer.WriteCharacterString(UniversalTagNumber.BMPString, "Microsoft Software Key Storage Provider"); + + return new AttributeAsn + { + AttrType = Oids.MsPkcs12KeyProviderName, + AttrValues = new[] { new ReadOnlyMemory(writer.Encode()) } + }; + } +#endif + private readonly partial struct Pkcs12Return { internal partial bool HasValue(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs index 4c1025a7c1c028..610d623ba9f0d0 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs @@ -4630,5 +4630,58 @@ internal static DSAParameters GetDSA1024Params() "007600690064006500721E4E004D006900630072006F0073006F006600740020" + "0053006F0066007400770061007200650020004B00650079002000530074006F" + "0072006100670065002000500072006F00760069006400650072").HexToByteArray(); + + // Uses Placeholder + internal static readonly byte[] SChannelPfx = ( + "3082062A020103308205E606092A864886F70D010701A08205D7048205D33082" + + "05CF3082038806092A864886F70D010701A08203790482037530820371308203" + + "6D060B2A864886F70D010C0A0102A08202B6308202B2301C060A2A864886F70D" + + "010C0103300E040823A7915B5E0EECA9020207D0048202905D79973EDC12FE3C" + + "839C839008F3E79933BAA119C8015AF35C0DA932704D0C45990F2A60FFDFD26A" + + "4CC138A3A04673A79CE067AD6D1376608B1A0E61B5F9284A9D9D229ABB17EEFC" + + "870EB8CD61E6C054FA0AE0202B0FF04452BF3487B38FE256F406CFCA94547EC1" + + "BD44DA25A857E90990EAB58EAAD26322FDCD8810E4019B81670AD455FF905675" + + "DABDAC3331AA662AB08D1DCDE0B56CB0F3B8D53F5ABDB613772174B3959EBE75" + + "EF085404D60DC6161E576B641E5BFB60400C462BA5F9F69CDD2F4F48BA5A3C64" + + "509FCFF53EF2A7C5AA471762C9BCE99B2AD8C0E415795A816BED896B46C66FE8" + + "E737829614E0FCF2E3BDC68D24710DDC86FEA5329F8355EF1A51330981303DE8" + + "38F4CD00D3187CE52F03ACB9BD5A62F98FA1395DA14E3D5FA50F8B466488C0A0" + + "4780074FD330CA3539067A8A194CC63C2D0D35B3A61A6F8EC711907DB0D8E3D0" + + "6912F8202746264C5A9C0A9CBD7AACB1176519D901100C299126E0B30D869C14" + + "B68AA04E8EF3B144447976581BDD63E83DCDDBC8C5C185E2EA598CCAFC137BB0" + + "F46D053F900D46381915322BFF04F6EADE31F14D6781FD98764613BB679ECE45" + + "01821EBA6C0AF5E603151903B2A7F19BCBF623401E14C3456DA20128BD53DE70" + + "1422EFF06FB0DA901D68D55389BE369D76F65B5D4AA112B6636C12F4A7806BE1" + + "A8DAABCD827D65AEE41B9F2AB9E978E936309F54CC77A5E693161E84D4031DB4" + + "26440E61F01464CBE564C97BD0F6E23D4E803C9CBB65A79BAEECED26FD03FA41" + + "4EB2DEE34044DE434D18CE643C2AA1A12A7336522020498642BBC2F2B8132E84" + + "87227D130B37BA4376D57EF4CF57ADC56F9AB38F60235DAC7C7EC7ED1A537C25" + + "E3BDF6E5D1B647817CA8D730E4FC0A6013CB4FF8920F76CBD287A50D40818638" + + "D7E44168E65E26513181A3301306092A864886F70D0109153106040401000000" + + "302106092A864886F70D01091431141E12004E0061006D006500640020004B00" + + "650079306906092B0601040182371101315C1E5A004D006900630072006F0073" + + "006F00660074002000520053004100200053004300680061006E006E0065006C" + + "002000430072007900700074006F006700720061007000680069006300200050" + + "0072006F007600690064006500723082023F06092A864886F70D010706A08202" + + "303082022C0201003082022506092A864886F70D010701301C060A2A864886F7" + + "0D010C0103300E0408543FA90B580A66D0020207D0808201F8DE9CCF9441B460" + + "478D35A09462569A805A1067DA2BA99BB5E7AC02FCE133BD4DD5246861D614DD" + + "903F02D1D342CC5CB3A939D5DABAB1157007B30BB9C9099F29A62FA812EA5DE8" + + "460B3312D8490EE1DAA69E7E7946488B28E33CDBF5378DD288427ED276FAC685" + + "12E3CC1D4D5F6BEB297EF639A331970B5D33C723E7F66C64FEF8907DB8CDB3C4" + + "BE64B5DE333E0D900FC2F5AF6E782114F1A6704D58698C4EF9C796F4A054D985" + + "5570D3E9B268A321E7A1F655B77EEB45C198EB2A7B6D11A30E816ADB8CF5FCDE" + + "29ADDA29CA15E807862E607DB8B0E4A67A2DC4C167EBB0EE4ECAB3CB3F844CC5" + + "528B569128F27F6ECE1495E7ACFC34FC114A8906A872BAF40CDB8425431FD985" + + "DC0BAC262D7E8FB4F71DF5151E242C4C160252FC56DBAF16EF1C082A2E1C32BB" + + "992E3A12F21D86259A37FF6528D1FEB98DC64E79D0901F9873372D0C0659C917" + + "A2F1CFA7176EEE82292A58EEFAFAC104FA61AC124114C4067EB1211C9590714C" + + "A0F67B315E597AFD1BE546C61E58490C093842A926BB1CF3C4D27395C520A707" + + "294234316A3E1837E6521C2565845E244345790FD53DAAD00673596D66438CCD" + + "82E5EA088AC6D2928DACFD3BBC7B40FC55BBD502B6410B575AADE4D50F6DBDB7" + + "94A24BAAACF49C2CCF025D454A1B7541B159CBAC7F12D184B7ECA8F4A7B72805" + + "EE10774B5501BDEBDE4A61D0A3B22C9B0F303B301F300706052B0E03021A0414" + + "95BAC18221F16265AFDD6688C9FA59BE80616F830414C3365AE07D90C0CD656F" + + "15661D39D56F307AB983020207D0").HexToByteArray(); } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs index 13fa4bb81971fc..b3db6721878199 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs @@ -786,6 +786,28 @@ public void LoadWithDuplicateAttributes(bool allowDuplicates) } } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void LoadWithLegacyProvider(bool preserveStorageProvider, bool ephemeralIfPossible) + { + Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits { PreserveStorageProvider = preserveStorageProvider }; + X509KeyStorageFlags flags = ephemeralIfPossible ? EphemeralIfPossible : X509KeyStorageFlags.DefaultKeySet; + bool expectLegacy = ((int)flags & 0x20) == 0 && preserveStorageProvider; + + X509Certificate2Collection coll = LoadPfxNoFile(TestData.SChannelPfx, TestData.PlaceholderPw, flags, limits); + + using (new CollectionDisposer(coll)) + { + foreach (X509Certificate2 cert in coll) + { + X509CertificateLoaderPkcs12Tests.VerifySChannelProvider(cert, expectLegacy); + } + } + } + private sealed class CollectionDisposer : IDisposable { private readonly X509Certificate2Collection _coll; diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs index 086721eef28b3f..44bb41f4ed08e8 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs @@ -776,6 +776,68 @@ public void LoadWithDuplicateAttributes(bool allowDuplicates) } } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void LoadWithLegacyProvider(bool preserveStorageProvider, bool ephemeralIfPossible) + { + Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits { PreserveStorageProvider = preserveStorageProvider, }; + X509KeyStorageFlags flags = ephemeralIfPossible ? EphemeralIfPossible : X509KeyStorageFlags.DefaultKeySet; + bool expectLegacy = ((int)flags & 0x20) == 0 && preserveStorageProvider; + + using (X509Certificate2 cert = LoadPfxNoFile(TestData.SChannelPfx, TestData.PlaceholderPw, flags, limits)) + { + VerifySChannelProvider(cert, expectLegacy); + } + } + + internal static void VerifySChannelProvider(X509Certificate2 cert, bool expectLegacy) + { + const string SChannelProviderName = "Microsoft RSA SChannel Cryptographic Provider"; + + Assert.True(cert.HasPrivateKey, "cert.HasPrivateKey"); + + using (RSA privateKey = cert.GetRSAPrivateKey()) + { + Assert.NotNull(privateKey); + + if (PlatformDetection.IsWindows) + { + string expectedProvider = expectLegacy ? + SChannelProviderName : + "Microsoft Software Key Storage Provider"; + + RSACng cng = Assert.IsType(privateKey); + Assert.Equal(expectedProvider, cng.Key.Provider.Provider); + } + + if (PlatformDetection.IsNetFramework) + { +#pragma warning disable SYSLIB0028 + if (expectLegacy) + { + AsymmetricAlgorithm otherPrivateKeyInstance = cert.PrivateKey; + + RSACryptoServiceProvider csp = + Assert.IsType(otherPrivateKeyInstance); + + Assert.Equal(SChannelProviderName, csp.CspKeyContainerInfo.ProviderName); + } + else + { + Assert.Throws(() => cert.PrivateKey); + } +#pragma warning restore SYSLIB0028 + } + + // Regardless of the platform, the key should work (partially because of the CNG wrapper of CAPI keys) + // Assert.NoThrow + privateKey.SignData(TestData.SChannelPfx, HashAlgorithmName.SHA256, RSASignaturePadding.Pss); + } + } + #if NET [Theory] [InlineData(false)] diff --git a/src/libraries/Microsoft.Bcl.Cryptography/tests/Microsoft.Bcl.Cryptography.Tests.csproj b/src/libraries/Microsoft.Bcl.Cryptography/tests/Microsoft.Bcl.Cryptography.Tests.csproj index 265d2a512e0e96..be85e50d295cbd 100644 --- a/src/libraries/Microsoft.Bcl.Cryptography/tests/Microsoft.Bcl.Cryptography.Tests.csproj +++ b/src/libraries/Microsoft.Bcl.Cryptography/tests/Microsoft.Bcl.Cryptography.Tests.csproj @@ -25,13 +25,13 @@ + Link="CommonTest\System\Security\Cryptography\X509Certificates\TestData.cs" /> + Link="CommonTest\System\Security\Cryptography\X509Certificates\X509CertificateLoaderPkcs12CollectionTests.cs" /> + Link="CommonTest\System\Security\Cryptography\X509Certificates\X509CertificateLoaderPkcs12Tests.cs" /> + Link="CommonTest\System\Security\Cryptography\X509Certificates\X509CertificateLoaderTests.cs" /> diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs index 59155e2b76a2e8..664f308027f58c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs @@ -212,26 +212,6 @@ internal static partial IStorePal FromSystemStore(string storeName, StoreLocatio return new StorePal(certStore); } - // this method maps a X509KeyStorageFlags enum to a combination of crypto API flags - private static Interop.Crypt32.PfxCertStoreFlags MapKeyStorageFlags(X509KeyStorageFlags keyStorageFlags) - { - Interop.Crypt32.PfxCertStoreFlags dwFlags = 0; - if ((keyStorageFlags & X509KeyStorageFlags.UserKeySet) == X509KeyStorageFlags.UserKeySet) - dwFlags |= Interop.Crypt32.PfxCertStoreFlags.CRYPT_USER_KEYSET; - else if ((keyStorageFlags & X509KeyStorageFlags.MachineKeySet) == X509KeyStorageFlags.MachineKeySet) - dwFlags |= Interop.Crypt32.PfxCertStoreFlags.CRYPT_MACHINE_KEYSET; - - if ((keyStorageFlags & X509KeyStorageFlags.Exportable) == X509KeyStorageFlags.Exportable) - dwFlags |= Interop.Crypt32.PfxCertStoreFlags.CRYPT_EXPORTABLE; - if ((keyStorageFlags & X509KeyStorageFlags.UserProtected) == X509KeyStorageFlags.UserProtected) - dwFlags |= Interop.Crypt32.PfxCertStoreFlags.CRYPT_USER_PROTECTED; - - if ((keyStorageFlags & X509KeyStorageFlags.EphemeralKeySet) == X509KeyStorageFlags.EphemeralKeySet) - dwFlags |= Interop.Crypt32.PfxCertStoreFlags.PKCS12_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_ALWAYS_CNG_KSP; - - return dwFlags; - } - // this method maps X509Store OpenFlags to a combination of crypto API flags private static Interop.Crypt32.CertStoreFlags MapX509StoreFlags(StoreLocation storeLocation, OpenFlags flags) { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Windows.cs index 346e8062d34561..2e43dc78b56700 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Windows.cs @@ -281,7 +281,8 @@ private static Interop.Crypt32.PfxCertStoreFlags MapKeyStorageFlags(X509KeyStora { Debug.Assert((keyStorageFlags & KeyStorageFlagsAll) == keyStorageFlags); - Interop.Crypt32.PfxCertStoreFlags pfxCertStoreFlags = 0; + Interop.Crypt32.PfxCertStoreFlags pfxCertStoreFlags = Interop.Crypt32.PfxCertStoreFlags.PKCS12_PREFER_CNG_KSP; + if ((keyStorageFlags & X509KeyStorageFlags.UserKeySet) == X509KeyStorageFlags.UserKeySet) pfxCertStoreFlags |= Interop.Crypt32.PfxCertStoreFlags.CRYPT_USER_KEYSET; else if ((keyStorageFlags & X509KeyStorageFlags.MachineKeySet) == X509KeyStorageFlags.MachineKeySet) @@ -297,7 +298,10 @@ private static Interop.Crypt32.PfxCertStoreFlags MapKeyStorageFlags(X509KeyStora // difficult to do SHA-2 RSA signatures with, simplifies the story for UWP, and reduces the // complexity of pointer interpretation. if ((keyStorageFlags & X509KeyStorageFlags.EphemeralKeySet) == X509KeyStorageFlags.EphemeralKeySet) + { + pfxCertStoreFlags &= ~Interop.Crypt32.PfxCertStoreFlags.PKCS12_PREFER_CNG_KSP; pfxCertStoreFlags |= Interop.Crypt32.PfxCertStoreFlags.PKCS12_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_ALWAYS_CNG_KSP; + } // In .NET Framework loading a PFX then adding the key to the Windows Certificate Store would // enable a native application compiled against CAPI to find that private key and interoperate with it. From d835983e6cf1807a08c02ee7f18198f56623fba1 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Tue, 27 Aug 2024 09:08:59 -0700 Subject: [PATCH 2/2] Name the 0x20/EphemeralKeySet const --- .../X509CertificateLoaderPkcs12CollectionTests.cs | 5 ++++- .../X509Certificates/X509CertificateLoaderPkcs12Tests.cs | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs index b3db6721878199..da5a2d6a7e929a 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs @@ -795,7 +795,10 @@ public void LoadWithLegacyProvider(bool preserveStorageProvider, bool ephemeralI { Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits { PreserveStorageProvider = preserveStorageProvider }; X509KeyStorageFlags flags = ephemeralIfPossible ? EphemeralIfPossible : X509KeyStorageFlags.DefaultKeySet; - bool expectLegacy = ((int)flags & 0x20) == 0 && preserveStorageProvider; + + // EphemeralKeySet is not available by name in the netfx build. + const X509KeyStorageFlags EphemeralKeySet = (X509KeyStorageFlags)0x20; + bool expectLegacy = (flags & EphemeralKeySet) == 0 && preserveStorageProvider; X509Certificate2Collection coll = LoadPfxNoFile(TestData.SChannelPfx, TestData.PlaceholderPw, flags, limits); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs index 44bb41f4ed08e8..b2005ab6052ac0 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs @@ -179,7 +179,7 @@ public abstract partial class X509CertificateLoaderPkcs12Tests #if NETFRAMEWORK X509KeyStorageFlags.DefaultKeySet; #else - PlatformDetection.UsesAppleCrypto ? + PlatformDetection.UsesAppleCrypto ? X509KeyStorageFlags.DefaultKeySet : X509KeyStorageFlags.EphemeralKeySet; #endif @@ -785,7 +785,10 @@ public void LoadWithLegacyProvider(bool preserveStorageProvider, bool ephemeralI { Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits { PreserveStorageProvider = preserveStorageProvider, }; X509KeyStorageFlags flags = ephemeralIfPossible ? EphemeralIfPossible : X509KeyStorageFlags.DefaultKeySet; - bool expectLegacy = ((int)flags & 0x20) == 0 && preserveStorageProvider; + + // EphemeralKeySet is not available by name in the netfx build. + const X509KeyStorageFlags EphemeralKeySet = (X509KeyStorageFlags)0x20; + bool expectLegacy = (flags & EphemeralKeySet) == 0 && preserveStorageProvider; using (X509Certificate2 cert = LoadPfxNoFile(TestData.SChannelPfx, TestData.PlaceholderPw, flags, limits)) {