Skip to content

Commit 57d6b36

Browse files
fix(security): remove proxy and cookie auth headers on insecure redirect (#653)
* fix: remove proxy and cookie auth headers on insecure redirect * fix: dispose of HttpRequestMessage instance * test: update tests to dispose HttpRequestMessage * fix(security): enable configuration to allow removing arbitrary header on redirect. Useful for API Keys and other custom header auth
1 parent 17c2c88 commit 57d6b36

4 files changed

Lines changed: 777 additions & 109 deletions

File tree

.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
"Kiota"
44
],
55
"editor.formatOnSave": true,
6-
"dotnet-test-explorer.testProjectPath": "**/*.Tests.csproj"
6+
"dotnet-test-explorer.testProjectPath": "**/*.Tests.csproj",
7+
"sarif-viewer.connectToGithubCodeScanning": "on"
78
}

src/http/httpClient/Middleware/Options/RedirectHandlerOption.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,45 @@ public int MaxRedirect
4444
/// A boolean value to determine if we redirects are allowed if the scheme changes(e.g. https to http). Defaults to false.
4545
/// </summary>
4646
public bool AllowRedirectOnSchemeChange { get; set; } = false;
47+
48+
/// <summary>
49+
/// A callback that is invoked to scrub sensitive headers from the request before following a redirect.
50+
/// This callback receives the request being modified, the original URI, the new redirect URI, and a proxy resolver function.
51+
/// The proxy resolver returns the proxy URI for a given destination, or null if no proxy applies.
52+
/// Defaults to <see cref="DefaultScrubSensitiveHeaders"/>.
53+
/// </summary>
54+
public Action<HttpRequestMessage, Uri, Uri, Func<Uri, Uri?>?> ScrubSensitiveHeaders { get; set; } = DefaultScrubSensitiveHeaders;
55+
56+
/// <summary>
57+
/// The default implementation for scrubbing sensitive headers during redirects.
58+
/// This method removes Authorization and Cookie headers when the host or scheme changes,
59+
/// and removes ProxyAuthorization headers when no proxy is configured or the proxy is bypassed for the new URI.
60+
/// </summary>
61+
/// <param name="request">The HTTP request message to modify.</param>
62+
/// <param name="originalUri">The original request URI.</param>
63+
/// <param name="newUri">The new redirect URI.</param>
64+
/// <param name="proxyResolver">A function that returns the proxy URI for a destination, or null if no proxy applies. Can be null if no proxy is configured.</param>
65+
public static void DefaultScrubSensitiveHeaders(HttpRequestMessage request, Uri originalUri, Uri newUri, Func<Uri, Uri?>? proxyResolver)
66+
{
67+
if(request == null) throw new ArgumentNullException(nameof(request));
68+
if(originalUri == null) throw new ArgumentNullException(nameof(originalUri));
69+
if(newUri == null) throw new ArgumentNullException(nameof(newUri));
70+
71+
// Remove Authorization and Cookie headers if http request's scheme or host changes
72+
var isDifferentHostOrScheme = !newUri.Host.Equals(originalUri.Host, StringComparison.OrdinalIgnoreCase) ||
73+
!newUri.Scheme.Equals(originalUri.Scheme, StringComparison.OrdinalIgnoreCase);
74+
if(isDifferentHostOrScheme)
75+
{
76+
request.Headers.Authorization = null;
77+
request.Headers.Remove("Cookie");
78+
}
79+
80+
// Remove ProxyAuthorization if no proxy is configured or the URL is bypassed
81+
var isProxyInactive = proxyResolver == null || proxyResolver(newUri) == null;
82+
if(isProxyInactive)
83+
{
84+
request.Headers.ProxyAuthorization = null;
85+
}
86+
}
4787
}
4888
}

src/http/httpClient/Middleware/RedirectHandler.cs

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,9 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
119119
newRequest.RequestUri = new Uri(baseAddress + response.Headers.Location);
120120
}
121121

122-
// Remove Auth if http request's scheme or host changes
123-
if(!newRequest.RequestUri.Host.Equals(request.RequestUri?.Host) ||
124-
!newRequest.RequestUri.Scheme.Equals(request.RequestUri?.Scheme))
125-
{
126-
newRequest.Headers.Authorization = null;
127-
}
122+
// Scrub sensitive headers before following the redirect
123+
var proxyResolver = GetProxyResolver();
124+
redirectOption.ScrubSensitiveHeaders(newRequest, request.RequestUri!, newRequest.RequestUri, proxyResolver);
128125

129126
// If scheme has changed. Ensure that this has been opted in for security reasons
130127
if(!newRequest.RequestUri.Scheme.Equals(request.RequestUri?.Scheme) && !redirectOption.AllowRedirectOnSchemeChange)
@@ -183,5 +180,48 @@ private static bool IsRedirect(HttpStatusCode statusCode)
183180
};
184181
}
185182

183+
/// <summary>
184+
/// Gets a callback that resolves the proxy URI for a given destination URI.
185+
/// </summary>
186+
/// <returns>A function that takes a destination URI and returns the proxy URI, or null if no proxy is configured or the destination is bypassed.</returns>
187+
private Func<Uri, Uri?>? GetProxyResolver()
188+
{
189+
var proxy = GetProxyFromFinalHandler();
190+
if(proxy == null)
191+
return null;
192+
return destination => proxy.IsBypassed(destination) ? null : proxy.GetProxy(destination);
193+
}
194+
195+
/// <summary>
196+
/// Traverses the handler chain to find the final handler and extract its proxy settings.
197+
/// </summary>
198+
/// <returns>The IWebProxy from the final handler, or null if not found.</returns>
199+
private IWebProxy? GetProxyFromFinalHandler()
200+
{
201+
#if BROWSER
202+
// Browser platform does not support proxy configuration
203+
return null;
204+
#else
205+
var handler = InnerHandler;
206+
while(handler != null)
207+
{
208+
#if NETFRAMEWORK
209+
if(handler is WinHttpHandler winHttpHandler)
210+
return winHttpHandler.Proxy;
211+
#endif
212+
#if NET5_0_OR_GREATER
213+
if (handler is SocketsHttpHandler socketsHandler)
214+
return socketsHandler.Proxy;
215+
#endif
216+
if(handler is HttpClientHandler httpClientHandler)
217+
return httpClientHandler.Proxy;
218+
if(handler is DelegatingHandler delegatingHandler)
219+
handler = delegatingHandler.InnerHandler;
220+
else
221+
break;
222+
}
223+
return null;
224+
#endif
225+
}
186226
}
187227
}

0 commit comments

Comments
 (0)