From 439487a007d3a8e884e80f174e3f913caa2c5c89 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Tue, 15 Aug 2023 21:48:49 +0000 Subject: [PATCH 1/2] Merged PR 32920: limit AIA download size This prevents using unlimited resources from evil sources. I originally wanted to split limits and have them separately for certificates, OCSP and CRLs. However, the HttpClient.MaxResponseContentBufferSize can be set only once so I decided to keep it simple for servicing. We could split the HttpClient and have one for small and one for large downloads. Or alternatively we can handle the body directly. But it is going to be unpleseant with the reflection and sync & async flavors. For now, this should plug the gap and we can improve it more in future. --- .../src/System/Net/Http/X509ResourceClient.cs | 26 +++++++++++- .../RevocationTests/AiaTests.cs | 40 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs b/src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs index 57538487800e1e..74e84bb750386a 100644 --- a/src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs +++ b/src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs @@ -12,6 +12,9 @@ namespace System.Net.Http { internal static partial class X509ResourceClient { + private const long DefaultAiaDownloadLimit = 100 * 1024 * 1024; + private static long AiaDownloadLimit { get; } = GetValue("System.Security.Cryptography.AiaDownloadLimit", DefaultAiaDownloadLimit); + private static readonly Func>? s_downloadBytes = CreateDownloadBytesFunc(); static partial void ReportNoClient(); @@ -111,6 +114,7 @@ internal static partial class X509ResourceClient ConstructorInfo? httpRequestMessageCtor = httpRequestMessageType.GetConstructor(Type.EmptyTypes); MethodInfo? sendMethod = httpClientType.GetMethod("Send", new Type[] { httpRequestMessageType, typeof(CancellationToken) }); MethodInfo? sendAsyncMethod = httpClientType.GetMethod("SendAsync", new Type[] { httpRequestMessageType, typeof(CancellationToken) }); + PropertyInfo? maxResponseContentBufferSizeProp = httpClientType.GetProperty("MaxResponseContentBufferSize"); PropertyInfo? responseContentProp = httpResponseMessageType.GetProperty("Content"); PropertyInfo? responseStatusCodeProp = httpResponseMessageType.GetProperty("StatusCode"); PropertyInfo? responseHeadersProp = httpResponseMessageType.GetProperty("Headers"); @@ -121,7 +125,7 @@ internal static partial class X509ResourceClient if (socketsHttpHandlerCtor == null || pooledConnectionIdleTimeoutProp == null || allowAutoRedirectProp == null || httpClientCtor == null || requestUriProp == null || httpRequestMessageCtor == null || - sendMethod == null || sendAsyncMethod == null || + sendMethod == null || sendAsyncMethod == null || maxResponseContentBufferSizeProp == null || responseContentProp == null || responseStatusCodeProp == null || responseHeadersProp == null || responseHeadersLocationProp == null || readAsStreamMethod == null || taskOfHttpResponseMessageResultProp == null) @@ -145,6 +149,7 @@ internal static partial class X509ResourceClient pooledConnectionIdleTimeoutProp.SetValue(socketsHttpHandler, TimeSpan.FromSeconds(PooledConnectionIdleTimeoutSeconds)); allowAutoRedirectProp.SetValue(socketsHttpHandler, false); object? httpClient = httpClientCtor.Invoke(new object?[] { socketsHttpHandler }); + maxResponseContentBufferSizeProp.SetValue(httpClient, AiaDownloadLimit); return async (string uriString, CancellationToken cancellationToken, bool async) => { @@ -302,5 +307,24 @@ private static bool IsAllowedScheme(string scheme) { return string.Equals(scheme, "http", StringComparison.OrdinalIgnoreCase); } + + private static long GetValue(string name, long defaultValue) + { + object? data = AppContext.GetData(name); + + if (data is null) + { + return defaultValue; + } + + try + { + return Convert.ToInt64(data); + } + catch + { + return defaultValue; + } + } } } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/AiaTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/AiaTests.cs index a1ce84ce5be8e8..5cceafd0e81108 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/AiaTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/AiaTests.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Security.Cryptography.X509Certificates.Tests.Common; +using Microsoft.DotNet.RemoteExecutor; using Test.Cryptography; using Xunit; @@ -178,5 +179,44 @@ public static void DisableAiaOptionWorks() }); } } + + [ActiveIssue("https://github.com/dotnet/runtime/issues/57506", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsMariner))] + [PlatformSpecific(TestPlatforms.Linux)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void AiaIgnoresCertOverLimit() + { + RemoteExecutor.Invoke(() => + { + AppContext.SetData("System.Security.Cryptography.AiaDownloadLimit", 100); + CertificateAuthority.BuildPrivatePki( + PkiOptions.AllRevocation, + out RevocationResponder responder, + out CertificateAuthority root, + out CertificateAuthority intermediate, + out X509Certificate2 endEntity, + pkiOptionsInSubject: false, + testName: Guid.NewGuid().ToString()); + + using (responder) + using (root) + using (intermediate) + using (endEntity) + using (X509Certificate2 rootCert = root.CloneIssuerCert()) + { + responder.AiaResponseKind = AiaResponseKind.Cert; + + using (ChainHolder holder = new ChainHolder()) + { + X509Chain chain = holder.Chain; + chain.ChainPolicy.CustomTrustStore.Add(rootCert); + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chain.ChainPolicy.VerificationTime = endEntity.NotBefore.AddMinutes(1); + chain.ChainPolicy.UrlRetrievalTimeout = DynamicRevocationTests.s_urlRetrievalLimit; + + Assert.False(chain.Build(endEntity)); + } + } + }).Dispose(); + } } } From 543e733ea3d0a89372aa6b68bcd09a2b65df1864 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Wed, 16 Aug 2023 18:06:27 +0000 Subject: [PATCH 2/2] Update Microsoft.DiaSymReader.Native to 16.11.29-beta1.23404.4 --- eng/Versions.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Versions.props b/eng/Versions.props index ff21c1ee1f07dc..82d85ceaaaada5 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -161,7 +161,7 @@ 1.0.0-prerelease.23362.5 1.0.0-prerelease.23362.5 - 16.11.27-beta1.23180.1 + 16.11.29-beta1.23404.4 2.0.0-beta4.23307.1 3.0.3 2.1.0