Runtime: Option to allow HttpClient to follow HTTPS -> HTTP redirects

[martin-heralecky]

API Proposal:
c# class SocketsHttpHandler { bool DangerousAllowHttpsToHttpRedirection { get; set; } }

---

Currently HttpClient does not follow HTTPS -> HTTP redirects. However, even though this increases security, sometimes it is necessary to follow these redirects.

I suggest to add an option which would make HttpClient follow HTTPS -> HTTP redirects.

[davidsh]

If you need to follow HTTPS -> HTTP redirects, the currently solution is to disable automatic redirection, HttpClientHandler.AllowAutoRedirect.

Then do the redirection manually by reading the Location: response header on the 3xx response.

[martin-heralecky]

Yes, I know. But it'd be easier just to construct HttpClient with some optional argument (or some other way) to have it follow these redirects automatically.

[davidsh]

You can also create a delegating handler (based on HttpMessageHandler or DelegatingHandler). In that handler you can do the manual redirection there.

See examples:
https://www.joelverhagen.com/blog/2014/11/more-control-in-httpclient-redirects/

https://stackoverflow.com/questions/11970313/delegatinghandler-for-response-in-webapi

[rmkerr]

Given that (1) this behavior is disabled for security reasons, and (2) there are well supported methods for working around its limitations, I don't think that we should change anything here.

I think that by leaving the process as slightly more involved, we increase the likelihood that anyone who enables insecure redirects actually understands the risk.

[davidsh]

@karelz

[karelz]

Related to dotnet/runtime#23813

I agree with @rmkerr and @davidsh - IMO we should not add such API. I am fine with keeping it open to collect more feedback / upvotes.
I wonder if having standalone community-driven NuGet package with delegating handler would be a reasonable middle ground solution.

[GSPP]

What is the security risk that this mitigates? It seems that if the server (deliberately) redirects to an HTTP url then so be it.

[rmkerr]

My primary concern is around the potential lack of transparency when we perform an https -> http redirect. For example, when a developer writes this code:

var result = await client.GetAsync("https://contoso.com/account");

They reasonably expect to be getting the content over a secure connection, and may make assumptions about the integrity of the response. If we silently redirect from https to http those assumptions will not necessarily hold true.

The current implementation does allow servers to break that assumption, but only if the client also explicitly opts in to that behavior by manually processing redirects or creating a delegating handler. We could make it easier for clients to opt in, but as I said above:

I think that by leaving the process as slightly more involved, we increase the likelihood that anyone who enables insecure redirects actually understands the risk.

[narciero]

why make everyone have to go through the trouble of manually handling redirects when a property called AllowInsecureRedirects could easily be added ? I think its pretty clear that the property enables insecure behavior - it would obviously default to false.

many websites in the wild perform 301/302 redirects from HTTPS -> HTTP (which then gets redirected back from HTTP -> HTTPS) - so it would be nice to be able to control that behavior from a client's perspective where you have no control over the server - left with the only option of manually implementing yourself which seems less than ideal since its such a simple API to add.

[karelz]

Triage:
We don't want to encourage insecure behavior in APIs, but given the larger impact, we are willing to take this one if our security folks agree.
We will expose it only on SocketsHttpHandler, because that is for more advanced scenarios.
Suggested name: DangerousAllowHttpsToHttpRedirection.

[terrajobst]

Video

This needs work:

• It's a dangerous API; the name seems OK, although we normally use the Unsafe prefix.
• We wonder whether instead of simple bool this should be callback that takes at least the URL or even the request message and return true whether it should be redirected or not.

C# namespace System.Net.Http { public partial class SocketsHttpHandler { public bool DangerousAllowHttpsToHttpRedirection { get; set; } } }

[davidsh]

It's a dangerous API; the name seems OK, although we normally use the Unsafe prefix.

We already use the prefix 'Dangerous' here in 'HttpClientHandler'
https://github.com/dotnet/runtime/blob/73eec1b1223eb5d974a3ceff2419f2dc425ae4d1/src/libraries/System.Net.Http/ref/System.Net.Http.cs#L93

c# public partial class HttpClientHandler : System.Net.Http.HttpMessageHandler { public static System.Func<System.Net.Http.HttpRequestMessage, System.Security.Cryptography.X509Certificates.X509Certificate2, System.Security.Cryptography.X509Certificates.X509Chain, System.Net.Security.SslPolicyErrors, bool> DangerousAcceptAnyServerCertificateValidator { get { throw null; } } // ... }

[davidsh]

We wonder whether instead of simple bool this should be callback that takes at least the URL or even the request message and return true whether it should be redirected or not.

Why make this API so complicated? Is this API really more dangerous than being able to easily bypass TLS certificate verification with the DangerousAcceptAnyServerCertificateValidator API?

As long as this API default is 'safe' (i.e. don't do redirect from HTTPS to HTTP), then I don't see why making this API difficult to use will help developers. If we make it so complicated to use, they might as well turn off auto-direct and parse the 'Location' header themselves from the 3xx response.

[danmosemsft]

@bartonjs do the guidelines suggest when to use Dangerous vs Unsafe? In my mind Dangerous means “difficult to use at all in a safe way” whereas Unsafe means more like “removes some safety checks” but that’s just what I’d assumed.

[bartonjs]

do the guidelines suggest when to use Dangerous vs Unsafe?

Nope. I also don't recall a Dangerous v Unsafe discussion in the meeting; only that maybe a context-providing delegate was a good balance between "do a bunch of work to do manual redirects" and "anything's fair game".

According to apisof.net, we only have 4 Dangerouses: SafeHandle.DangerousAddRef, SafeHandle.DangerousRelease, SafeHandle.DangerousGetHandle, HttpClient.DangerousAcceptAnyServerCertificateValidator (then ASP.Net has a handful). But generally "Unsafe" means "pointer-type operations that may be lacking in bounds checking" and (for ThreadPool) "doesn't capture context".

I'd call this one Dangerous if it's a boolean property, but that's because I like keeping Unsafe as being related to the unsafe code keyword.

[scalablecory]

Why make this API so complicated?

@davidsh there's a suggestion that you could do eg. handler.RedirectValidator = e => IsTrustedIntranet(e.Hostname) for when you're okay with redirects to specific trusted sources (maybe intranet) but want to guard against anything else.