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.

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

Conditionally copy Strict-Transport-Security #2306

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion docs/docfx/articles/header-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ services.AddReverseProxy()
.ConfigureHttpClient((_, handler) => handler.ActivityHeadersPropagator = null);
```

### Strict-Transport-Security

This header instructs clients to always use HTTPS, but there may be a conflict between values provided by the proxy and destination. To avoid confusion, the destination's value is not copied to the response if one was already added to the response by the proxy application.

## Other Header Guidelines

### Host
Expand Down Expand Up @@ -74,4 +78,3 @@ This response header indicates what server technology was used to generate the r
### X-Powered-By

This response header indicates what web framework was used to generate the response (ASP.NET, etc.). ASP.NET Core does not generate this header but IIS can. This header is proxied from the destination by default. Applications that want to remove it can use the [ResponseHeaderRemove](transforms.md#responseheaderremove) transform.

14 changes: 13 additions & 1 deletion src/ReverseProxy/Forwarder/HttpTransformer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.Primitives;
using Microsoft.Net.Http.Headers;
using Yarp.ReverseProxy.Transforms.Builder;

Expand Down Expand Up @@ -258,7 +259,18 @@ private static void CopyResponseHeaders(HttpHeaders source, IHeaderDictionary de
continue;
}

destination[headerName] = RequestUtilities.Concat(destination[headerName], header.Value);
var currentValue = destination[headerName];

// https://github.com/microsoft/reverse-proxy/issues/2269
// The Strict-Transport-Security may be added by the proxy before forwarding. Only copy the header
// if it's not already present.
if (!StringValues.IsNullOrEmpty(currentValue)
&& string.Equals(headerName, HeaderNames.StrictTransportSecurity, StringComparison.OrdinalIgnoreCase))
{
continue;
}

destination[headerName] = RequestUtilities.Concat(currentValue, header.Value);
}
}
}
3 changes: 1 addition & 2 deletions src/ReverseProxy/Forwarder/RequestUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ internal static bool ShouldSkipResponseHeader(string headerName)
return _headersToExclude.Contains(headerName);
}

private static readonly FrozenSet<string> _headersToExclude = new HashSet<string>(18, StringComparer.OrdinalIgnoreCase)
private static readonly FrozenSet<string> _headersToExclude = new HashSet<string>(17, StringComparer.OrdinalIgnoreCase)
{
HeaderNames.Connection,
HeaderNames.TransferEncoding,
Expand All @@ -87,7 +87,6 @@ internal static bool ShouldSkipResponseHeader(string headerName)
HeaderNames.UpgradeInsecureRequests,
HeaderNames.TE,
HeaderNames.AltSvc,
HeaderNames.StrictTransportSecurity,
}.ToFrozenSet(StringComparer.OrdinalIgnoreCase);

// Headers marked as HttpHeaderType.Content in
Expand Down
33 changes: 32 additions & 1 deletion test/ReverseProxy.Tests/Forwarder/HttpTransformerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public class HttpTransformerTests
HeaderNames.UpgradeInsecureRequests,
HeaderNames.TE,
HeaderNames.AltSvc,
HeaderNames.StrictTransportSecurity,
};

[Fact]
Expand Down Expand Up @@ -163,6 +162,38 @@ public async Task TransformResponseAsync_RemovesRestrictedHeaders()
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task TransformResponseAsync_StrictTransportSecurity_CopiedIfNotPresent(bool alreadyPresent)
{
var transformer = HttpTransformer.Default;
var httpContext = new DefaultHttpContext();
var proxyResponse = new HttpResponseMessage()
{
Content = new ByteArrayContent(Array.Empty<byte>())
};

if (alreadyPresent)
{
httpContext.Response.Headers.StrictTransportSecurity = "max-age=31536000; includeSubDomains";
}

Assert.True(proxyResponse.Headers.TryAddWithoutValidation(HeaderNames.StrictTransportSecurity, "max-age=31000; preload"));

await transformer.TransformResponseAsync(httpContext, proxyResponse, CancellationToken.None);

var result = httpContext.Response.Headers.StrictTransportSecurity;
if (alreadyPresent)
{
Assert.Equal("max-age=31536000; includeSubDomains", result);
}
else
{
Assert.Equal("max-age=31000; preload", result);
}
}

[Fact]
public async Task TransformResponseAsync_ContentLengthAndTransferEncoding_ContentLengthRemoved()
{
Expand Down
Loading