From 2af13658fcd601f951ed9ff60446b7a5241efbab Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Thu, 5 Jul 2018 11:31:46 -0700 Subject: [PATCH] Unprotect key material with the local cache of certificates before checking the cert store In some cases, private keys for certificates is not completely available. When attempting to decrypt key material, this can cause 'CryptographicException: Keyset does not exist'. This changes the order in which key material decryption looks up private keys to first key the certificate options provided explicitly to the API, and then falling back to the cert store for decryption keys. --- .vscode/launch.json | 10 +++ .../XmlEncryption/EncryptedXmlDecryptor.cs | 68 +++++++++--------- .../XmlEncryption/XmlKeyDecryptionOptions.cs | 24 +++++-- .../DataProtectionProviderTests.cs | 65 ++++++++++++++--- .../TestFiles/TestCert3.pfx | Bin 0 -> 2429 bytes .../TestFiles/TestCert3WithoutPrivateKey.pfx | Bin 0 -> 1040 bytes .../TestFiles/TestCertWithoutPrivateKey.pfx | Bin 0 -> 968 bytes 7 files changed, 118 insertions(+), 49 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx create mode 100644 test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx create mode 100644 test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCertWithoutPrivateKey.pfx diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000000..f4fc2e3731 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,10 @@ +{ + "configurations": [ + { + "name": ".NET Core Attach", + "type": "coreclr", + "request": "attach", + "processId": "${command:pickProcess}" + } + ] +} diff --git a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs index e020ac7bb0..fee981b2d7 100644 --- a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs +++ b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Security.Cryptography.Xml; @@ -63,8 +62,7 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption var elementToDecrypt = (XmlElement)xmlDocument.DocumentElement.FirstChild; // Perform the decryption and update the document in-place. - var decryptionCerts = _options?.KeyDecryptionCertificates; - var encryptedXml = new EncryptedXmlWithCertificateKeys(decryptionCerts, xmlDocument); + var encryptedXml = new EncryptedXmlWithCertificateKeys(_options, xmlDocument); _decryptor.PerformPreDecryptionSetup(encryptedXml); encryptedXml.DecryptDocument(); @@ -83,48 +81,40 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption /// private class EncryptedXmlWithCertificateKeys : EncryptedXml { - private readonly IReadOnlyDictionary _certificates; + private readonly XmlKeyDecryptionOptions _options; - public EncryptedXmlWithCertificateKeys(IReadOnlyDictionary certificates, XmlDocument document) + public EncryptedXmlWithCertificateKeys(XmlKeyDecryptionOptions options, XmlDocument document) : base(document) { - _certificates = certificates; + _options = options; } public override byte[] DecryptEncryptedKey(EncryptedKey encryptedKey) { - byte[] key = base.DecryptEncryptedKey(encryptedKey); - if (key != null) + if (_options != null && _options.KeyDecryptionCertificateCount > 0) { - return key; - } - - if (_certificates == null || _certificates.Count == 0) - { - return null; - } - - var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator(); - if (keyInfoEnum == null) - { - return null; - } - - while (keyInfoEnum.MoveNext()) - { - if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data)) + var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator(); + if (keyInfoEnum == null) { - continue; + return null; } - key = GetKeyFromCert(encryptedKey, kiX509Data); - if (key != null) + while (keyInfoEnum.MoveNext()) { - return key; + if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data)) + { + continue; + } + + byte[] key = GetKeyFromCert(encryptedKey, kiX509Data); + if (key != null) + { + return key; + } } } - return null; + return base.DecryptEncryptedKey(encryptedKey); } private byte[] GetKeyFromCert(EncryptedKey encryptedKey, KeyInfoX509Data keyInfo) @@ -142,17 +132,25 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption continue; } - if (!_certificates.TryGetValue(certInfo.Thumbprint, out var certificate)) + if (!_options.TryGetKeyDecryptionCertificates(certInfo, out var keyDecryptionCerts)) { continue; } - using (RSA privateKey = certificate.GetRSAPrivateKey()) + foreach (var keyDecryptionCert in keyDecryptionCerts) { - if (privateKey != null) + if (!keyDecryptionCert.HasPrivateKey) { - var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl; - return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP); + continue; + } + + using (RSA privateKey = keyDecryptionCert.GetRSAPrivateKey()) + { + if (privateKey != null) + { + var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl; + return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP); + } } } } diff --git a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs index 01999c224d..7da598816f 100644 --- a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs +++ b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs @@ -12,16 +12,28 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption /// internal class XmlKeyDecryptionOptions { - private readonly Dictionary _certs = new Dictionary(StringComparer.Ordinal); + private readonly Dictionary> _certs = new Dictionary>(StringComparer.Ordinal); - /// - /// A mapping of key thumbprint to the X509Certificate2 - /// - public IReadOnlyDictionary KeyDecryptionCertificates => _certs; + public int KeyDecryptionCertificateCount => _certs.Count; + + public bool TryGetKeyDecryptionCertificates(X509Certificate2 certInfo, out IReadOnlyList keyDecryptionCerts) + { + var key = GetKey(certInfo); + var retVal = _certs.TryGetValue(key, out var keyDecryptionCertsRetVal); + keyDecryptionCerts = keyDecryptionCertsRetVal; + return retVal; + } public void AddKeyDecryptionCertificate(X509Certificate2 certificate) { - _certs[certificate.Thumbprint] = certificate; + var key = GetKey(certificate); + if (!_certs.TryGetValue(key, out var certificates)) + { + certificates = _certs[key] = new List(); + } + certificates.Add(certificate); } + + private string GetKey(X509Certificate2 cert) => cert.Thumbprint; } } diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs index d20332c1e2..a66ebec2e8 100644 --- a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs +++ b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.Reflection; using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -13,7 +12,6 @@ using Microsoft.AspNetCore.DataProtection.Repositories; using Microsoft.AspNetCore.DataProtection.Test.Shared; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -120,10 +118,12 @@ namespace Microsoft.AspNetCore.DataProtection public void System_UsesProvidedDirectoryAndCertificate() { var filePath = Path.Combine(GetTestFilesPath(), "TestCert.pfx"); - var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); - store.Open(OpenFlags.ReadWrite); - store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable)); - store.Close(); + using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + store.Open(OpenFlags.ReadWrite); + store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable)); + store.Close(); + } WithUniqueTempDirectory(directory => { @@ -139,7 +139,12 @@ namespace Microsoft.AspNetCore.DataProtection // Step 2: instantiate the system and round-trip a payload var protector = DataProtectionProvider.Create(directory, certificate).CreateProtector("purpose"); - Assert.Equal("payload", protector.Unprotect(protector.Protect("payload"))); + var data = protector.Protect("payload"); + + // add a cert without the private key to ensure the decryption will still fallback to the cert store + var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCertWithoutPrivateKey.pfx"), "password"); + var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certWithoutKey)).CreateProtector("purpose"); + Assert.Equal("payload", unprotector.Unprotect(data)); // Step 3: validate that there's now a single key in the directory and that it's is protected using the certificate var allFiles = directory.GetFiles(); @@ -157,6 +162,50 @@ namespace Microsoft.AspNetCore.DataProtection }); } + [ConditionalFact] + [X509StoreIsAvailable(StoreName.My, StoreLocation.CurrentUser)] + public void System_UsesProvidedCertificateNotFromStore() + { + using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + store.Open(OpenFlags.ReadWrite); + var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3WithoutPrivateKey.pfx"), "password3", X509KeyStorageFlags.Exportable); + Assert.False(certWithoutKey.HasPrivateKey, "Cert should not have private key"); + store.Add(certWithoutKey); + store.Close(); + } + + WithUniqueTempDirectory(directory => + { + using (var certificateStore = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + certificateStore.Open(OpenFlags.ReadWrite); + var certInStore = certificateStore.Certificates.Find(X509FindType.FindBySubjectName, "TestCert", false)[0]; + Assert.NotNull(certInStore); + Assert.False(certInStore.HasPrivateKey); + + try + { + var certWithKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3.pfx"), "password3"); + + var protector = DataProtectionProvider.Create(directory, certWithKey).CreateProtector("purpose"); + var data = protector.Protect("payload"); + + var keylessUnprotector = DataProtectionProvider.Create(directory).CreateProtector("purpose"); + Assert.Throws(() => keylessUnprotector.Unprotect(data)); + + var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certInStore, certWithKey)).CreateProtector("purpose"); + Assert.Equal("payload", unprotector.Unprotect(data)); + } + finally + { + certificateStore.Remove(certInStore); + certificateStore.Close(); + } + } + }); + } + [Fact] public void System_UsesInMemoryCertificate() { @@ -242,7 +291,7 @@ namespace Microsoft.AspNetCore.DataProtection /// private static void WithUniqueTempDirectory(Action testCode) { - string uniqueTempPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + string uniqueTempPath = Path.Combine(AppContext.BaseDirectory, Path.GetRandomFileName()); var dirInfo = new DirectoryInfo(uniqueTempPath); try { diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx new file mode 100644 index 0000000000000000000000000000000000000000..364251ba09d52ea927529d4f68f72a2553837e4e GIT binary patch literal 2429 zcmV-@34-=8f(dy70Ru3C2|or2Duzgg_YDCD0ic2jFa&}LEHHuzC@_Kp-v$XPhDe6@ z4FLxRpn?O?FoFZj0s#Opf&;|{2`Yw2hW8Bt2LUh~1_~;MNQU<@de?&A}mBRReUK4ay3K^70K3~=DIZp(2~?WxCqWH zBF=MQEiEyJmp22`2;kO~)0ZgqoY%{8f6>x1WvsgJi%sv}1%;FmjLT)}Iz7;BYW8&W zrYNIvYXSYWeC@x1E@JkE8n^r(YpMvj?BqujtU%J)uN*-V;}|)lkZt|z3~w|$a!Bt8 zvxvBT`0O#O(0B-x4oNgVWbFuao1XwMtOJFtW1B6nj1xTdH&(Z3NE>m&L9BM>h=rcH z4h6X;Trc}1gqq4heR_k~HH1nx;XBqxr09ngLg8@+w#Z`^*%cR^8hvFO#?}UDoGs&0 z1hrzrls8VxMBsT|t3Kq=Ra-?xxNX((!?hAHr4iI6v1jZ$k7Q*9-3kUM2r3w!c!S=? zDM&I!IH>(Ks1&viqm(CViF*3}<{b_0L?1jT1RKr6O-(iGG-;I%Kx>sKnp4n`_}dH( zxs`nb#SzK|bH)caehl7BemJ6*HJ4?;*fk`iI4fQL8$DmM9kjE!rA(^FzyCHX=U`cJ zlWY0y6m@xMlQ7UPK#-5IKD{04hEm=Bi%cTd%ku#gf4`Gi068?8X(`|rDxjGdiXqu2 z(+sI?dq}wh)5Tm4ET8eSVjVnhKH^#mAVJNuIDc>9(A;$TK>$XtoY5lxSUQsFpU>ZW zd*3`t@^R&I2GrZX%Z|XA3moK_qor2ojh8!1uPI?$IW|4bx#8WbLfL&m@4*+&ZvFjk z`PziR0dDFx=7kg_jRJh|3=_F9I3><`5%6BU!iE_n zFp(WH`q{(Vn49~)lqE3(7(UgrHfsIuKEVAdIRQ1GW#Db!LmEE3{W^oewY+kiWCgfD zt)2?dT-9Hf#_o=elr!i23?^JW#j-4)Ac#pC;FoFd^ z1_>&LNQU@!suzZD2uetj~f4YvN=S2`Qro*-galV3-Aj zp3$Or`8i>Xx|tV1o2X%a`epELx&$F{ia+Icrbk5y{@Y2q9%Fo#{8HY1OO-u2;FOeZ z$7yHc*Gs?r&dMu;pqHdTKhyMKv@c@Ws5Li}mU!3Q_Ls)i@o`v{sVmbF>@CSIbJr{A zOfFnV`>KeQTl`vM40fm2?>;`&!I{!CDX$3-7@~j3PY2Gs+-A$x@}e6<3d`0itb#b7 z5ADOd=Q%#STsZ0_u}KyHtP&{ks4PEfe;)Q*jX68HH2|m#X!fY@QpQMSIz2V`dk>jD zc~~1bJ3S0~+jzjBCb~Tpqfk(8=Ym$RJXqm)Ov=u`#_L?!pdHRov?=hDg_>x&# zr9z=@K)B~0p6gMD;Fwr{b-O;bEzT~d7X{H>fE({WLSUM(F+p)v!>Gp@X!DX3?#s_S zo_lARq!a*m4M0vx{gsr;Y+pv*;(NQZn#z7@XM#k3$)O z{DlFjX`)dlHKC0j9JL0eC&<{i%>5Jms*eZbTMe%^M4wR#Oj=^|V^y|J6Z0j{@sdfx zY=i64MbVC*rFb;vPX~ib%o)QSdmK_WyfjFHCXzWJ#yvU$M>D;+VZcJzD6FKhsrxf%n1*Nd~)_e;RCqvCg)uM zn+WN;y2Lt};`c%&Gi3`J8LBbWrg-gIO7A(#fv9vE@Y5;ctAylS988|;g&swD-QhRR z7V&yrlQN@|9+s{gkD^rDOXGxu?CT0c($nEnS-m`gI6U$1HahGb?IIv(eK!YoVBN5C zT%&8=YuhJ>gYJn>Y^gxnkoW}fFhe_XLprWO1g)fP7U?eD!CC_zd>c><2S>V%0{Kr&7u( zD+Oqz{!9IhupEwZVxFfQ4J^UsSYTEkpN;v5Y>i286(cB-zo^HUaPyhsu(Mxd9zGh= zL8e|wfBT>nki`9d<5u#F0VEa-@0-TYCudi1VxC)d$VQl0Y{{X3JqWiY-md$qg#5n! z10dZ?gIwim226<8kpq>0xm!_xvEBp-YzCsze2}F@0s;sCiUVtk literal 0 HcmV?d00001 diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx new file mode 100644 index 0000000000000000000000000000000000000000..9776e9006d7edb14c0028c2327a4c3fe84580b0a GIT binary patch literal 1040 zcmV+r1n>JWf&>f#0Ru3C1JVWwDuzgg_YDCD0ic2d!vul@zc7LWyD)+Sw+0C+hDe6@ z4FLxRpn?OaFoFZ50s#Opf&-lf2`Yw2hW8Bt2LUh~1_~;MNQUX&|#mJ(eRet7OylA62aMH^7^9l!3u$nRoYoG%g3`FR7^+FUoNxk zJ^?6t?sVe~hu~@2JNl@R{)@DW-BEk19C#~eAr&J!Ftpd96${sr{BTLWJ}m~GkRWUq zoKPU(o{^8|%WpR1|Qb;%9g$G_NM3u)Kb9%sLkuGh_td8zeuXO zX6JkKXXTO5PyPuIu8k-i`#7!zW0$r$^!W+4w3jTbqT)#4r);P8i@53<+}d*gdw{JV z)=Xi^Ah;rt%z<@CM_1aU+>zXaqf^@P}?>x~C`mBwrtXEzRe>Ye&X4S}(#_`+{s z?<*5904^MyPzxdzcwdw=k#!W1z_PCG1)|=eYzAS#MvPoBVUmq*Bo?RPe>SO&z5eZ) zE_EIMp?SuN#Z`Si*qpRuh$Al;=wLRVogPhiWOoT$d7CCS5!yLk2SFv<6M8SOUEGA^ z+zVWg)SqR1f{O-7>wmOC^`bwxOTq$`h+^551b z`K^_WT-kA;;^YDmQ;&nb1#U~Lj?4xHB?0u0%LjquG2PAq;5PR{7!$hG3P^cR4EmS9 zyn6@|XpmULlB|7}iY%PvVA@vymJ`72iva!EioBBM8+di(Z>*VezdRUfhjRix6go(mW3=%jR7 z5apMV!g<+NSex6JxX`4XekrLw738PO^)HeMZ@%F^k`dRkMsWnwl1B52d&f9EV7FSE z>?%h+$+#wiOT66M&_Dh1GlA?MY-FMlbLIy?`+r%+>YhU)jY(|s%lwnhh0J@MWd^s0 z{`4+|hK79yd}87N0t}3b_wl>-Q|&!%Tra#(0kM-tS&UmapI8|6pO`Vnz$1hM<5(9=CwPx5KjWTk;O#r{leMdJ}PKMPaNuyAzH?wdYY(cXA}K!5IFD8O0TTSdQO3e#qnQ$jh~F zOy#>PLfh6`78i|*Z^;aN6qZ&9=6P5>g}uB6W$C)Z6SpH8DFdt@szD)n_Eo%qt%z`?WzFb>s%2~HpAn#9F3u1Ol(2O z0!a=3MDXBt(w7MZCR?kw?W%(sgVXsd?VAKX!wBoKL(u_LQEL_^6jn4>8<0&1Yo-g8pW`Lx!x8hYK?uA+0 zxOX~TJ=)@_f5@5>@@s?F7hkHJDpOqHHbs}PP&G1V<)H}S#wAO1yjp;q3c^$s0)wR0sp>A1Wg#qdyvsIb}9fNZf*C3aj;C?^$L zu8p+5orz>6c}>0i)pW}SKPk1;lg9!HoO6z*OWFV_5-W}CD4y|RFflM8FbM_)D-Ht! q8U+9Z6p}U?mQ(+72&RT0XI&*yC)8o$Hv|Z%W(LKg5515A0tf)#K)Xc% literal 0 HcmV?d00001