Skip to content

[KeyVault] Migrate BouncyCastle Library #28924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[KeyVault] Migrate BouncyCastle Library #28924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/KeyVault/KeyVault/Az.KeyVault.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ RequiredModules = @(@{ModuleName = 'Az.Accounts'; ModuleVersion = '5.3.1'; })
# Assemblies that must be loaded prior to importing this module
RequiredAssemblies = 'Azure.Security.KeyVault.Administration.dll',
'Azure.Security.KeyVault.Certificates.dll',
'Azure.Security.KeyVault.Keys.dll', 'BouncyCastle.Crypto.dll',
'Azure.Security.KeyVault.Keys.dll',
'KeyVault.Autorest/bin/Az.KeyVault.private.dll',
'Microsoft.Azure.KeyVault.dll',
'Microsoft.Azure.KeyVault.WebKey.dll',
Expand Down
2 changes: 2 additions & 0 deletions src/KeyVault/KeyVault/ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- Additional information about change #1
-->
## Upcoming Release
* Updated security domain PEM key handling to use built-in .NET cryptography instead of Portable.BouncyCastle
- Maintains support for standard PKCS#1 and PKCS#8 keys while resolving the CodeQL cs/use-approved-crypto-library finding
Comment on lines +21 to +22
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry uses technical acronyms "PEM", "PKCS#1", and "PKCS#8" without explanation. According to the ChangeLog.md guidelines for Azure PowerShell, less-obvious acronyms should be explained on first use in a release section, as the primary audience is Azure PowerShell users, not developers.

Consider updating to:

* Updated security domain PEM (Privacy Enhanced Mail) key handling to use built-in .NET cryptography instead of Portable.BouncyCastle
    - Maintains support for standard PKCS#1 and PKCS#8 key formats while resolving the CodeQL cs/use-approved-crypto-library finding

Copilot generated this review using guidance from repository custom instructions.

## Version 6.4.1
* Updated Azure.Core from 1.45.0 to 1.47.3
Expand Down
1 change: 0 additions & 1 deletion src/KeyVault/KeyVault/KeyVault.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
<PackageReference Include="Azure.Security.KeyVault.Administration" Version="4.4.0-beta.1" />
<PackageReference Include="Azure.Security.KeyVault.Keys" Version="4.6.0-beta.1" />
<PackageReference Include="Azure.Security.KeyVault.Certificates" Version="4.6.0" />
<PackageReference Include="Portable.BouncyCastle" Version="1.8.8" />
<PackageReference Include="Microsoft.Azure.KeyVault" Version="3.0.1" />
<PackageReference Include="Microsoft.Azure.KeyVault.WebKey" Version="3.0.1" />
<PackageReference Include="System.Security.Cryptography.Cng" Version="4.5.0" />
Expand Down
64 changes: 13 additions & 51 deletions src/KeyVault/KeyVault/SecurityDomain/Models/CertKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Microsoft.Azure.Commands.KeyVault.SecurityDomain.Common;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.OpenSsl;
using System;
using System.IO;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -51,61 +49,25 @@ public void Load(KeyPath path)

static RSAParameters RsaParamsFromPem(string path, string password)
{
using (var stream = File.OpenText(path))
{
var reader = string.IsNullOrEmpty(password) ? new PemReader(stream) : new PemReader(stream, new PasswordFinder(password));
var keyParameters = reader.ReadObject() as RsaPrivateCrtKeyParameters;
string pem = File.ReadAllText(path);

return ToRSAParameters(keyParameters);
}
}

static RSAParameters ToRSAParameters(RsaPrivateCrtKeyParameters privKey)
{
RSAParameters rp = new RSAParameters
using (RSA rsa = RSA.Create())
{
Modulus = privKey.Modulus.ToByteArrayUnsigned(),
Exponent = privKey.PublicExponent.ToByteArrayUnsigned(),
P = privKey.P.ToByteArrayUnsigned(),
Q = privKey.Q.ToByteArrayUnsigned()
};
rp.D = ConvertRSAParametersField(privKey.Exponent, rp.Modulus.Length);
rp.DP = ConvertRSAParametersField(privKey.DP, rp.P.Length);
rp.DQ = ConvertRSAParametersField(privKey.DQ, rp.Q.Length);
rp.InverseQ = ConvertRSAParametersField(privKey.QInv, rp.Q.Length);
return rp;
}


static byte[] ConvertRSAParametersField(Org.BouncyCastle.Math.BigInteger n, int size)
{
byte[] bs = n.ToByteArrayUnsigned();
if (bs.Length == size)
return bs;
if (bs.Length > size)
throw new ArgumentException("Specified size too small", "size");
byte[] padded = new byte[size];
Array.Copy(bs, 0, padded, size - bs.Length, bs.Length);
return padded;
if (string.IsNullOrEmpty(password))
{
rsa.ImportFromPem(pem);
}
else
{
rsa.ImportFromEncryptedPem(pem, password);
Comment on lines +58 to +62
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The methods ImportFromPem and ImportFromEncryptedPem require .NET 5.0 or later, but this project targets netstandard2.0 (as defined in Az.props). This will cause runtime errors when the code is executed on .NET Framework or .NET Standard 2.0 runtimes.

Since the BouncyCastle library was previously used to support these older runtimes, removing it without ensuring the replacement methods are available will break backwards compatibility. You need to either:

  1. Keep using a PEM parsing library compatible with netstandard2.0, or
  2. Add conditional compilation to use different implementations based on the target framework, or
  3. Upgrade the project's target framework to net5.0 or later (which may have broader implications for Azure PowerShell)

Copilot uses AI. Check for mistakes.
}

return rsa.ExportParameters(true);
}
}

X509Certificate2 _cert;
RSA _key;
byte[] _thumbprint;

private class PasswordFinder : IPasswordFinder
{
private readonly string _password;

public PasswordFinder(string password)
{
_password = password;
}

public char[] GetPassword()
{
return _password.ToCharArray();
}
}
}
}
1 change: 0 additions & 1 deletion src/KeyVault/KeyVault/Track2Models/Track2KeyVaultDataServiceClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
using Microsoft.Azure.Commands.KeyVault.Models;
using Microsoft.Azure.KeyVault.Models;
using Org.BouncyCastle.X509;

using System;
using System.Collections;
Expand Down
Loading