Skip to content

Commit 6ba2c3f

Browse files
Copilottg123
andauthored
Fix multi-certificate loading from certificate-authority-data (#1690)
* Initial plan * Fix multi-certificate loading in SetClusterDetails method Co-authored-by: tg123 <[email protected]> * Specify UTF8 encoding when reading certificate file Co-authored-by: tg123 <[email protected]> * Fix multi-certificate loading for all .NET frameworks - Add CertUtils.LoadFromPemText() method for both modern .NET and Classic - Use ImportFromPem for .NET 5.0+ (net8.0, net9.0, net10.0) - Use BouncyCastle for .NET Standard 2.0 and .NET Framework 4.8 - Simplify SetClusterDetails to use CertUtils methods for both certificate-authority-data and certificate-authority Co-authored-by: tg123 <[email protected]> * Fix build: Remove unnecessary using and add System.Text to Classic - Remove unused System.Security.Cryptography.X509Certificates using from ConfigFile.cs - Add System.Text using to Classic CertUtils.cs for Encoding class Co-authored-by: tg123 <[email protected]> * Add dedicated unit tests for LoadFromPemText method - Add LoadFromPemTextWithMultiCert test for multiple certificates - Add LoadFromPemTextWithSingleCert test for single certificate - Tests follow the same pattern as existing LoadPemWithMultiCert test Co-authored-by: tg123 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: tg123 <[email protected]>
1 parent fb26f62 commit 6ba2c3f

File tree

7 files changed

+148
-17
lines changed

7 files changed

+148
-17
lines changed

src/KubernetesClient.Classic/CertUtils.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Org.BouncyCastle.Security;
88
using Org.BouncyCastle.X509;
99
using System.Security.Cryptography.X509Certificates;
10+
using System.Text;
1011

1112
namespace k8s
1213
{
@@ -39,6 +40,33 @@ public static X509Certificate2Collection LoadPemFileCert(string file)
3940
return certCollection;
4041
}
4142

43+
/// <summary>
44+
/// Load pem encoded certificates from text
45+
/// </summary>
46+
/// <param name="pemText">PEM encoded certificate text</param>
47+
/// <returns>List of x509 instances.</returns>
48+
public static X509Certificate2Collection LoadFromPemText(string pemText)
49+
{
50+
var certCollection = new X509Certificate2Collection();
51+
using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(pemText)))
52+
{
53+
var certs = new X509CertificateParser().ReadCertificates(stream);
54+
55+
// Convert BouncyCastle X509Certificates to the .NET cryptography implementation and add
56+
// it to the certificate collection
57+
//
58+
foreach (Org.BouncyCastle.X509.X509Certificate cert in certs)
59+
{
60+
// This null password is to change the constructor to fix this KB:
61+
// https://support.microsoft.com/en-us/topic/kb5025823-change-in-how-net-applications-import-x-509-certificates-bf81c936-af2b-446e-9f7a-016f4713b46b
62+
string nullPassword = null;
63+
certCollection.Add(new X509Certificate2(cert.GetEncoded(), nullPassword));
64+
}
65+
}
66+
67+
return certCollection;
68+
}
69+
4270
/// <summary>
4371
/// Generates pfx from client configuration
4472
/// </summary>

src/KubernetesClient/CertUtils.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ public static X509Certificate2Collection LoadPemFileCert(string file)
2323
return certCollection;
2424
}
2525

26+
/// <summary>
27+
/// Load pem encoded certificates from text
28+
/// </summary>
29+
/// <param name="pemText">PEM encoded certificate text</param>
30+
/// <returns>List of x509 instances.</returns>
31+
public static X509Certificate2Collection LoadFromPemText(string pemText)
32+
{
33+
var certCollection = new X509Certificate2Collection();
34+
certCollection.ImportFromPem(pemText);
35+
return certCollection;
36+
}
37+
2638
/// <summary>
2739
/// Generates pfx from client configuration
2840
/// </summary>

src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Diagnostics;
55
using System.Net;
66
using System.Runtime.InteropServices;
7-
using System.Security.Cryptography.X509Certificates;
87
using System.Text;
98

