diff --git a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs index 033c9c9db0d9f8..740d464b96e060 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs @@ -52,6 +52,7 @@ internal static partial class Oids internal const string CertificateAuthorityIssuers = "1.3.6.1.5.5.7.48.2"; internal const string Pkcs9ExtensionRequest = "1.2.840.113549.1.9.14"; internal const string MsPkcs12KeyProviderName = "1.3.6.1.4.1.311.17.1"; + internal const string MsPkcs12MachineKeySet = "1.3.6.1.4.1.311.17.2"; // Key wrap algorithms internal const string CmsRc2Wrap = "1.2.840.113549.1.9.16.3.7"; diff --git a/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/Pkcs12LoaderLimits.cs b/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/Pkcs12LoaderLimits.cs index 3200771cee0adb..f19859c54b0352 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/Pkcs12LoaderLimits.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/X509Certificates/Pkcs12LoaderLimits.cs @@ -22,6 +22,7 @@ public sealed class Pkcs12LoaderLimits private bool _preserveUnknownAttributes; private bool _ignorePrivateKeys; private bool _ignoreEncryptedAuthSafes; + private bool _allowDuplicateAttributes; /// /// Gets a shared reference to the default loader limits. @@ -72,6 +73,7 @@ public sealed class Pkcs12LoaderLimits PreserveKeyName = true, PreserveCertificateAlias = true, PreserveUnknownAttributes = true, + AllowDuplicateAttributes = true, }); /// @@ -117,6 +119,7 @@ public Pkcs12LoaderLimits(Pkcs12LoaderLimits copyFrom) _preserveUnknownAttributes = copyFrom._preserveUnknownAttributes; _ignorePrivateKeys = copyFrom._ignorePrivateKeys; _ignoreEncryptedAuthSafes = copyFrom._ignoreEncryptedAuthSafes; + _allowDuplicateAttributes = copyFrom._allowDuplicateAttributes; } /// @@ -366,6 +369,24 @@ public bool IgnoreEncryptedAuthSafes } } + /// + /// Gets or sets a value indicating whether duplicate attributes are permitted. + /// + /// + /// to permit duplicate attributes; + /// to fail loading when duplicate attributes are found. + /// The default is . + /// + internal bool AllowDuplicateAttributes + { + get => _allowDuplicateAttributes; + set + { + CheckReadOnly(); + _allowDuplicateAttributes = value; + } + } + private static void CheckNonNegative( int? value, [CallerArgumentExpression(nameof(value))] string? paramName = null) 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 196af51bc4d91f..0a216a3a191058 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; +using System.Collections.Generic; using System.Diagnostics; using System.Formats.Asn1; using System.Security.Cryptography.Asn1; @@ -268,12 +269,19 @@ private static void ProcessSafeContents( AsnValueReader reader = outer.ReadSequence(); outer.ThrowIfNotEmpty(); + HashSet duplicateAttributeCheck = new(); + while (reader.HasData) { SafeBagAsn.Decode(ref reader, contentData, out SafeBagAsn bag); if (bag.BagId == Oids.Pkcs12CertBag) { + if (bag.BagAttributes is not null && !loaderLimits.AllowDuplicateAttributes) + { + RejectDuplicateAttributes(bag.BagAttributes, duplicateAttributeCheck); + } + CertBagAsn certBag = CertBagAsn.Decode(bag.BagValue, AsnEncodingRules.BER); if (certBag.CertId == Oids.Pkcs12X509CertBagType) @@ -302,6 +310,11 @@ private static void ProcessSafeContents( } else if (bag.BagId is Oids.Pkcs12KeyBag or Oids.Pkcs12ShroudedKeyBag) { + if (bag.BagAttributes is not null && !loaderLimits.AllowDuplicateAttributes) + { + RejectDuplicateAttributes(bag.BagAttributes, duplicateAttributeCheck); + } + if (loaderLimits.IgnorePrivateKeys) { continue; @@ -344,6 +357,9 @@ private static void ProcessSafeContents( attrType switch { Oids.LocalKeyId => true, + // MsPkcs12MachineKeySet can be forced off with the UserKeySet flag, or on with MachineKeySet, + // so always preserve it. + Oids.MsPkcs12MachineKeySet => true, Oids.Pkcs9FriendlyName => limits.PreserveKeyName, Oids.MsPkcs12KeyProviderName => limits.PreserveStorageProvider, _ => limits.PreserveUnknownAttributes, @@ -355,6 +371,21 @@ private static void ProcessSafeContents( } } + private static void RejectDuplicateAttributes(AttributeAsn[] bagAttributes, HashSet duplicateAttributeCheck) + { + duplicateAttributeCheck.Clear(); + + foreach (AttributeAsn attrSet in bagAttributes) + { + // Use >1 instead of =1 to account for MsPkcs12MachineKeySet, which is a named set with no values. + // An empty attribute set can't be followed by the same empty set, or a non-empty set. + if (!duplicateAttributeCheck.Add(attrSet.AttrType) || attrSet.AttrValues.Length > 1) + { + throw new Pkcs12LoadLimitExceededException(nameof(Pkcs12LoaderLimits.AllowDuplicateAttributes)); + } + } + } + private static void FilterAttributes( Pkcs12LoaderLimits loaderLimits, ref SafeBagAsn bag, @@ -362,10 +393,13 @@ private static void FilterAttributes( { if (bag.BagAttributes is not null) { - // Should this dedup/fail-on-dup? int attrIdx = -1; - for (int i = bag.BagAttributes.Length - 1; i > attrIdx; i--) + // Filter the attributes, per the loader limits. + // Because duplicates might be permitted by the options, this filter + // needs to be order preserving. + + for (int i = 0; i < bag.BagAttributes.Length; i++) { string attrType = bag.BagAttributes[i].AttrType; @@ -373,30 +407,28 @@ private static void FilterAttributes( { attrIdx++; - if (i > attrIdx) + if (attrIdx != i) { AttributeAsn attr = bag.BagAttributes[i]; +#if DEBUG bag.BagAttributes[i] = bag.BagAttributes[attrIdx]; +#endif bag.BagAttributes[attrIdx] = attr; - - // After swapping, back up one position to check if the attribute - // swapped into this position should also be preserved. - i++; } } } - attrIdx++; + int attrLen = attrIdx + 1; - if (attrIdx < bag.BagAttributes.Length) + if (attrLen < bag.BagAttributes.Length) { - if (attrIdx == 0) + if (attrLen == 0) { bag.BagAttributes = null; } else { - Array.Resize(ref bag.BagAttributes, attrIdx); + Array.Resize(ref bag.BagAttributes, attrLen); } } } 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 458f9b04f4cb46..4c1025a7c1c028 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/TestData.cs @@ -4550,5 +4550,85 @@ internal static DSAParameters GetDSA1024Params() "928343622B80009D8C5B0440E74D33A5F7DA48801A4EF4FD65D0F442F26C845F" + "A418D3E0D78FD0285A92A74B433661F516C6955EC40FE46DAB813B6AE940C2DE" + "FE8F8F5E32E6B491C999D598020127").HexToByteArray(); + + // This is a PFX that has a lot of duplicated attributes: + // Integrity: None + // SC[0] Unencrypted + // Key (3DES, "Placeholder") + // MachineKey (empty) + // MachineKey (empty) + // LocalKeyId { 1 } + // LocalKeyId { 1, 2 } + // FriendlyName { keyname000, keyname001 } + // ProviderName Microsoft Enhanced RSA and AES Cryptographic Provider + // ProviderName Microsoft Software Key Storage Provider + // SC[1] Unencrypted + // Cert, CN=Cerificate 1 + // LocalKeyId { 1, 4 } + // FriendlyName { (CAPI CSP), (CNG KSP) } + // + // Note that this test cannot be built by Pkcs12Builder, because that type + // always unifies attribute sets. + internal static readonly byte[] DuplicateAttributesPfx = ( + "308207760201033082076F06092A864886F70D010701A08207600482075C3082" + + "07583082043006092A864886F70D010701A08204210482041D30820419308204" + + "15060B2A864886F70D010C0A0102A08202A6308202A2301C060A2A864886F70D" + + "010C0103300E0408F7A34F4CC26F79890202100004820280BD3E4386BE9699C0" + + "F53C27D4971B17E984BE885F3E1D3B1B75B666EF3F53BED590A70AA071C05057" + + "7CF587B92AF7F84F0D6E79475CA0D46C5F86A6D548ADE9538A955B7033154F2D" + + "A356B1DD930576A0A1D6CC87FF9055BB00CECFF5E61040B709FC19C29046A4AA" + + "BD4B8D1CA8F09B119ED6A8FBDB985E3F22E531D5DB13E292278D73A6C4E05498" + + "79642858CAA162BB21E9A17B4B14341B388665E0F5B90EB1EFB0C4B211B9B49E" + + "F8C7F435F9B2D32A319D43B9133039844E7ED8C0BA663E8681C8EFDF707356DC" + + "A1B78551839300323F046A497DD4C468232993946343764F151AB3EFFFC7FD27" + + "9739CBE00337399AF2341E7842DA1CCDD98B12A7831A4DE9827F8F1CCB5A0F4E" + + "ECB412D208983CA5D801D720B3F1E118C20BB8A1853770435177253EF59A62A2" + + "43E53ABC531F310E245CC1A0626E5456ADC08924F15E1408B2FD30BFDD4A4F32" + + "01B1983DE0F7F42E7EEF2E8EA6D9B0EAB98174A4B4D0410FD04167670FDFC20A" + + "1EFC58AB2A41ABD3EC42D3071F31EA5D0A6B93EC070E1D543F0FC7BB8B88361A" + + "D904E81ADD0C3B0261F1406EACF956F19055FC1C2832F25209DFBD35C8EDD8B4" + + "091B626E8C07D58F8537C519C90E23E94E8E61DCF3862C1DFB63010D2D909037" + + "6CFB21042041B550FE62122E3473B88E479B42153FB17077C4BC1318715BAB99" + + "597226F0C24524FB844CEAA4EC8DD164321DDFB74509FBC4844C205FFC27B067" + + "C9E4A78B8B12F4643F3A4C754E84F244F84D7A075F290C10A3B544264E317BFA" + + "41647EC4F50D6B1B2A691B5F0575B9492484019E88355CADADBC0A30FAEED71E" + + "4392E37BE497900408A85C711BF68B27A84433B0F546DF2FC2CA3FD22C4367BA" + + "BC074313B982E5012B26863FA98148E5DBF43D26423369C13182015A300D0609" + + "2B06010401823711023100300D06092B06010401823711023100301006092A86" + + "4886F70D0109153103040101301306092A864886F70D01091531060401010401" + + "02303906092A864886F70D010914312C1E14006B00650079006E0061006D0065" + + "0030003000301E14006B00650079006E0061006D006500300030003130790609" + + "2B0601040182371101316C1E6A004D006900630072006F0073006F0066007400" + + "200045006E00680061006E006300650064002000520053004100200061006E00" + + "640020004100450053002000430072007900700074006F006700720061007000" + + "6800690063002000500072006F00760069006400650072305D06092B06010401" + + "8237110131501E4E004D006900630072006F0073006F0066007400200053006F" + + "0066007400770061007200650020004B00650079002000530074006F00720061" + + "00670065002000500072006F007600690064006500723082032006092A864886" + + "F70D010701A08203110482030D3082030930820305060B2A864886F70D010C0A" + + "0103A082020F3082020B060A2A864886F70D01091601A08201FB048201F73082" + + "01F33082015CA00302010202080828E93144879296300D06092A864886F70D01" + + "010B0500303C31223020060355040B13194475706C6963617465204174747269" + + "62757465732054657374311630140603550403130D4365727469666963617465" + + "2031301E170D3234303730323231303531395A170D3234303730323231313031" + + "395A303C31223020060355040B13194475706C69636174652041747472696275" + + "7465732054657374311630140603550403130D43657274696669636174652031" + + "30819F300D06092A864886F70D010101050003818D0030818902818100E6EB18" + + "5FCE3E6A5A6F73D260B408FA9DD06DC99D546AE9A1F1F8792FDF818E8A2759FF" + + "D0FCD8A88301117CA992ACEC9AA4E429655AB7A73EE20994155FE97572471D06" + + "2C06295FF4EE218090DF64AAF787BAD7F511DF329F2F685FFC3B819F95F17811" + + "E9C158D97D832208C27214C958D844432481981B03FE8C9E0E8C1A5605020301" + + "0001300D06092A864886F70D01010B0500038181009243F8BCFB5B21BC5BFB45" + + "83DF87F0D1FE7680C62D1F35698F1A25A0D0517F17B977F039985457A85058DC" + + "7375D1ED845EB30B20FF7FC3E8970871F03AEDE3658FF964B1EF1BFD1EEB7CF5" + + "E4A219F1B00DDED1F00BE42F132A2409D0FE78BA131634F5059B06E1B905AE18" + + "6897E1E10716282E0BE25D460AE93483E9BC0329E13181E2301306092A864886" + + "F70D01091531060401010401043081CA06092A864886F70D0109143181BC1E6A" + + "004D006900630072006F0073006F0066007400200045006E00680061006E0063" + + "00650064002000520053004100200061006E0064002000410045005300200043" + + "0072007900700074006F0067007200610070006800690063002000500072006F" + + "007600690064006500721E4E004D006900630072006F0073006F006600740020" + + "0053006F0066007400770061007200650020004B00650079002000530074006F" + + "0072006100670065002000500072006F00760069006400650072").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 84d57f6e7e74e7..13fa4bb81971fc 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12CollectionTests.cs @@ -742,6 +742,50 @@ public void LoadPfx_VerifyIgnoreEncryptedSafes_EmptyIfIgnored(bool ignoreEncrypt } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void LoadWithDuplicateAttributes(bool allowDuplicates) + { + Pkcs12LoaderLimits limits = Pkcs12LoaderLimits.Defaults; + + if (allowDuplicates) + { + limits = Pkcs12LoaderLimits.DangerousNoLimits; + } + + // remove the edit lock + limits = new Pkcs12LoaderLimits(limits) + { + PreserveCertificateAlias = false, + PreserveKeyName = false, + PreserveStorageProvider = false, + PreserveUnknownAttributes = false, + }; + + Func func = + () => LoadPfxNoFile(TestData.DuplicateAttributesPfx, TestData.PlaceholderPw, loaderLimits: limits); + + if (allowDuplicates) + { + X509Certificate2Collection coll = func(); + + using (new CollectionDisposer(coll)) + { + Assert.Equal(1, coll.Count); + X509Certificate2 cert = coll[0]; + + Assert.Equal("Certificate 1", cert.GetNameInfo(X509NameType.SimpleName, false)); + Assert.True(cert.HasPrivateKey, "cert.HasPrivateKey"); + } + } + else + { + Pkcs12LoadLimitExceededException ex = Assert.Throws(() => func()); + Assert.Contains("AllowDuplicateAttributes", ex.Message); + } + } + private sealed class CollectionDisposer : IDisposable { private readonly X509Certificate2Collection _coll; diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.WindowsAttributes.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.WindowsAttributes.cs index 444094c15b0ca8..3e07c4e25f5632 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.WindowsAttributes.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.WindowsAttributes.cs @@ -9,9 +9,11 @@ namespace System.Security.Cryptography.X509Certificates.Tests public abstract partial class X509CertificateLoaderPkcs12Tests { [Theory] - [InlineData(true)] - [InlineData(false)] - public void VerifyPreserveKeyName(bool preserveName) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void VerifyPreserveKeyName(bool preserveName, bool machineKey) { Pkcs12LoaderLimits loaderLimits = new Pkcs12LoaderLimits { @@ -19,7 +21,7 @@ public void VerifyPreserveKeyName(bool preserveName) }; string keyName = Guid.NewGuid().ToString("D"); - byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: "Non-preserved"); + byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: "Non-preserved", machineKey: machineKey); X509Certificate2 cert = LoadPfxNoFile( pfx, @@ -29,9 +31,8 @@ public void VerifyPreserveKeyName(bool preserveName) using (cert) { using (RSA key = cert.GetRSAPrivateKey()) + using (CngKey cngKey = Assert.IsType(key).Key) { - CngKey cngKey = Assert.IsType(key).Key; - if (preserveName) { Assert.Equal(keyName, cngKey.KeyName); @@ -40,6 +41,9 @@ public void VerifyPreserveKeyName(bool preserveName) { Assert.NotEqual(keyName, cngKey.KeyName); } + + // MachineKey is preserved irrespective of PreserveKeyName + Assert.Equal(machineKey, cngKey.IsMachineKey); } // Alias was not preserved @@ -48,9 +52,11 @@ public void VerifyPreserveKeyName(bool preserveName) } [Theory] - [InlineData(true)] - [InlineData(false)] - public void VerifyPreserveAlias(bool preserveAlias) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void VerifyPreserveAlias(bool preserveAlias, bool machineKey) { Pkcs12LoaderLimits loaderLimits = new Pkcs12LoaderLimits { @@ -59,7 +65,7 @@ public void VerifyPreserveAlias(bool preserveAlias) string keyName = Guid.NewGuid().ToString("D"); string alias = Guid.NewGuid().ToString("D"); - byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: alias); + byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: alias, machineKey: machineKey); X509Certificate2 cert = LoadPfxNoFile( pfx, @@ -78,21 +84,27 @@ public void VerifyPreserveAlias(bool preserveAlias) } using (RSA key = cert.GetRSAPrivateKey()) + using (CngKey cngKey = Assert.IsType(key).Key) { - CngKey cngKey = Assert.IsType(key).Key; - // Key name was not preserved Assert.NotEqual(keyName, cngKey.KeyName); + + // MachineKey is preserved irrespective of PreserveCertificateAlias + Assert.Equal(machineKey, cngKey.IsMachineKey); } } } [Theory] - [InlineData(true, true)] - [InlineData(true, false)] - [InlineData(false, true)] - [InlineData(false, false)] - public void VerifyPreservePreserveProvider(bool preserveProvider, bool preserveName) + [InlineData(true, true, true)] + [InlineData(true, true, false)] + [InlineData(true, false, true)] + [InlineData(true, false, false)] + [InlineData(false, true, true)] + [InlineData(false, true, false)] + [InlineData(false, false, true)] + [InlineData(false, false, false)] + public void VerifyPreserveProvider(bool preserveProvider, bool preserveName, bool machineKey) { // This test forces a key creation with CAPI, and verifies that // PreserveStorageProvider keeps the key in CAPI. Additionally, @@ -105,7 +117,7 @@ public void VerifyPreservePreserveProvider(bool preserveProvider, bool preserveN string keyName = Guid.NewGuid().ToString("D"); string alias = Guid.NewGuid().ToString("D"); - byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: alias, useCapi: true); + byte[] pfx = MakeAttributeTest(keyName: keyName, friendlyName: alias, useCapi: true, machineKey: machineKey); X509Certificate2 cert = LoadPfxNoFile( pfx, @@ -115,9 +127,8 @@ public void VerifyPreservePreserveProvider(bool preserveProvider, bool preserveN using (cert) { using (RSA key = cert.GetRSAPrivateKey()) + using (CngKey cngKey = Assert.IsType(key).Key) { - CngKey cngKey = Assert.IsType(key).Key; - if (preserveName) { Assert.Equal(keyName, cngKey.KeyName); @@ -137,6 +148,9 @@ public void VerifyPreservePreserveProvider(bool preserveProvider, bool preserveN { Assert.NotEqual(CapiProvider, cngKey.Provider.Provider); } + + // MachineKey is preserved irrespective of PreserveKeyName or PreserveStorageProvider + Assert.Equal(machineKey, cngKey.IsMachineKey); } // Alias is not preserved @@ -144,10 +158,55 @@ public void VerifyPreservePreserveProvider(bool preserveProvider, bool preserveN } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void VerifyNamesWithDuplicateAttributes(bool noLimits) + { + // This test mainly shows that when duplicate attributes are present contents + // processed by our filter and processed directly by PFXImportCertStore come up + // with the same answer. + + Pkcs12LoaderLimits limits = Pkcs12LoaderLimits.DangerousNoLimits; + + // DangerousNoLimits is tested by reference, by cloning the object we + // use a functional equivalent using the work-limiting and attribute-filtering + // loader. + if (!noLimits) + { + limits = new Pkcs12LoaderLimits(limits); + } + + X509Certificate2 cert = LoadPfxNoFile( + TestData.DuplicateAttributesPfx, + TestData.PlaceholderPw, + X509KeyStorageFlags.DefaultKeySet, + loaderLimits: limits); + + using (cert) + { + Assert.Equal("Certificate 1", cert.GetNameInfo(X509NameType.SimpleName, false)); + Assert.True(cert.HasPrivateKey, "cert.HasPrivateKey"); + Assert.Equal("Microsoft Enhanced RSA and AES Cryptographic Provider", cert.FriendlyName); + + using (RSA key = cert.GetRSAPrivateKey()) + using (CngKey cngKey = Assert.IsType(key).Key) + { + Assert.Equal("Microsoft Enhanced RSA and AES Cryptographic Provider", cngKey.Provider.Provider); + Assert.True(cngKey.IsMachineKey, "cngKey.IsMachineKey"); + + // If keyname000 gets taken, we'll get a random key name on import. What's important is that we + // don't pick the second entry: keyname001. + Assert.NotEqual("keyname001", cngKey.KeyName); + } + } + } + private static byte[] MakeAttributeTest( string? keyName = null, string? friendlyName = null, bool useCapi = false, + bool machineKey = false, [CallerMemberName] string testName = null) { CngKey cngKey = null; @@ -164,6 +223,7 @@ private static byte[] MakeAttributeTest( CspParameters cspParameters = new CspParameters(24) { KeyContainerName = keyName, + Flags = machineKey ? CspProviderFlags.UseMachineKeyStore : CspProviderFlags.NoFlags, }; rsaCsp = new RSACryptoServiceProvider(2048, cspParameters); @@ -174,6 +234,7 @@ private static byte[] MakeAttributeTest( CngKeyCreationParameters cngParams = new CngKeyCreationParameters { ExportPolicy = CngExportPolicies.AllowPlaintextExport, + KeyCreationOptions = machineKey ? CngKeyCreationOptions.MachineKey : CngKeyCreationOptions.None, }; cngKey = CngKey.Create(CngAlgorithm.Rsa, keyName, cngParams); 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 6568be9756eb2f..086721eef28b3f 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/X509CertificateLoaderPkcs12Tests.cs @@ -4,6 +4,7 @@ using System.Buffers; using System.IO; using System.IO.MemoryMappedFiles; +using System.Security.Cryptography.Pkcs; using Test.Cryptography; using Xunit; @@ -735,5 +736,153 @@ public void VerifyIndependentLifetime() } } } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void LoadWithDuplicateAttributes(bool allowDuplicates) + { + Pkcs12LoaderLimits limits = Pkcs12LoaderLimits.Defaults; + + if (allowDuplicates) + { + limits = Pkcs12LoaderLimits.DangerousNoLimits; + } + + // remove the edit lock + limits = new Pkcs12LoaderLimits(limits) + { + PreserveCertificateAlias = false, + PreserveKeyName = false, + PreserveStorageProvider = false, + PreserveUnknownAttributes = false, + }; + + Func func = + () => LoadPfxNoFile(TestData.DuplicateAttributesPfx, TestData.PlaceholderPw, loaderLimits: limits); + + if (allowDuplicates) + { + using (X509Certificate2 cert = func()) + { + Assert.Equal("Certificate 1", cert.GetNameInfo(X509NameType.SimpleName, false)); + Assert.True(cert.HasPrivateKey, "cert.HasPrivateKey"); + } + } + else + { + Pkcs12LoadLimitExceededException ex = Assert.Throws(() => func()); + Assert.Contains("AllowDuplicateAttributes", ex.Message); + } + } + +#if NET + [Theory] + [InlineData(false)] + [InlineData(true)] + public void LoadWithDuplicateAttributes_KeyOnly(bool ignorePrivateKeys) + { + byte[] pfx; + + using (ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP256)) + { + CertificateRequest req = new CertificateRequest( + "CN=No Duplicates Here", + key, + HashAlgorithmName.SHA256); + + DateTimeOffset now = DateTimeOffset.UtcNow; + + using (X509Certificate2 cert = req.CreateSelfSigned(now, now.AddMinutes(1))) + { + Pkcs12SafeContents contents = new Pkcs12SafeContents(); + Pkcs9LocalKeyId keyId = new Pkcs9LocalKeyId(new byte[] { 2, 2, 4 }); + + Pkcs12CertBag certBag = contents.AddCertificate(cert); + certBag.Attributes.Add(keyId); + + Pkcs12KeyBag keyBag = contents.AddKeyUnencrypted(key); + keyBag.Attributes.Add(keyId); + keyBag.Attributes.Add(keyId); + + Pkcs12Builder builder = new Pkcs12Builder(); + builder.AddSafeContentsUnencrypted(contents); + builder.SealWithoutIntegrity(); + pfx = builder.Encode(); + } + } + + Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits + { + IgnorePrivateKeys = ignorePrivateKeys, + }; + + Exception ex = Assert.Throws( + () => LoadPfxNoFile(pfx, loaderLimits: limits)); + + Assert.Contains("AllowDuplicateAttributes", ex.Message); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void LoadWithDuplicateAttributes_EncryptedOnly(bool ignoreEncryptedAuthSafes) + { + byte[] pfx; + + using (ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP256)) + { + CertificateRequest req = new CertificateRequest( + "CN=No Duplicates Here", + key, + HashAlgorithmName.SHA256); + + DateTimeOffset now = DateTimeOffset.UtcNow; + + using (X509Certificate2 cert = req.CreateSelfSigned(now, now.AddMinutes(1))) + { + Pkcs12SafeContents certSafe = new Pkcs12SafeContents(); + Pkcs12SafeContents keySafe = new Pkcs12SafeContents(); + Pkcs9LocalKeyId keyId = new Pkcs9LocalKeyId(new byte[] { 2, 2, 4 }); + + Pkcs12CertBag certBag = certSafe.AddCertificate(cert); + certBag.Attributes.Add(keyId); + + Pkcs12KeyBag keyBag = keySafe.AddKeyUnencrypted(key); + keyBag.Attributes.Add(keyId); + keyBag.Attributes.Add(keyId); + + Pkcs12Builder builder = new Pkcs12Builder(); + builder.AddSafeContentsUnencrypted(certSafe); + + builder.AddSafeContentsEncrypted( + keySafe, + "", + new PbeParameters(PbeEncryptionAlgorithm.TripleDes3KeyPkcs12, HashAlgorithmName.SHA1, 1)); + + builder.SealWithoutIntegrity(); + pfx = builder.Encode(); + } + } + + Pkcs12LoaderLimits limits = new Pkcs12LoaderLimits + { + IgnoreEncryptedAuthSafes = ignoreEncryptedAuthSafes, + }; + + Func func = () => LoadPfxNoFile(pfx, loaderLimits: limits); + + if (ignoreEncryptedAuthSafes) + { + // Assert.NoThrow + func().Dispose(); + } + else + { + Exception ex = Assert.Throws(() => func()); + Assert.Contains("AllowDuplicateAttributes", ex.Message); + } + } +#endif } } 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 309d9b2e1c16b0..a08fb7f599bd17 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 @@ -34,6 +34,7 @@ + diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxTests.cs index 7a6a8db3567914..f59c1e7767c91e 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxTests.cs @@ -481,6 +481,38 @@ public static void CollectionPerphemeralImport_HasKeyName() } } + [Fact] + public static void VerifyNamesWithDuplicateAttributes() + { + // This is the same as the Windows Attributes test for X509CertificateLoaderPkcs12Tests, + // but using the legacy X509Certificate2 ctor, to test the settings for that set of + // loader limits with respect to duplicates. + + X509Certificate2 cert = new X509Certificate2(TestData.DuplicateAttributesPfx, TestData.PlaceholderPw); + + using (cert) + { + Assert.Equal("Certificate 1", cert.GetNameInfo(X509NameType.SimpleName, false)); + Assert.True(cert.HasPrivateKey, "cert.HasPrivateKey"); + + if (OperatingSystem.IsWindows()) + { + Assert.Equal("Microsoft Enhanced RSA and AES Cryptographic Provider", cert.FriendlyName); + + using (RSA key = cert.GetRSAPrivateKey()) + using (CngKey cngKey = Assert.IsType(key).Key) + { + Assert.Equal("Microsoft Enhanced RSA and AES Cryptographic Provider", cngKey.Provider.Provider); + Assert.True(cngKey.IsMachineKey, "cngKey.IsMachineKey"); + + // If keyname000 gets taken, we'll get a random key name on import. What's important is that we + // don't pick the second entry: keyname001. + Assert.NotEqual("keyname001", cngKey.KeyName); + } + } + } + } + internal static bool IsPkcs12IterationCountAllowed(long iterationCount, long allowedIterations) { if (allowedIterations == UnlimitedIterations)