Skip to content

Commit

Permalink
Merged PR 32920: limit AIA download size
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Tomas Weinfurt authored and carlossanlop committed Aug 15, 2023
1 parent 9607d25 commit 439487a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, CancellationToken, bool, Task<byte[]?>>? s_downloadBytes = CreateDownloadBytesFunc();

static partial void ReportNoClient();
Expand Down Expand Up @@ -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");
Expand All @@ -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)
Expand All @@ -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) =>
{
Expand Down Expand Up @@ -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;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Linq;
using System.Security.Cryptography.X509Certificates.Tests.Common;
using Microsoft.DotNet.RemoteExecutor;
using Test.Cryptography;
using Xunit;

Expand Down Expand Up @@ -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();
}
}
}

0 comments on commit 439487a

Please sign in to comment.