109
namespace k8s
@@ -308,26 +307,14 @@ private void SetClusterDetails(K8SConfiguration k8SConfig, Context activeContext
308307
if (!string.IsNullOrEmpty(clusterDetails.ClusterEndpoint.CertificateAuthorityData))
309308
{
310309
var data = clusterDetails.ClusterEndpoint.CertificateAuthorityData;
311-
#if NET9_0_OR_GREATER
312-
SslCaCerts = new X509Certificate2Collection(X509CertificateLoader.LoadCertificate(Convert.FromBase64String(data)));
313-
#else
314-
string nullPassword = null;
315-
// This null password is to change the constructor to fix this KB:
316-
// https://support.microsoft.com/en-us/topic/kb5025823-change-in-how-net-applications-import-x-509-certificates-bf81c936-af2b-446e-9f7a-016f4713b46b
317-
SslCaCerts = new X509Certificate2Collection(new X509Certificate2(Convert.FromBase64String(data), nullPassword));
318-
#endif
310+
var pemText = Encoding.UTF8.GetString(Convert.FromBase64String(data));
311+
SslCaCerts = CertUtils.LoadFromPemText(pemText);
319312
}
320313
else if (!string.IsNullOrEmpty(clusterDetails.ClusterEndpoint.CertificateAuthority))
321314
{
322-
#if NET9_0_OR_GREATER
323-
SslCaCerts = new X509Certificate2Collection(X509CertificateLoader.LoadCertificateFromFile(GetFullPath(
315+
SslCaCerts = CertUtils.LoadPemFileCert(GetFullPath(
324316
k8SConfig,
325-
clusterDetails.ClusterEndpoint.CertificateAuthority)));
326-
#else
327-
SslCaCerts = new X509Certificate2Collection(new X509Certificate2(GetFullPath(
328-
k8SConfig,
329-
clusterDetails.ClusterEndpoint.CertificateAuthority)));
330-
#endif
317+
clusterDetails.ClusterEndpoint.CertificateAuthority));
331318
}
332319
}
333320
}

tests/KubernetesClient.Tests/CertUtilsTests.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,5 +98,49 @@ public void LoadPemWithMultiCert()
9898
Assert.True(certCollection[0].RawData.SequenceEqual(intermediateCert.RawData));
9999
Assert.True(certCollection[1].RawData.SequenceEqual(rootCert.RawData));
100100
}
101+
102+
/// <summary>
103+
/// Checks that multiple certificates can be loaded from PEM text
104+
/// </summary>
105+
[Fact]
106+
public void LoadFromPemTextWithMultiCert()
107+
{
108+
// Read the PEM text from the ca-bundle file
109+
var pemText = System.IO.File.ReadAllText("assets/ca-bundle.crt");
110+
var certCollection = CertUtils.LoadFromPemText(pemText);
111+
112+
#if NET9_0_OR_GREATER
113+
using var intermediateCert = X509CertificateLoader.LoadCertificateFromFile("assets/ca-bundle-intermediate.crt");
114+
using var rootCert = X509CertificateLoader.LoadCertificateFromFile("assets/ca-bundle-root.crt");
115+
#else
116+
using var intermediateCert = new X509Certificate2("assets/ca-bundle-intermediate.crt");
117+
using var rootCert = new X509Certificate2("assets/ca-bundle-root.crt");
118+
#endif
119+
120+
Assert.Equal(2, certCollection.Count);
121+
122+
Assert.True(certCollection[0].RawData.SequenceEqual(intermediateCert.RawData));
123+
Assert.True(certCollection[1].RawData.SequenceEqual(rootCert.RawData));
124+
}
125+
126+
/// <summary>
127+
/// Checks that a single certificate can be loaded from PEM text
128+
/// </summary>
129+
[Fact]
130+
public void LoadFromPemTextWithSingleCert()
131+
{
132+
// Read a single certificate PEM text
133+
var pemText = System.IO.File.ReadAllText("assets/ca-bundle-root.crt");
134+
var certCollection = CertUtils.LoadFromPemText(pemText);
135+
136+
#if NET9_0_OR_GREATER
137+
using var rootCert = X509CertificateLoader.LoadCertificateFromFile("assets/ca-bundle-root.crt");
138+
#else
139+
using var rootCert = new X509Certificate2("assets/ca-bundle-root.crt");
140+
#endif
141+
142+
Assert.Single(certCollection);
143+
Assert.True(certCollection[0].RawData.SequenceEqual(rootCert.RawData));
144+
}
101145
}
102146
}

tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,5 +822,31 @@ public void LoadInClusterNamespace()
822822
Assert.Equal("some namespace", config.Namespace);
823823
}
824824
}
825+
826+
/// <summary>
827+
/// Checks that multiple certificates are loaded from certificate-authority-data
828+
/// </summary>
829+
[Fact]
830+
public void LoadMultipleCertificatesFromCertificateAuthorityData()
831+
{
832+
var fi = new FileInfo("assets/kubeconfig.multi-ca.yml");
833+
var cfg = KubernetesClientConfiguration.BuildConfigFromConfigFile(fi, "multi-ca-context");
834+
835+
Assert.NotNull(cfg.SslCaCerts);
836+
Assert.Equal(2, cfg.SslCaCerts.Count);
837+
}
838+
839+
/// <summary>
840+
/// Checks that multiple certificates are loaded from certificate-authority file
841+
/// </summary>
842+
[Fact]
843+
public void LoadMultipleCertificatesFromCertificateAuthorityFile()
844+
{
845+
var fi = new FileInfo("assets/kubeconfig.multi-ca-file.yml");
846+
var cfg = KubernetesClientConfiguration.BuildConfigFromConfigFile(fi, "multi-ca-file-context", useRelativePaths: false);
847+
848+
Assert.NotNull(cfg.SslCaCerts);
849+
Assert.Equal(2, cfg.SslCaCerts.Count);
850+
}
825851
}
826852
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
apiVersion: v1
2+
kind: Config
3+
clusters:
4+
- cluster:
5+
certificate-authority: assets/ca-bundle.crt
6+
server: https://multi-ca-file-test.example.com:6443
7+
name: multi-ca-file-cluster
8+
contexts:
9+
- context:
10+
cluster: multi-ca-file-cluster
11+
user: test-user
12+
name: multi-ca-file-context
13+
current-context: multi-ca-file-context
14+
users:
15+
- name: test-user
16+
user:
17+
token: test-token
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
apiVersion: v1
2+
kind: Config
3+
clusters:
4+
- cluster:
5+
certificate-authority-data: LS0tIENvbW1lbnRzIHRvIG1ha2Ugc3VyZSB3ZSBzdGlsbCBwYXJzZSB0aGUgY2VydCBjb3JyZWN0bHkKCkludGVybWVkaWF0ZSBDZXJ0aWZpY2F0ZQoKLS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lDRUFFd0RRWUpLb1pJaHZjTkFRRUxCUUF3RlRFVE1CRUdBMVVFQXd3S2EzVmkKWlhKdVpYUmxjekFlRncweE9UQXpNRE14TnpBNE1EbGFGdzB5T1RBeU1qZ3hOekE0TURsYU1CWXhGREFTQmdOVgpCQU1NQzJWMFkyaGhibWN0ZFdJMU1JSUJJakFOQmdrcWhraUc5dzBCQVFFRkFBT0NBUThBTUlJQkNnS0NBUUVBCngxVHA3RGEzTmJqZEhtWWRZWi9HTnBDUkd2RkZhcDdFRzFwb2toZklMS1NiUHVzcWlPOXduS0RFNEFmZG4vWkUKQ1FWMFdod3RveDNqY3pCT0lSeStQNkZ2bFB5aEFwVXB5blZUd2dDaXVoVE0rdGhnT0RncGU2R1htVmxWSkd2dgpBb0x3N0NNbmRCNXNNczVISCtxQTJVMXE0VkZJL2NzcjMveWVLeldCaWszZFpWb2gwNHNJOVdUVkwrYmwvMVg1CjBkbDVxcnFrWWlEeDh5Y0FIeU9ubDhkaEpXK1JHbDY3SGlsaXVVZVNxNnZ3c2Z2OXJoM1RQOXdIVkYxUFhGSnAKV2ZYeTRXYkxtdWxkNXd4WG5RVk8yZzUxanFmcU45ZkQ4RkhJa2FlMUlrTy9QVVR1Y2xvTmxMaUZzcmFnUU9URApSVlNQK1RWM2dzaEFUQnMyTU1WWE13SURBUUFCbzJZd1pEQWRCZ05WSFE0RUZnUVUvM3c5QVIyY25FZXBXSDRFCjhhMXhMWkFuanlrd0h3WURWUjBqQkJnd0ZvQVVMcy9semN0OENHdlZkSWlxNHQ5VDRpZHU1T3d3RWdZRFZSMFQKQVFIL0JBZ3dCZ0VCL3dJQkFEQU9CZ05WSFE4QkFmOEVCQU1DQVlZd0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBS3c3NDFWMXdzekl0aEhCVjhkdkN5UW95b3pCSnVBbzRJSGJpaUZtenVpUXV5c2hNY1grUXM5YStnNk9HNWQxClVid0ZmVWxxem1aUWNiY1IvSmM2d016M3dPNkhveTVwUzN3L0ZSMlVNR1IzOW85NS83WENrVElPd0NxYXU2UHcKZHBndmJuYWlxUEZQcUQzb2hkVXVWUmNYRzN2YTVBbUtUc1VuN20rbFIvOTMvcXB0dCtTVVZwNmp3bmJHY3dvQgpzM3UyWFh4NXMxTTd0cXFqM3RBRU9QQ0tsb2hTNm1RNFgzd3VsZ3BaMVhwSjBXVHZjdm9QWEV0QTU2azd2WDNhCjRFNng2NkxaQ0ZBMlpSLzVDT3Y1RDA1NUFocmloS0w4a2JBdXR4aGZBMjdTSi9NR293em1UVDdrVlFoYTNTdTMKYW9PWVpnY1V3dytTa1JTR1ZydGdNZ1E9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0KClJvb3QgY2VydGlmaWNhdGUKLS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURFRENDQWZpZ0F3SUJBZ0lKQUpwYjlpcktnMkpqTUEwR0NTcUdTSWIzRFFFQkN3VUFNQlV4RXpBUkJnTlYKQkFNTUNtdDFZbVZ5Ym1WMFpYTXdIaGNOTVRrd016QXpNREF5TVRJM1doY05Nemt3TWpJMk1EQXlNVEkzV2pBVgpNUk13RVFZRFZRUUREQXByZFdKbGNtNWxkR1Z6TUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCCkNnS0NBUUVBdW9LK05hK0x5S0lpcG1mdEJ3NFkrWjE5Zzh5djZZK2RZdDZLbEJnMjRYSE5BMHYxY013dE9DREYKTWxtNHJzRDdKZDVVTzZ1Z2RrM2Z4RXZHR2hteldUWFNCUlVXY1RiU2NBTTQ5bUFMQkZrQ3ZOVFBLMnZWaGs3UAppbTJRUWw4YTV2all6OEhMS0pUYi9PKzBxK0trdHBkN1hUYVUyVTdaZWJpTFZzNWJ2TmJiM1pEdElqQUFSWTlTCmFsWjRoT3p1Vk5hU1g5TUJScVRXcTNIdUt3RGlWVFQzZGFuL0FCb1U4TmRlZFBmSWJ5WTQ4d2lRZ2pFWWI2NGcKM2dlWXBBckxRZWZmbzhmbWhVRVBSUi8xV3Jmdll2dm04c1Y4alQrcnF4SVRLSjVWbzVrcFpVcG9tRE90R1ZNUwpnR0FsZTZtY1RycWxyc0NGYzRnRlJSb0hpSDFPRFFJREFRQUJvMk13WVRBZEJnTlZIUTRFRmdRVUxzL2x6Y3Q4CkNHdlZkSWlxNHQ5VDRpZHU1T3d3SHdZRFZSMGpCQmd3Rm9BVUxzL2x6Y3Q4Q0d2VmRJaXE0dDlUNGlkdTVPd3cKRHdZRFZSMFRBUUgvQkFVd0F3RUIvekFPQmdOVkhROEJBZjhFQkFNQ0FZWXdEUVlKS29aSWh2Y05BUUVMQlFBRApnZ0VCQUZNcTV6NE9vYUlocXgvaS9idHBWTFJuUURjcER3ZFV1ckUwaU5QejNiZ09JNVFpSWU5NW9Vd1hTRlFMCmNpRUN2T2JmTW1pVnV6K3A3TkQ1ZU5keFlSNGhscTFXMVBZY2dSUWd1c1hDQzRYZC9YQUdhTFpYekgzU0JybXAKYnM1c2Zva2hYS05jY3NuUXU1WWEzSlJrQUx4eFVKK0RjT24rdmk5Z21FQXppK25YYnlxVWpJaFNENW55Z0NsWAowYVNLYnZoVW1YeWFKcFVIMGk3ZFN4V1AzTHFDRGp0cnUvNWVqTnRCMDk3ZE5jeUY4anMzWXVrM2h3cXllZ1F4CkVMbjRjL1RLUEw5TDh2RTd0SmcvTTc4RFBBdlJDaXV3bDBIUWNhc0JFMkFYMHdkcFkwVWVYc05EeXpVZi8yV0YKZkhZNERudUJkZVZkSHRsMXlQbFhtUWtNb1FNPQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==
6+
server: https://multi-ca-test.example.com:6443
7+
name: multi-ca-cluster
8+
contexts:
9+
- context:
10+
cluster: multi-ca-cluster
11+
user: test-user
12+
name: multi-ca-context
13+
current-context: multi-ca-context
14+
users:
15+
- name: test-user
16+
user:
17+
token: test-token

0 commit comments

Comments
 (0)