Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always enforce HttpVersionPolicy on WebSocket version negotiation #2729

Merged
merged 4 commits into from
Feb 6, 2025
Merged

Always enforce HttpVersionPolicy on WebSocket version negotiation #2729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64f2f9c
Always enforce HttpVersionPolicy on WebSocket version negotiation
MihaZupan Jan 28, 2025
33bff40
Fix Mac tests
MihaZupan Jan 28, 2025
93d0a04
Improve the error message for SPDY
MihaZupan Jan 28, 2025
5ac0681
Add test for SPDY when disallowed by version policy
MihaZupan Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/ReverseProxy/Forwarder/HttpForwarder.cs
View file
Open in desktop
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.
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed 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
Loading