Skip to content

Commit

Permalink
Always enforce HttpVersionPolicy on WebSocket version negotiation (#2729
Browse files Browse the repository at this point in the history 
)

* Always enforce HttpVersionPolicy on WebSocket version negotiation

* Fix Mac tests

* Improve the error message for SPDY

* Add test for SPDY when disallowed by version policy
  • Loading branch information
@MihaZupan
MihaZupan authored Feb 6, 2025
1 parent 87d8308 commit 41e563c
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 23 deletions.
27 changes: 21 additions & 6 deletions src/ReverseProxy/Forwarder/HttpForwarder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ public async ValueTask<ForwarderError> SendAsync(
// Trying again
activityCancellationSource.ResetTimeout();

Debug.Assert(requestConfig?.VersionPolicy is null or HttpVersionPolicy.RequestVersionOrLower || requestConfig.Version?.Major is null or 1,
"HTTP/1.X was disallowed by policy, we shouldn't be retrying.");

var config = requestConfig! with
{
Version = HttpVersion.Version11,
Expand Down Expand Up @@ -361,15 +364,16 @@ public async ValueTask<ForwarderError> SendAsync(
&& string.Equals(WebSocketName, connectProtocol, StringComparison.OrdinalIgnoreCase);
#endif

var outgoingHttps = destinationPrefix.StartsWith("https://");
var outgoingHttps = destinationPrefix.StartsWith("https://", StringComparison.OrdinalIgnoreCase);
var outgoingVersion = requestConfig?.Version ?? DefaultVersion;
var outgoingPolicy = requestConfig?.VersionPolicy ?? DefaultVersionPolicy;
var outgoingUpgrade = false;
var outgoingConnect = false;
var tryDowngradingH2WsOnFailure = false;

if (isSpdyRequest)
{
// Can only be done on HTTP/1.1, force regardless of options.
// Can only be done on HTTP/1.1.
outgoingUpgrade = true;
}
else if (isH1WsRequest || isH2WsRequest)
Expand All @@ -378,9 +382,10 @@ public async ValueTask<ForwarderError> SendAsync(
{
#if NET7_0_OR_GREATER
case (2, HttpVersionPolicy.RequestVersionExact, _):
case (2, HttpVersionPolicy.RequestVersionOrHigher, true):
case (2, HttpVersionPolicy.RequestVersionOrHigher, _):
outgoingConnect = true;
break;

case (1, HttpVersionPolicy.RequestVersionOrHigher, true):
case (2, HttpVersionPolicy.RequestVersionOrLower, true):
case (3, HttpVersionPolicy.RequestVersionOrLower, true):
Expand All @@ -389,18 +394,26 @@ public async ValueTask<ForwarderError> SendAsync(
tryDowngradingH2WsOnFailure = true;
break;
#endif
// 1.x Lower or Exact, regardless of HTTPS
// Anything else without HTTPS except 2 Exact

default:
// Override to use HTTP/1.1, nothing else is supported.
outgoingUpgrade = true;
break;
}
}

bool http1IsAllowed = outgoingPolicy == HttpVersionPolicy.RequestVersionOrLower || outgoingVersion.Major == 1;

if (outgoingUpgrade)
{
// Can only be done on HTTP/1.1, force regardless of options.
// Can only be done on HTTP/1.1, throw if disallowed by options.
if (!http1IsAllowed)
{
throw new HttpRequestException(isSpdyRequest
? "SPDY requests require HTTP/1.1 support, but outbound HTTP/1.1 was disallowed by HttpVersionPolicy."
: "An outgoing HTTP/1.1 Upgrade request is required to proxy this request, but is disallowed by HttpVersionPolicy.");
}

destinationRequest.Version = HttpVersion.Version11;
destinationRequest.VersionPolicy = HttpVersionPolicy.RequestVersionOrLower;
destinationRequest.Method = HttpMethod.Get;
Expand All @@ -413,10 +426,12 @@ public async ValueTask<ForwarderError> SendAsync(
destinationRequest.VersionPolicy = HttpVersionPolicy.RequestVersionExact;
destinationRequest.Method = HttpMethod.Connect;
destinationRequest.Headers.Protocol = connectProtocol ?? WebSocketName;
tryDowngradingH2WsOnFailure &= http1IsAllowed;
}
#endif
else
{
Debug.Assert(http1IsAllowed || outgoingVersion.Major != 1);
destinationRequest.Method = RequestUtilities.GetHttpMethod(context.Request.Method);
destinationRequest.Version = outgoingVersion;
destinationRequest.VersionPolicy = outgoingPolicy;
Expand Down
8 changes: 4 additions & 4 deletions test/ReverseProxy.FunctionalTests/Common/TestEnvironment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ public class TestEnvironment

public Func<ClusterConfig, RouteConfig, (ClusterConfig Cluster, RouteConfig Route)> ConfigTransformer { get; set; } = (a, b) => (a, b);

public Version DestionationHttpVersion { get; set; }
public Version DestinationHttpVersion { get; set; }

public HttpVersionPolicy? DestionationHttpVersionPolicy { get; set; }
public HttpVersionPolicy? DestinationHttpVersionPolicy { get; set; }

public HttpProtocols DestinationProtocol { get; set; } = HttpProtocols.Http1AndHttp2;

Expand Down Expand Up @@ -117,8 +117,8 @@ public IHost CreateProxy(string destinationAddress)
},
HttpRequest = new Forwarder.ForwarderRequestConfig
{
Version = DestionationHttpVersion,
VersionPolicy = DestionationHttpVersionPolicy,
Version = DestinationHttpVersion,
VersionPolicy = DestinationHttpVersionPolicy,
}
};
(cluster, route) = ConfigTransformer(cluster, route);
Expand Down
166 changes: 156 additions & 10 deletions test/ReverseProxy.FunctionalTests/WebSocketTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Net;
Expand Down Expand Up @@ -35,6 +36,147 @@ public WebSocketTests(ITestOutputHelper output)
_output = output;
}

#if NET7_0_OR_GREATER
public static IEnumerable<object[]> WebSocketVersionNegotiation_TestData()
{
foreach (Version incomingVersion in new[] { HttpVersion.Version11, HttpVersion.Version20 })
{
foreach (HttpVersionPolicy versionPolicy in Enum.GetValues<HttpVersionPolicy>())
{
foreach (Version destinationVersion in new[] { HttpVersion.Version11, HttpVersion.Version20, HttpVersion.Version30 })
{
foreach (HttpProtocols destinationProtocols in new[] { HttpProtocols.Http1, HttpProtocols.Http2, HttpProtocols.Http1AndHttp2 })
{
foreach (bool useHttpsOnDestination in new[] { true, false })
{
(int version, bool canDowngrade) = (destinationVersion.Major, versionPolicy, useHttpsOnDestination) switch
{
(1, HttpVersionPolicy.RequestVersionOrHigher, true) => (2, true),
(1, _, _) => (1, false),
(2, HttpVersionPolicy.RequestVersionOrLower, true) => (2, true),
(2, HttpVersionPolicy.RequestVersionOrLower, false) => (1, false),
(2, _, _) => (2, false),
(3, HttpVersionPolicy.RequestVersionOrLower, true) => (2, true),
(3, HttpVersionPolicy.RequestVersionOrLower, false) => (1, false),
(3, _, _) => (-1, false), // RequestCreation error
_ => throw new Exception()
};

ForwarderError? expectedProxyError = version == -1 ? ForwarderError.RequestCreation : null;
bool e2eWillFail = expectedProxyError.HasValue;

if (version == 2 && destinationProtocols == HttpProtocols.Http1)
{
// ALPN rejects HTTP/2.
if (canDowngrade)
{
Debug.Assert(useHttpsOnDestination);
version = 1;
}
else
{
e2eWillFail = true;
expectedProxyError = ForwarderError.Request;
}
}

if (version == 1 && destinationProtocols == HttpProtocols.Http2)
{
// ALPN rejects HTTP/1.1, or the server sends back an error response when not using TLS.
e2eWillFail = true;

// An error response is just a bad status code, not a failed request from the proxy's perspective.
if (useHttpsOnDestination)
{
expectedProxyError = ForwarderError.Request;
}
}

if (version == 2 && destinationProtocols == HttpProtocols.Http1AndHttp2 && !useHttpsOnDestination)
{
// No ALPN, Kestrel doesn't know whether to use HTTP/1.1 or HTTP/2, defaulting to HTTP/1.1.
// YARP will see an 'HTTP_1_1_REQUIRED' error and return a 502.
Debug.Assert(!canDowngrade);
e2eWillFail = true;
expectedProxyError = ForwarderError.Request;
}

string expectedVersion = version == 1 ? "HTTP/1.1" : "HTTP/2";

#if NET7_0
if (version == 2 && destinationProtocols is HttpProtocols.Http1 or HttpProtocols.Http1AndHttp2 && !useHttpsOnDestination)
{
// https://github.com/dotnet/runtime/issues/80056
continue;
}
#endif

yield return new object[] { incomingVersion, versionPolicy, destinationVersion, destinationProtocols, useHttpsOnDestination, expectedVersion, expectedProxyError, e2eWillFail };
}
}
}
}
}
}

[Theory]
[MemberData(nameof(WebSocketVersionNegotiation_TestData))]
public async Task WebSocketVersionNegotiation(Version incomingVersion, HttpVersionPolicy versionPolicy, Version requestedDestinationVersion, HttpProtocols destinationProtocols, bool useHttpsOnDestination,
string expectedVersion, ForwarderError? expectedProxyError, bool e2eWillFail)
{
#if !NET8_0_OR_GREATER
if (OperatingSystem.IsMacOS() && useHttpsOnDestination && destinationProtocols != HttpProtocols.Http1)
{
// Does not support ALPN until .NET 8
return;
}
#endif

using var cts = CreateTimer();

var test = CreateTestEnvironment();
test.ProxyProtocol = incomingVersion.Major == 1 ? HttpProtocols.Http1 : HttpProtocols.Http2;
test.DestinationProtocol = destinationProtocols;
test.DestinationHttpVersion = requestedDestinationVersion;
test.DestinationHttpVersionPolicy = versionPolicy;
test.UseHttpsOnDestination = useHttpsOnDestination;

int proxyRequests = 0;
ForwarderError? error = null;

test.ConfigureProxyApp = builder =>
{
builder.Use(async (context, next) =>
{
proxyRequests++;
await next(context);

error = context.Features.Get<IForwarderErrorFeature>()?.Error;
});
};

await test.Invoke(async uri =>
{
using var client = new ClientWebSocket();
client.Options.HttpVersion = incomingVersion;
client.Options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;

if (e2eWillFail)
{
var ex = await Assert.ThrowsAsync<WebSocketException>(() => SendWebSocketRequestAsync(client, uri, expectedVersion, cts.Token));
Assert.IsNotType<TaskCanceledException>(ex.InnerException);
}
else
{
await SendWebSocketRequestAsync(client, uri, expectedVersion, cts.Token);
}
}, cts.Token);

Assert.Equal(1, proxyRequests);
Assert.Equal(expectedProxyError, error);
}
#endif

[Theory]
[InlineData(WebSocketMessageType.Binary)]
[InlineData(WebSocketMessageType.Text)]
Expand Down Expand Up @@ -148,7 +290,7 @@ public async Task WebSocket11_To_11(bool useHttps)
var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http1;
test.DestinationProtocol = HttpProtocols.Http1;
test.DestionationHttpVersion = HttpVersion.Version11;
test.DestinationHttpVersion = HttpVersion.Version11;
test.UseHttpsOnProxy = useHttps;
test.UseHttpsOnDestination = useHttps;

Expand All @@ -165,18 +307,20 @@ await test.Invoke(async uri =>
[InlineData(false)]
public async Task WebSocket20_To_20(bool useHttps)
{
#if !NET8_0_OR_GREATER
if (OperatingSystem.IsMacOS() && useHttps)
{
// Does not support ALPN until .NET 8
return;
}
#endif

using var cts = CreateTimer();

var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http2;
test.DestinationProtocol = HttpProtocols.Http2;
test.DestionationHttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;
test.DestinationHttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;
test.UseHttpsOnProxy = useHttps;
test.UseHttpsOnDestination = useHttps;

Expand All @@ -194,18 +338,20 @@ await test.Invoke(async uri =>
[InlineData(false)]
public async Task WebSocket20_To_11(bool useHttps)
{
#if !NET8_0_OR_GREATER
if (OperatingSystem.IsMacOS() && useHttps)
{
// Does not support ALPN until .NET 8
return;
}
#endif

using var cts = CreateTimer();

var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http2;
test.DestinationProtocol = HttpProtocols.Http1;
test.DestionationHttpVersion = HttpVersion.Version11;
test.DestinationHttpVersion = HttpVersion.Version11;
test.UseHttpsOnProxy = useHttps;
test.UseHttpsOnDestination = useHttps;

Expand All @@ -223,18 +369,20 @@ await test.Invoke(async uri =>
[InlineData(false)]
public async Task WebSocket11_To_20(bool useHttps)
{
#if !NET8_0_OR_GREATER
if (OperatingSystem.IsMacOS() && useHttps)
{
// Does not support ALPN until .NET 8
return;
}
#endif

using var cts = CreateTimer();

var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http1;
test.DestinationProtocol = HttpProtocols.Http2;
test.DestionationHttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;
test.DestinationHttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;
test.UseHttpsOnProxy = useHttps;
test.UseHttpsOnDestination = useHttps;

Expand All @@ -256,7 +404,7 @@ public async Task WebSocketFallbackFromH2()
test.ProxyProtocol = HttpProtocols.Http1;
// The destination doesn't support HTTP/2, as determined by ALPN
test.DestinationProtocol = HttpProtocols.Http1;
test.DestionationHttpVersion = HttpVersion.Version20;
test.DestinationHttpVersion = HttpVersion.Version20;
test.UseHttpsOnDestination = true;

await test.Invoke(async uri =>
Expand All @@ -279,7 +427,7 @@ public async Task WebSocketFallbackFromH2_FailureInSecondRequestTransform_Treate
ProxyProtocol = HttpProtocols.Http1,
// The destination doesn't support HTTP/2, as determined by ALPN
DestinationProtocol = HttpProtocols.Http1,
DestionationHttpVersion = HttpVersion.Version20,
DestinationHttpVersion = HttpVersion.Version20,
UseHttpsOnDestination = true,
ConfigureProxy = builder =>
{
Expand Down Expand Up @@ -359,8 +507,8 @@ public async Task WebSocketCantFallbackFromH2(HttpVersionPolicy policy, bool use
var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http1;
test.DestinationProtocol = HttpProtocols.Http1;
test.DestionationHttpVersion = HttpVersion.Version20;
test.DestionationHttpVersionPolicy = policy;
test.DestinationHttpVersion = HttpVersion.Version20;
test.DestinationHttpVersionPolicy = policy;
test.UseHttpsOnDestination = useHttps;

await test.Invoke(async uri =>
Expand Down Expand Up @@ -392,8 +540,6 @@ public async Task InvalidKeyHeader_400(HttpProtocols destinationProtocol)
var test = CreateTestEnvironment();
test.ProxyProtocol = HttpProtocols.Http1;
test.DestinationProtocol = destinationProtocol;
test.DestionationHttpVersion = HttpVersion.Version20;
test.DestionationHttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;

test.ConfigureProxyApp = builder =>
{
Expand Down
Loading

0 comments on commit 41e563c

Please sign in to comment.