Skip to content

Commit ad6e251

Browse files
surajitshil-03surajitshil-03
authored
Fix X509Certficate error in .NET10 (#5480)
* Fix X509Certficate issue * Checking the certificateContentType before calling the methods * Adding check for PKCS#7 certificate type * Refactoring the code * Changing the certificate selection logic in PKCS#7 * Refactoring and supporting more formats of certificates * Cleaning the common util function to handle only the supported certificate types and rest are handled by the old constructor * Adding L0 test cases for the new utility function * Resolving conflicts and code cleanup * Minor change
1 parent c765c82 commit ad6e251

File tree

6 files changed

+283
-21
lines changed

6 files changed

+283
-21
lines changed

‎src/Agent.Listener/ValidationHelper/InstallerVerifier.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,6 @@ public static void VerifyFileSignedByMicrosoft(string filePath, Tracing trace, s
111111
CRYPT_PROVIDER_CERT provCert = (CRYPT_PROVIDER_CERT)Marshal.PtrToStructure(pProviderCertificate, typeof(CRYPT_PROVIDER_CERT));
112112

113113
// Check for our EKU in the certificate
114-
// Disable the warning. TODO: Remove this warning suppression after the code is refactored to use X509CertificateLoader instead.
115-
#pragma warning disable SYSLIB0057
116114
using (X509Certificate2 x509Cert = new X509Certificate2(provCert.pCert))
117115
{
118116
if (((X509EnhancedKeyUsageExtension)x509Cert.Extensions[EXTENDED_KEY_USAGE]).EnhancedKeyUsages[expectedEKU] == null)
@@ -123,8 +121,6 @@ public static void VerifyFileSignedByMicrosoft(string filePath, Tracing trace, s
123121

124122
trace.Info(String.Format("Authenticode signature for file {0} is signed with a certificate containing the EKU {1}.", filePath, expectedEKU));
125123
}
126-
// Re-enable the warning.
127-
#pragma warning restore SYSLIB0057
128124
}
129125
finally
130126
{

‎src/Agent.Plugins/TFCliManager.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using Agent.Sdk;
5+
using Agent.Sdk.Util;
56
using System;
67
using System.Collections.Generic;
78
using System.IO;
@@ -158,18 +159,17 @@ public void SetupProxy(string proxyUrl, string proxyUsername, string proxyPasswo
158159
}
159160

160161
public void SetupClientCertificate(string clientCert, string clientCertKey, string clientCertArchive, string clientCertPassword)
161-
{ // Disable the warning. TODO: Remove this warning suppression after the code is refactored to use X509CertificateLoader instead.
162-
#pragma warning disable SYSLIB0057
162+
{
163163
ArgUtil.File(clientCert, nameof(clientCert));
164-
X509Certificate2 cert = new X509Certificate2(clientCert);
164+
165+
// Pass null for password to maintain original behavior (certificate without password)
166+
X509Certificate2 cert = CertificateUtil.LoadCertificate(clientCert, password: null);
167+
165168
ExecutionContext.Debug($"Set VstsClientCertificate={cert.Thumbprint} for Tf.exe to support client certificate.");
166169
AdditionalEnvironmentVariables["VstsClientCertificate"] = cert.Thumbprint;
167170

168171
// Script Tf commands in tasks
169172
ExecutionContext.SetVariable("VstsClientCertificate", cert.Thumbprint);
170-
171-
// Re-enable the warning.
172-
#pragma warning restore SYSLIB0057
173173
}
174174

175175
public async Task ShelveAsync(string shelveset, string commentFile, bool move)

‎src/Agent.Sdk/AgentClientCertificateManager.cs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using System.Security.Cryptography.X509Certificates;
5+
using Agent.Sdk.Util;
56
using Microsoft.VisualStudio.Services.Common;
67

78
namespace Agent.Sdk
@@ -35,11 +36,7 @@ public void AddClientCertificate(string clientCertificateArchiveFile, string cli
3536
{
3637
if (!string.IsNullOrEmpty(clientCertificateArchiveFile))
3738
{
38-
// Disable the warning. TODO: Remove this warning suppression after the code is refactored to use X509CertificateLoader instead.
39-
#pragma warning disable SYSLIB0057
40-
_clientCertificates.Add(new X509Certificate2(clientCertificateArchiveFile, clientCertificatePassword));
41-
// Re-enable the warning.
42-
#pragma warning restore SYSLIB0057
39+
_clientCertificates.Add(CertificateUtil.LoadCertificate(clientCertificateArchiveFile, clientCertificatePassword));
4340
}
4441
}
4542
}

‎src/Agent.Sdk/Util/CertificateUtil.cs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Security.Cryptography.X509Certificates;
5+
6+
namespace Agent.Sdk.Util
7+
{
8+
public static class CertificateUtil
9+
{
10+
/// <summary>
11+
/// Loads an X509Certificate2 from a file, handling different certificate formats.
12+
/// Uses X509CertificateLoader for .NET 9+ for Cert and Pkcs12 formats.
13+
/// For all other formats, uses the legacy constructor with warning suppression.
14+
/// </summary>
15+
/// <param name="certificatePath">Path to the certificate file</param>
16+
/// <param name="password">Optional password for PKCS#12/PFX files</param>
17+
/// <returns>The loaded X509Certificate2</returns>
18+
public static X509Certificate2 LoadCertificate(string certificatePath, string password = null)
19+
{
20+
#if NET9_0_OR_GREATER
21+
var contentType = X509Certificate2.GetCertContentType(certificatePath);
22+
switch (contentType)
23+
{
24+
case X509ContentType.Cert:
25+
// DER-encoded or PEM-encoded certificate
26+
return X509CertificateLoader.LoadCertificateFromFile(certificatePath);
27+
28+
case X509ContentType.Pkcs12:
29+
// Note: X509ContentType.Pfx has the same value (3) as Pkcs12 refer: https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.x509certificates.x509contenttype?view=net-10.0
30+
return X509CertificateLoader.LoadPkcs12FromFile(certificatePath, password);
31+
32+
default:
33+
// For all other formats (Pkcs7, SerializedCert, SerializedStore, Authenticode, Unknown),
34+
// use the legacy constructor with warning suppression
35+
#pragma warning disable SYSLIB0057
36+
if (string.IsNullOrEmpty(password))
37+
{
38+
return new X509Certificate2(certificatePath);
39+
}
40+
else
41+
{
42+
return new X509Certificate2(certificatePath, password);
43+
}
44+
#pragma warning restore SYSLIB0057
45+
}
46+
#else
47+
// For .NET 8 and earlier, use the traditional constructor
48+
// The constructor automatically handles all certificate types
49+
if (string.IsNullOrEmpty(password))
50+
{
51+
return new X509Certificate2(certificatePath);
52+
}
53+
else
54+
{
55+
return new X509Certificate2(certificatePath, password);
56+
}
57+
#endif
58+
}
59+
}
60+
}

‎src/Agent.Worker/Build/TFCommandManager.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using Agent.Sdk.Util;
45
using Microsoft.VisualStudio.Services.Agent.Util;
56
using System;
67
using System.Collections.Generic;
@@ -156,18 +157,16 @@ public void SetupProxy(string proxyUrl, string proxyUsername, string proxyPasswo
156157

157158
public void SetupClientCertificate(string clientCert, string clientCertKey, string clientCertArchive, string clientCertPassword)
158159
{
159-
// Disable the warning. TODO: Remove this warning suppression after the code is refactored to use X509CertificateLoader instead.
160-
#pragma warning disable SYSLIB0057
161160
ArgUtil.File(clientCert, nameof(clientCert));
162-
X509Certificate2 cert = new X509Certificate2(clientCert);
161+
162+
// Pass null for password to maintain original behavior (certificate without password)
163+
X509Certificate2 cert = CertificateUtil.LoadCertificate(clientCert, password: null);
164+
163165
ExecutionContext.Debug($"Set VstsClientCertificate={cert.Thumbprint} for Tf.exe to support client certificate.");
164166
AdditionalEnvironmentVariables["VstsClientCertificate"] = cert.Thumbprint;
165167

166168
// Script Tf commands in tasks
167169
ExecutionContext.SetVariable("VstsClientCertificate", cert.Thumbprint, false, false);
168-
169-
// Re-enable the warning.
170-
#pragma warning restore SYSLIB0057
171170
}
172171

173172
public async Task ShelveAsync(string shelveset, string commentFile, bool move)

‎src/Test/L0/Util/CertificateUtilL0.cs‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.IO;
6+
using System.Security.Cryptography;
7+
using System.Security.Cryptography.X509Certificates;
8+
using Agent.Sdk.Util;
9+
using Xunit;
10+
11+
namespace Microsoft.VisualStudio.Services.Agent.Tests.Util
12+
{
13+
/// <summary>
14+
/// Tests for CertificateUtil.LoadCertificate which works on both .NET 8 and .NET 10.
15+
/// Tests cover: Cert (DER/PEM) and PFX/PKCS#12 formats.
16+
/// </summary>
17+
public sealed class CertificateUtilL0 : IDisposable
18+
{
19+
private readonly string _tempDir;
20+
21+
public CertificateUtilL0()
22+
{
23+
_tempDir = Path.Combine(Path.GetTempPath(), $"CertUtilTests_{Guid.NewGuid():N}");
24+
Directory.CreateDirectory(_tempDir);
25+
}
26+
27+
public void Dispose()
28+
{
29+
if (Directory.Exists(_tempDir))
30+
{
31+
Directory.Delete(_tempDir, recursive: true);
32+
}
33+
}
34+
35+
#region PFX/PKCS#12 Tests (X509ContentType.Pkcs12)
36+
37+
[Fact]
38+
[Trait("Level", "L0")]
39+
[Trait("Category", "Common")]
40+
public void LoadCertificate_Pfx_WithPassword_LoadsSuccessfully()
41+
{
42+
// Arrange
43+
var (expectedThumbprint, pfxPath) = CreatePfxCertificate("test-password");
44+
45+
// Act
46+
using var loadedCert = CertificateUtil.LoadCertificate(pfxPath, "test-password");
47+
48+
// Assert
49+
Assert.NotNull(loadedCert);
50+
Assert.Equal(expectedThumbprint, loadedCert.Thumbprint);
51+
}
52+
53+
[Fact]
54+
[Trait("Level", "L0")]
55+
[Trait("Category", "Common")]
56+
public void LoadCertificate_Pfx_WithoutPassword_LoadsSuccessfully()
57+
{
58+
// Arrange
59+
var (expectedThumbprint, pfxPath) = CreatePfxCertificate(password: null);
60+
61+
// Act
62+
using var loadedCert = CertificateUtil.LoadCertificate(pfxPath, password: null);
63+
64+
// Assert
65+
Assert.NotNull(loadedCert);
66+
Assert.Equal(expectedThumbprint, loadedCert.Thumbprint);
67+
}
68+
69+
[Fact]
70+
[Trait("Level", "L0")]
71+
[Trait("Category", "Common")]
72+
public void LoadCertificate_Pfx_WrongPassword_ThrowsException()
73+
{
74+
// Arrange
75+
var (_, pfxPath) = CreatePfxCertificate("correct-password");
76+
77+
// Act & Assert
78+
Assert.ThrowsAny<CryptographicException>(() =>
79+
CertificateUtil.LoadCertificate(pfxPath, "wrong-password"));
80+
}
81+
82+
[Fact]
83+
[Trait("Level", "L0")]
84+
[Trait("Category", "Common")]
85+
public void LoadCertificate_Pfx_PasswordProtectedButNoPasswordProvided_ThrowsException()
86+
{
87+
// Arrange
88+
var (_, pfxPath) = CreatePfxCertificate("some-password");
89+
90+
// Act & Assert
91+
Assert.ThrowsAny<CryptographicException>(() =>
92+
CertificateUtil.LoadCertificate(pfxPath, password: null));
93+
}
94+
95+
#endregion
96+
97+
#region DER Tests (X509ContentType.Cert)
98+
99+
[Fact]
100+
[Trait("Level", "L0")]
101+
[Trait("Category", "Common")]
102+
public void LoadCertificate_Der_LoadsSuccessfully()
103+
{
104+
// Arrange
105+
var (expectedThumbprint, derPath) = CreateDerCertificate();
106+
107+
// Act
108+
using var loadedCert = CertificateUtil.LoadCertificate(derPath);
109+
110+
// Assert
111+
Assert.NotNull(loadedCert);
112+
Assert.Equal(expectedThumbprint, loadedCert.Thumbprint);
113+
}
114+
115+
#endregion
116+
117+
#region PEM Tests (X509ContentType.Cert)
118+
119+
[Fact]
120+
[Trait("Level", "L0")]
121+
[Trait("Category", "Common")]
122+
public void LoadCertificate_Pem_LoadsSuccessfully()
123+
{
124+
// Arrange
125+
var (expectedThumbprint, pemPath) = CreatePemCertificate();
126+
127+
// Act
128+
using var loadedCert = CertificateUtil.LoadCertificate(pemPath);
129+
130+
// Assert
131+
Assert.NotNull(loadedCert);
132+
Assert.Equal(expectedThumbprint, loadedCert.Thumbprint);
133+
}
134+
135+
#endregion
136+
137+
#region Helper Methods
138+
139+
/// <summary>
140+
/// Creates a test PFX/PKCS#12 certificate file (X509ContentType.Pkcs12).
141+
/// </summary>
142+
private (string thumbprint, string path) CreatePfxCertificate(string password)
143+
{
144+
using var rsa = RSA.Create(2048);
145+
var request = new CertificateRequest(
146+
"CN=TestPfxCertificate",
147+
rsa,
148+
HashAlgorithmName.SHA256,
149+
RSASignaturePadding.Pkcs1);
150+
151+
using var cert = request.CreateSelfSigned(
152+
DateTimeOffset.UtcNow.AddMinutes(-5),
153+
DateTimeOffset.UtcNow.AddYears(1));
154+
155+
var pfxPath = Path.Combine(_tempDir, $"test_{Guid.NewGuid():N}.pfx");
156+
var pfxBytes = cert.Export(X509ContentType.Pfx, password);
157+
File.WriteAllBytes(pfxPath, pfxBytes);
158+
159+
return (cert.Thumbprint, pfxPath);
160+
}
161+
162+
/// <summary>
163+
/// Creates a test DER-encoded certificate file (X509ContentType.Cert).
164+
/// </summary>
165+
private (string thumbprint, string path) CreateDerCertificate()
166+
{
167+
using var rsa = RSA.Create(2048);
168+
var request = new CertificateRequest(
169+
"CN=TestDerCertificate",
170+
rsa,
171+
HashAlgorithmName.SHA256,
172+
RSASignaturePadding.Pkcs1);
173+
174+
using var cert = request.CreateSelfSigned(
175+
DateTimeOffset.UtcNow.AddMinutes(-5),
176+
DateTimeOffset.UtcNow.AddYears(1));
177+
178+
var derPath = Path.Combine(_tempDir, $"test_{Guid.NewGuid():N}.cer");
179+
var derBytes = cert.Export(X509ContentType.Cert);
180+
File.WriteAllBytes(derPath, derBytes);
181+
182+
return (cert.Thumbprint, derPath);
183+
}
184+
185+
/// <summary>
186+
/// Creates a test PEM-encoded certificate file.
187+
/// </summary>
188+
private (string thumbprint, string path) CreatePemCertificate()
189+
{
190+
using var rsa = RSA.Create(2048);
191+
var request = new CertificateRequest(
192+
"CN=TestPemCertificate",
193+
rsa,
194+
HashAlgorithmName.SHA256,
195+
RSASignaturePadding.Pkcs1);
196+
197+
using var cert = request.CreateSelfSigned(
198+
DateTimeOffset.UtcNow.AddMinutes(-5),
199+
DateTimeOffset.UtcNow.AddYears(1));
200+
201+
var pemPath = Path.Combine(_tempDir, $"test_{Guid.NewGuid():N}.pem");
202+
var pemContent = cert.ExportCertificatePem();
203+
File.WriteAllText(pemPath, pemContent);
204+
205+
return (cert.Thumbprint, pemPath);
206+
}
207+
208+
#endregion
209+
}
210+
}

0 commit comments

Comments
 (0)