Skip to content

Commit

Permalink
Obsolete X509Certificate{2} constructors and X509Certificate2Collecti…
Browse files Browse the repository at this point in the history
…on.Import
  • Loading branch information
vcsjones authored Jul 5, 2024
1 parent b9673cb commit 02bd1da
Show file tree
Hide file tree
Showing 38 changed files with 141 additions and 35 deletions.
1 change: 1 addition & 0 deletions docs/project/list-of-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ The PR that reveals the implementation of the `<IncludeInternalObsoleteAttribute
| __`SYSLIB0053`__ | AesGcm should indicate the required tag size for encryption and decryption. Use a constructor that accepts the tag size. |
| __`SYSLIB0054`__ | Thread.VolatileRead and Thread.VolatileWrite are obsolete. Use Volatile.Read or Volatile.Write respectively instead. |
| __`SYSLIB0055`__ | The underlying hardware instruction does not perform a signed saturate narrowing operation, and it always returns an unsigned result. Use the unsigned overload instead. |
| __`SYSLIB0057`__ | Loading certificate data through the constructor or Import is obsolete. Use X509CertificateLoader instead to load certificates. |

## Analyzer Warnings

Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Common/src/System/Obsoletions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,5 +180,8 @@ internal static class Obsoletions

internal const string LoadFromHashAlgorithmMessage = "LoadFrom with a custom AssemblyHashAlgorithm is obsolete. Use overloads without an AssemblyHashAlgorithm.";
internal const string LoadFromHashAlgorithmDiagId = "SYSLIB0056";

internal const string X509CtorCertDataObsoleteMessage = "Loading certificate data through the constructor or Import is obsolete. Use X509CertificateLoader instead to load certificates.";
internal const string X509CtorCertDataObsoleteDiagId = "SYSLIB0057";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,8 @@
<PackageReference Include="System.Security.Principal.Windows" Version="$(SystemSecurityPrincipalWindowsVersion)" />
</ItemGroup>

<ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))">
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Bcl.Cryptography\src\Microsoft.Bcl.Cryptography.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,8 @@ private void LoadCertificateCollection(List<byte[]> certificatesToLoad)
{
try
{
_certificates.Import(rawCert);
X509Certificate2 cert = X509CertificateLoader.LoadCertificate(rawCert);
_certificates.Add(cert);
}
catch (System.Security.Cryptography.CryptographicException)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkCurrent)</TargetFrameworks>
<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<DefineConstants>$(DefineConstants);WINHTTPHANDLER_TEST</DefineConstants>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Net\Configuration.cs"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<NoWarn>$(NoWarn);0436</NoWarn>
<NoWarn>$(NoWarn);0436;SYSLIB0057</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<StringResourcesPath>../../src/Resources/Strings.resx</StringResourcesPath>
<TargetFramework>$(NetCoreAppCurrent)-windows</TargetFramework>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-linux;$(NetCoreAppCurrent)-android;$(NetCoreAppCurrent)-browser;$(NetCoreAppCurrent)-osx</TargetFrameworks>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
<EventSourceSupport Condition="'$(TestNativeAot)' == 'true'">true</EventSourceSupport>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public HttpConnection(Socket sock, HttpEndPointListener epl, bool secure, X509Ce
return true;
}

_clientCert = c as X509Certificate2 ?? new X509Certificate2(c.GetRawCertData());
_clientCert = c as X509Certificate2 ?? X509CertificateLoader.LoadCertificate(c.GetRawCertData());
_clientCertErrors = new int[] { (int)e };
return true;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ private void GetClientCertificateCore()
{
byte[] certEncoded = new byte[pClientCertInfo->CertEncodedSize];
Marshal.Copy((IntPtr)pClientCertInfo->pCertEncoded, certEncoded, 0, certEncoded.Length);
ClientCertificate = new X509Certificate2(certEncoded);
ClientCertificate = X509CertificateLoader.LoadCertificate(certEncoded);
}
catch (CryptographicException exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private static unsafe void IOCompleted(ListenerClientCertAsyncResult asyncResult
{
byte[] certEncoded = new byte[pClientCertInfo->CertEncodedSize];
Marshal.Copy((IntPtr)pClientCertInfo->pCertEncoded, certEncoded, 0, certEncoded.Length);
result = httpListenerRequest.ClientCertificate = new X509Certificate2(certEncoded);
result = httpListenerRequest.ClientCertificate = X509CertificateLoader.LoadCertificate(certEncoded);
}
catch (CryptographicException exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
<DefineConstants>$(DefineConstants);NETWORKINFORMATION_TEST</DefineConstants>
<EventSourceSupport Condition="'$(TestNativeAot)' == 'true'">true</EventSourceSupport>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<Compile Include="AssemblyInfo.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ internal async Task<bool> StartAsyncCertificateValidation(IntPtr certificatePtr,
if (certData.Length > 0)
{
Debug.Assert(certificate == null);
certificate = new X509Certificate2(certData.Span);
certificate = X509CertificateLoader.LoadCertificate(certData.Span);
}

result = _connection._sslConnectionOptions.ValidateCertificate(certificate, certData.Span, chainData.Span);
Expand Down Expand Up @@ -205,8 +205,11 @@ private QUIC_TLS_ALERT_CODES ValidateCertificate(X509Certificate2? certificate,

if (chainData.Length > 0)
{
Debug.Assert(X509Certificate2.GetCertContentType(chainData) is X509ContentType.Pkcs7);
X509Certificate2Collection additionalCertificates = new X509Certificate2Collection();
#pragma warning disable SYSLIB0057
additionalCertificates.Import(chainData);
#pragma warning restore SYSLIB0057
chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-linux;$(NetCoreAppCurrent)-osx</TargetFrameworks>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
<StringResourcesPath>../../src/Resources/Strings.resx</StringResourcesPath>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<RdXmlFile Include="default.rd.xml" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<DefineConstants>$(DefineConstants);NETSTANDARD</DefineConstants>
<IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
<!-- SYSLIB0014: WebRequest, HttpWebRequest, ServicePoint, and WebClient are obsolete. Use HttpClient instead. -->
<NoWarn>$(NoWarn);SYSLIB0014</NoWarn>
<NoWarn>$(NoWarn);SYSLIB0014;SYSLIB0057</NoWarn>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
<EventSourceSupport Condition="'$(TestNativeAot)' == 'true'">true</EventSourceSupport>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ internal static void SetCertificate(SafeSslHandle sslContext, SslStreamCertifica
// The current value of intermediateCert is still in elements, which will
// get Disposed at the end of this method. The new value will be
// in the intermediate certs array, which also gets serially Disposed.
intermediateCert = new X509Certificate2(intermediateCert.RawDataMemory.Span);
intermediateCert = X509CertificateLoader.LoadCertificate(intermediateCert.RawDataMemory.Span);
}

ptrs[i + 1] = intermediateCert.Handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
<EventSourceSupport Condition="'$(TestNativeAot)' == 'true'">true</EventSourceSupport>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<Compile Include="AssemblyInfo.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
-->
<NoWarn>436</NoWarn>
<!-- Disable: CLSCompliant attribute is not needed -->
<NoWarn>$(NoWarn);3021</NoWarn>
<NoWarn>$(NoWarn);3021;SYSLIB0057</NoWarn>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-unix;$(NetCoreAppCurrent)-browser;$(NetCoreAppCurrent)-osx;$(NetCoreAppCurrent)-ios;$(NetCoreAppCurrent)-android</TargetFrameworks>
<IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<DefineConstants>$(DefineConstants);NETSTANDARD</DefineConstants>
<!-- SYSLIB0014: WebRequest, HttpWebRequest, ServicePoint, and WebClient are obsolete. Use HttpClient instead. -->
<NoWarn>$(NoWarn);SYSLIB0014</NoWarn>
<NoWarn>$(NoWarn);SYSLIB0014;SYSLIB0057</NoWarn>
<IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
</PropertyGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<StringResourcesPath>../src/Resources/Strings.resx</StringResourcesPath>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-browser</TargetFrameworks>
<DefineConstants>$(DefineConstants);NETSTANDARD</DefineConstants>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetOS)' == 'browser'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public override DecryptorPal Decode(
{
if (certChoice.Certificate != null)
{
originatorCerts.Add(new X509Certificate2(certChoice.Certificate.Value.ToArray()));
originatorCerts.Add(X509CertificateLoader.LoadCertificate(certChoice.Certificate.Value.Span));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public static X509Certificate2Collection GetOriginatorCerts(this SafeCryptMsgHan
for (int index = 0; index < numCertificates; index++)
{
byte[] encodedCertificate = hCryptMsg.GetMsgParamAsByteArray(CryptMsgParamType.CMSG_CERT_PARAM, index);
X509Certificate2 cert = new X509Certificate2(encodedCertificate);
X509Certificate2 cert = X509CertificateLoader.LoadCertificate(encodedCertificate);
certs.Add(cert);
}
return certs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@ System.Security.Cryptography.Pkcs.EnvelopedCms</PackageDescription>
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
</ItemGroup>

<ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))">
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Bcl.Cryptography\src\Microsoft.Bcl.Cryptography.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
<Reference Include="System.Security" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public X509Certificate2 GetCertificate()
throw new InvalidOperationException(SR.Cryptography_Pkcs12_CertBagNotX509);
}

return new X509Certificate2(PkcsHelpers.DecodeOctetString(_decoded.CertValue));
return X509CertificateLoader.LoadCertificate(PkcsHelpers.DecodeOctetString(_decoded.CertValue));
}

private static byte[] EncodeBagValue(Oid certificateType, ReadOnlyMemory<byte> encodedCertificate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,7 @@ public X509Certificate2Collection Certificates
{
if (choice.Certificate.HasValue)
{
coll.Add(new X509Certificate2(choice.Certificate.Value
#if NET
.Span
#else
.ToArray()
#endif
));
coll.Add(X509CertificateLoader.LoadCertificate(choice.Certificate.Value.Span));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent);$(NetFrameworkCurrent)</TargetFrameworks>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Security\Cryptography\ByteUtils.cs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ System.Security.Cryptography.Xml.XmlLicenseTransform</PackageDescription>
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
</ItemGroup>

<ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))">
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Bcl.Cryptography\src\Microsoft.Bcl.Cryptography.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(IsPartialFacadeAssembly)' == 'true'">
<Reference Include="System.Security" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ public KeyInfoX509Data() { }

public KeyInfoX509Data(byte[] rgbCert)
{
X509Certificate2 certificate = new X509Certificate2(rgbCert);
// Compat: this accepts null arrays for certificate data and would not throw. X509CertificateLoader throws
// for a null input. This uses the X509Certificate2 constructor for null inputs to preserve the existing
// behavior. Since the input is null and there is nothing to decode, the input is safe for the constructor.
#pragma warning disable SYSLIB0057
X509Certificate2 certificate = rgbCert is null ?
new X509Certificate2((byte[])null!) :
X509CertificateLoader.LoadCertificate(rgbCert);
#pragma warning restore SYSLIB0057

AddCertificate(certificate);
}

Expand Down Expand Up @@ -316,7 +324,9 @@ public override void LoadXml(XmlElement element)

foreach (XmlNode node in x509CertificateNodes)
{
AddCertificate(new X509Certificate2(Convert.FromBase64String(Utils.DiscardWhiteSpaces(node.InnerText))));
AddCertificate(
X509CertificateLoader.LoadCertificate(
Convert.FromBase64String(Utils.DiscardWhiteSpaces(node.InnerText))));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);SYSLIB0057</NoWarn>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Security\Cryptography\PlatformSupport.cs"
Expand Down
Loading

0 comments on commit 02bd1da

Please sign in to comment.