Skip to content

Commit

Permalink
fix: PKCS12 ephemeral key and non-encrypted MAC are not supported mac…
Browse files Browse the repository at this point in the history
…OS (#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 <junjiegao@microsoft.com>

---------

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
  • Loading branch information
JeyJeyGao committed Aug 17, 2023
1 parent a685a90 commit d655df5
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 4 deletions.
@@ -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
Expand Down
50 changes: 50 additions & 0 deletions 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<ValidationException>(() => 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);

}
}
}
Expand Up @@ -11,6 +11,7 @@
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
<PackageReference Include="Moq" Version="4.18.4" />
<PackageReference Include="Moq.AutoMock" Version="3.5.0" />
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="7.0.3" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
Binary file not shown.
Binary file not shown.
2 changes: 0 additions & 2 deletions Notation.Plugin.AzureKeyVault/Certificate/CertificateChain.cs
@@ -1,6 +1,4 @@
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using Notation.Plugin.Protocol;

namespace Notation.Plugin.AzureKeyVault.Certificate
{
Expand Down
51 changes: 51 additions & 0 deletions Notation.Plugin.AzureKeyVault/Certificate/Pkcs12.cs
@@ -0,0 +1,51 @@
using System.Security.Cryptography.Pkcs;
using Notation.Plugin.Protocol;

namespace Notation.Plugin.AzureKeyVault.Certificate
{
static class Pkcs12
{
/// <summary>
/// Re-encode the PKCS12 data to remove the MAC and keys.
/// The macOS doesn't support PKCS12 with non-encrypted MAC.
/// </summary>
/// <param name="data"></param>
/// <returns></returns>
/// <exception cref="ValidationException"></exception>
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();
}
}
}
19 changes: 18 additions & 1 deletion 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")]
Expand Down Expand Up @@ -190,7 +192,22 @@ public async Task<X509Certificate2Collection> 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
Expand Down
Expand Up @@ -20,6 +20,7 @@
<PackageReference Include="Azure.Security.KeyVault.Certificates" Version="4.5.1" />
<PackageReference Include="Azure.Security.KeyVault.Keys" Version="4.5.0" />
<PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.5.0" />
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="7.0.3" />
</ItemGroup>

<Target Name="GenerateBuildMetadata" BeforeTargets="CoreCompile">
Expand Down

0 comments on commit d655df5

Please sign in to comment.