From d655df554f38cfea2c67b2751d6c8e32cbda7a99 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Thu, 17 Aug 2023 16:49:55 +0800 Subject: [PATCH] fix: PKCS12 ephemeral key and non-encrypted MAC are not supported macOS (#124) Fix bugs for PKCS12 certificates on macOS: - doesn't support ephemeral key - doesn't support non-encrypted MAC Test on Linux, macOS, Windows Signed-off-by: Junjie Gao --------- Signed-off-by: Junjie Gao --- .../Certificate/CertificateChainTests.cs | 1 - .../Certificate/Pkcs12Tests.cs | 50 +++++++++++++++++ ...Notation.Plugin.AzureKeyVault.Tests.csproj | 1 + .../TestData/cert_invalid_mac.pfx | Bin 0 -> 2623 bytes .../TestData/cert_without_mac.pfx | Bin 0 -> 2093 bytes .../Certificate/CertificateChain.cs | 2 - .../Certificate/Pkcs12.cs | 51 ++++++++++++++++++ .../KeyVault/KeyVaultClient.cs | 19 ++++++- .../Notation.Plugin.AzureKeyVault.csproj | 1 + 9 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 Notation.Plugin.AzureKeyVault.Tests/Certificate/Pkcs12Tests.cs create mode 100644 Notation.Plugin.AzureKeyVault.Tests/TestData/cert_invalid_mac.pfx create mode 100644 Notation.Plugin.AzureKeyVault.Tests/TestData/cert_without_mac.pfx create mode 100644 Notation.Plugin.AzureKeyVault/Certificate/Pkcs12.cs diff --git a/Notation.Plugin.AzureKeyVault.Tests/Certificate/CertificateChainTests.cs b/Notation.Plugin.AzureKeyVault.Tests/Certificate/CertificateChainTests.cs index 930b1fb3..904a6024 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/Certificate/CertificateChainTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/Certificate/CertificateChainTests.cs @@ -1,7 +1,6 @@ using System.Collections.Generic; using System.IO; using System.Security.Cryptography.X509Certificates; -using Notation.Plugin.Protocol; using Xunit; namespace Notation.Plugin.AzureKeyVault.Certificate.Tests diff --git a/Notation.Plugin.AzureKeyVault.Tests/Certificate/Pkcs12Tests.cs b/Notation.Plugin.AzureKeyVault.Tests/Certificate/Pkcs12Tests.cs new file mode 100644 index 00000000..cb361797 --- /dev/null +++ b/Notation.Plugin.AzureKeyVault.Tests/Certificate/Pkcs12Tests.cs @@ -0,0 +1,50 @@ +using System.IO; +using System.Security.Cryptography.Pkcs; +using Notation.Plugin.Protocol; +using Xunit; + +namespace Notation.Plugin.AzureKeyVault.Certificate.Tests +{ + public class Pkcs12Tests + { + [Fact] + public void ReEncode() + { + // read the pfx file + byte[] data = File.ReadAllBytes(Path.Combine(Directory.GetCurrentDirectory(), "TestData", "cert_chain.pfx")); + Pkcs12Info originPfx = Pkcs12Info.Decode(data, out _); + Assert.True(originPfx.IntegrityMode == Pkcs12IntegrityMode.Password); + + // re-encode the pfx file + byte[] newData = Pkcs12.ReEncode(data); + Pkcs12Info pfxWithoutMac = Pkcs12Info.Decode(newData, out _); + Assert.True(pfxWithoutMac.IntegrityMode == Pkcs12IntegrityMode.None); + } + + [Fact] + public void ReEncode_WithInvalidMac() + { + // read the pfx file + byte[] data = File.ReadAllBytes(Path.Combine(Directory.GetCurrentDirectory(), "TestData", "cert_invalid_mac.pfx")); + Pkcs12Info originPfx = Pkcs12Info.Decode(data, out _); + Assert.True(originPfx.IntegrityMode == Pkcs12IntegrityMode.Password); + + // re-encode the pfx file + Assert.Throws(() => Pkcs12.ReEncode(data)); + } + + [Fact] + public void ReEncode_withoutMac(){ + // read the pfx file + byte[] data = File.ReadAllBytes(Path.Combine(Directory.GetCurrentDirectory(), "TestData", "cert_without_mac.pfx")); + Pkcs12Info originPfx = Pkcs12Info.Decode(data, out _); + Assert.True(originPfx.IntegrityMode == Pkcs12IntegrityMode.None); + + // re-encode the pfx file + byte[] newData = Pkcs12.ReEncode(data); + Pkcs12Info pfxWithoutMac = Pkcs12Info.Decode(newData, out _); + Assert.True(pfxWithoutMac.IntegrityMode == Pkcs12IntegrityMode.None); + + } + } +} diff --git a/Notation.Plugin.AzureKeyVault.Tests/Notation.Plugin.AzureKeyVault.Tests.csproj b/Notation.Plugin.AzureKeyVault.Tests/Notation.Plugin.AzureKeyVault.Tests.csproj index b7e396f2..081fb7f3 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/Notation.Plugin.AzureKeyVault.Tests.csproj +++ b/Notation.Plugin.AzureKeyVault.Tests/Notation.Plugin.AzureKeyVault.Tests.csproj @@ -11,6 +11,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/Notation.Plugin.AzureKeyVault.Tests/TestData/cert_invalid_mac.pfx b/Notation.Plugin.AzureKeyVault.Tests/TestData/cert_invalid_mac.pfx new file mode 100644 index 0000000000000000000000000000000000000000..59e404036b80f39837b643f25337397757b2f990 GIT binary patch literal 2623 zcmai$XE+;*8pmZ4#A$%W5n@s zPlFpBR)4K?)3-)r?h3^h2VEXFl*we9IGDrq@FAz

Gbn*l;;sBMut10n-d0+`ZIZ zai-8V9}3W(tn74V!ielSuu__GsGvIUwM6GG@m3Sj!e{f$sPtv-5>pCxB#71^6IUg? zmPpRbnCIgJE1lgQqkL%ZDeP;0g~`o(;#BwOlvA&HVx1&5x8{zQ=j)jtMCEYIH~^}n z+e1-Y^Om=?+$Afc$-K9L{!B5bvFIEd-s~%(LULnBzxt+dGprkLp|Ht&Xe->w`_Rl3 z_bCTpvy`PsFLc}peNdku$PK$0^07-xx%pO>g!Yb-LJV#(W4L?yffX{zQtAShOE}vVE&gIRxB24?oA#a1>{g!K){rP)2DrmuWn#IpBua`A;t%|k z&t|=-LB2N0TCm%HyqV7Zktq%FWsH6^J2jHJRsMC;Xq253gIzO!2$|E5s`z9@Nmn|s z-_wf6frgZXDGyip z<><)OdcVd*1n?5yauxko)kXMyK81^u2zw)!^T zA0)ta8T6cyy_MDK%l^(f7u4BO$|A>c-op3-x75rB9$OZH&PdkGgcT#(w(@r$Y1k~X zarkh&R2~A^S;1$>2R6`Yx@g8lpD9<6Hy6?O*}+=fnht5%+wG~H$Gc9b2N=CR#7k+d zD-i9(p6^aKn#^B#$pX!zCg)FDL-JMx;ma!bdpM;^2hX$x6TrUKeGN4x0DjJP$Du_+ z)qxFYiHLnR5aF;l@qFQUe`3H&kY(JkO}!XXRE9aVs<*X86=9!`S!uaL=|cp87MD7V zABVje+RW6mI&ZYsRvVXYt^TrOEPT;3*hqIZPPqt8g2epv!+(VX2?moO{^$7q`FKIu z{_i4oRsit4#GJ!R|0I|si+U+dtSI&$1#=7i4j|5WJ8GKN(QTt$t_voC>Az^6jD0Q9 z;kSA2yp@P2(pG7Z&rICNYUPw<&6$83BLI4DD}{TpFoJu|YqINC*-?ypW)$PwlwDZ6 zETvX0qoTt>H8xRm6nf+jB!v>wP@T=IIOP+BlWh2_K9 zQ-+?O>rutcLWc1Pk@o^;h|hsbsJ_ye^`2db7>)_7J8o~uBR;<-v&xx1z#JLxS@E`{%#`~C>hVQF=^kWuJ^!Xd_JV%tW?+|u-|uf} zXWP|*EmRRe&)UWWzocpjuh1hJ;eOf+`SlE+t#qq(6318%R z(ucqU#s&9CyH=->?@Z?K)pZTaK^|vMea|KLlZOi{dgM@-=WoB;N2>vAW&arMPaI?TE7WDPkRie!;HNFo1NaYd>NBH|$8RGQl`3PNC2CvT#}$ zcdn^L_RHf35S?yW1H@*{tiv-L(ABX>3x4Ft#THUAFih5gT$3#uGsz4)6kwfM&c?Ya zq=Wpgt~?TgmygLdU*CGroApE5`o8C@t3@M+t;S)#Yv#VlQ)3!pm+yz;_6DInM91w* zfo6cz=s~L0Ft6Izb)Qz5#^jzN??iTU@yGqDdPM1df~8d>q8O+Mc?6K<7l4{b{*@A% zF?+JesCHKw%|KbxN`kdxtyR~r0&j-~FP~P`@B}P$i1Y}Q_*}6Zr=@)9G^;sQ9HHn6 zYzIa_$Zze4cZ7uk6qB~o2%%g-ySaxdG5Zg>;kJ3zX$@$UO&CYpi^cM(UR&oVawI_m z(o-+3z@mrW^uST(oEdY_JHJ$p#z^h#c)5tgf{|JSF=#&!nSuW zvi#VKeV4?HHeWZ^h2}sWuTAU_Yc=zJgs}?;c2Qd@VKW(m{T*=j5Cc3R)M*MFRGqWD z*l)f-IidNd#ttzn&EE0u!&5utO}!LZ!X>j*g)$xhtLMDW?9`A#@f&)Aa{4XTO#{0F z$~fwb3m|DZUX@%#Wm(}`{`MFiXnNo8Htg;H_%R4N3E)Q5BCp(*1u6&^TJD`i)B_B z=xJ0H8%c|l8%s7aGBJt{P#SnI@TY$c28ovZDOeaUh7XKL5Hk_J%OVow)cO9N0CoDn z=kk&#S`&>y!+*YCKmddZEUqGmszwT5)cdo-e(xz&hUAGjA_;CyM7!X$wLsYca8I@yKRi4G@8Bndn9uT; zIoJy<$)jmrrI5w>up|Q%m=&9(p0dY(0c_br$u)1fECR!y_~jI5Du-UmXj9e_kwVf; zeHs*Wi`koAT?c_6pYLR=A}K1-;=V)pVgkhBqCaVRnK;Ut|)g1%aTS_#I4 zfe4>;Ex<`jnvt01rGWm{WfyDAc7308huEcshq858%Im)-wti+_&{B!^m^n3U7$%^+ z-R4w%zvb}%(2)I($Wt}}$9!bnd(re!YXg63<67fOF-&a%3M0^2bKPD>;sMRbID=C- zbQ5Y{hV*qi<8U&2Jd6j*>@H&@WUVd?yw#0QMqJDWWTvqhuur(fsJbr0i9`v3ZT3*&Xo(D85b`P zjg7k_gQ||5z(Rz={3&XSn72stYY#(-1Hx;xBN!Mo>c*8rbiF(!vCq-I9a0_|$X79A z-}q#2l(-_)Q`f{3CD+cQ9tBJ?D`hB}U)0eYR{G)5;|?pmH0@>1FZ1H_JufJhB$*1Q z?UKi@!&V`w(0T`JO1%NeDLo(1*t-6EB*9yRMoyQ znvk_bG5=sm&xF!4{s1!BNfWU&QrIA zCLzIX)}d(GuX^b_NXh3>zIxe6V-;#KCoXOSg6|m-2RW)dvH!XyNlK%LkYB|L}n1M+9IWO2+t$5Pbg7$;z`PYRRy(fF$f|HqJSo*(8 zNOPp|Y5a+cX1lh+Tg}j-whNRF_RX$B^7fo;evX~XChYQYB(y4xai!g07r$&2uu^vs zNnqLvC)1_Wj9eZo=3GktKW%l&tD zGiRy`ittp}Zo_8TlXkT`CYS!$HylJp8g-CxDm=e2aPU>tZ>#}#n2SB7Qs&dvL5F7I z9_=>uX`Mc;p4uvC2oU?e`Zhpe1JG?%oY=<*{WImv4do5keVnpijWP3TT~2#Rd`cE- z2ZJ>~F`5NN@%({9yo~J_9?g9FIn}H%Fh?pXG=khT!^zcKUhGE}myXOwQipYrEWvx5 zA_H4LJ7}T{o;|ne#riZpzE6W|r%oR9F+ny1PFK8kLtCb}9FY992oj$VH@M(OZgxjH z;&F6A(wHqtQJ&hxTR@-we-=^puxS(EmXl0iX5k91^j@w#ShUM`fSWnr)o^bygplIr z{GC{47s^pt+tqkYw?LG48pfIrXMJuOSgRiP>O7|tP$zB5giQ_@+>R#?1WIjrClf!3 zG)wpI(3q7<(Cbz&e@Q?9nH_ti{-wk7;MHtYK?w+d_E)&L za?+Bp?m%6OXWrER+bu*SulCB3UH?q*OBgBln<~75x=Y7$>ZOL{J zY5+K}Be~Wp7U11$Dnb`1yA-8bAeKx+8^ly)GE4%*LP{N2lBf5}qo|K!-)ulbwmdYT zChokoAMK_9(Nz%mundED>@u8yzR- zURk7l+u!Letw%gG_VskzyHgqV6lS{ayO`{FGMHaJg5&s{5A8)6jtzB|GHDmYXP8Fc z<7}+ujC#9KV(RPDrvx0iLoNmD(GyjL4vX2^ + /// Re-encode the PKCS12 data to remove the MAC and keys. + /// The macOS doesn't support PKCS12 with non-encrypted MAC. + /// + /// + /// + /// + public static byte[] ReEncode(byte[] data) + { + Pkcs12Info pfx = Pkcs12Info.Decode(data, out _); + // only remove the MAC if it is password protected + if (pfx.IntegrityMode != Pkcs12IntegrityMode.Password) + { + return data; + } + // verify the MAC with null password + if (!pfx.VerifyMac(null)) + { + throw new ValidationException("Invalid MAC or the MAC password is not null"); + } + + // re-build PFX without MAC and keys + Pkcs12Builder pfxBuilder = new Pkcs12Builder(); + foreach (var safeContent in pfx.AuthenticatedSafe) + { + // decrypt with null password + safeContent.Decrypt((byte[]?)null); + + // create a newSafeContent and only contains the certificate bag + var newSafeContent = new Pkcs12SafeContents(); + foreach (var bag in safeContent.GetBags()) + { + if (bag is Pkcs12CertBag) + { + newSafeContent.AddSafeBag(bag); + } + } + pfxBuilder.AddSafeContentsUnencrypted(newSafeContent); + } + pfxBuilder.SealWithoutIntegrity(); + return pfxBuilder.Encode(); + } + } +} diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 11821476..87dd8cad 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -1,9 +1,11 @@ using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Security.Cryptography.X509Certificates; using Azure.Identity; using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Keys.Cryptography; using Azure.Security.KeyVault.Secrets; +using Notation.Plugin.AzureKeyVault.Certificate; using Notation.Plugin.Protocol; [assembly: InternalsVisibleTo("Notation.Plugin.AzureKeyVault.Tests")] @@ -190,7 +192,22 @@ public async Task GetCertificateChainAsync() { case "application/x-pkcs12": // If the secret is a PKCS12 file, decode the base64 encoding - chain.Import(Convert.FromBase64String(secretValue), "", X509KeyStorageFlags.EphemeralKeySet); + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + // macOS doesn't support non-encrypted MAC + // https://github.com/dotnet/runtime/issues/23635 + chain.Import( + rawData: Pkcs12.ReEncode(Convert.FromBase64String(secretValue)), + password: null, + keyStorageFlags: X509KeyStorageFlags.DefaultKeySet); + } + else + { + chain.Import( + rawData: Convert.FromBase64String(secretValue), + password: null, + keyStorageFlags: X509KeyStorageFlags.EphemeralKeySet); + } break; case "application/x-pem-file": // If the secret is a PEM file, parse the PEM content directly diff --git a/Notation.Plugin.AzureKeyVault/Notation.Plugin.AzureKeyVault.csproj b/Notation.Plugin.AzureKeyVault/Notation.Plugin.AzureKeyVault.csproj index bcefa212..7ba5e827 100644 --- a/Notation.Plugin.AzureKeyVault/Notation.Plugin.AzureKeyVault.csproj +++ b/Notation.Plugin.AzureKeyVault/Notation.Plugin.AzureKeyVault.csproj @@ -20,6 +20,7 @@ +