[Test proxy] SSL validation error on cross-domain redirect in record mode

[timovv]

I encountered this when writing tests for new ACR functionality, so let me use it as an example:

When downloading a blob from ACR, the container registry initially responds with a 307 Temporary Redirect which points to a storage blob URL. This storage blob URL has a different hostname to the ACR service itself.

This is the flow that ensues:

1. Client makes request to the original URL via the test proxy
2. The test proxy forwards this request upstream correctly and receives a redirect status back. The redirect location has a different hostname to the original incoming request.
3. Since System.Net.HttpClient is configured by default to follow the redirect, it makes a request to the resource at the end of the redirect. It uses the redirect URL (from the Location header) to figure out where to make the request, as it should. But, since the Host header was set manually when preparing the initial upstream request, it doesn't compute the Host from the redirect URI and instead uses the manually set value, which is the same as that from the initial upstream request.
4. The incorrect Host header confuses the upstream server causing an SSL error due to the mismatch during the SNI process.
5. The test proxy returns a 500 status due to the exception.

Essentially, there's a mismatch between the web server the redirected request is sent to (which is correct), and the Host header that is sent with that request (which is incorrect).

A couple of solutions spring to mind:

1. Instead of manually overriding the Host header, we can just ignore it like we do for the Proxy-Connection header.
2. We can just make the test proxy not follow redirects (see HttpClient.AllowAutoRedirect), nullifying the issue. I think this makes more sense, but it doesn't work for JS at the moment - I tried it to see, and the redirected request doesn't get sent through the proxy as it would need to, and is instead made to the live service. I assume that's due to the ordering of policies in the pipeline. But I suppose we could look into fixing that on the JS side?

At the moment I'm working around it by running the test proxy with fix (1) applied locally. Happy to contribute that fix if that's what we want to go with :).

Cc: @scbedd @HarshaNalluru

[Mohit-Chakraborty]

I finally reached the point in my work for the ACR .NET SDK to hit the same issue.
Thanks for the details in the issue @timovv.

[scbedd]

After discussing this with Mike, going to publish a version of the test-proxy with redirect disabled. Everywhere else the test-proxy is 100% transparent as to what's happening with the request under the hood. Allowing the redirects would go against that.

I will publish a test-proxy version that contains these changes before I merge to main, to allow you folks time debug through.

[mikeharder]

Yes, I think we should try to disable HttpClient.AllowAutoRedirect in test-proxy, and let the redirect flow through and be handled by the client calling test-proxy. Then try to debug why this doesn't work for JS.

[scbedd]

Following-up. Image and Nuget version for the redirectless test-proxy are at 1.0.0-dev.20220401.2.

[timovv]

Thanks Scott, I pulled down that image and gave it a spin. Can confirm that that version isn't following redirects, and that JS is having the issue I described where the redirected request is not being passed through the test proxy. I'll investigate on the JS end, but I'm not sure how involved the fix may be.

[mikeharder]

@timovv: The redirected request needs to go through test proxy. I'm not sure how our core-http pipeline handles redirects (or if its consistent across languages). Does the core-http pipeline tell the underlying HTTP client to disable auto-redirects, so all redirects flow up and down the full core-http pipeline, and policies are run on the redirected request? Or is the redirect handled within the underlying HTTP client, and invisible to policies?

If it's the former, this new behavior of test-proxy should just work with JS core-http. If it's the latter, we will need to either modify core-http to support this, or try to have test-proxy handle the redirect.

But I still think the redirect should be handled in the SDK client, since the goal of test-proxy is to be as transparent as possible, with the SDK client receiving exactly the same HTTP traffic as live.

[mikeharder]

In .NET, it appears that redirects are handled by azure-core instead of System.Net.HttpClient, so I think the new test-proxy behavior should "just work" with .NET, unless there are ordering issues with the redirect and test-proxy policies.

https://github.com/Azure/azure-sdk-for-net/blob/0aaeada992e384d77caf92819607a0c89cf30426/sdk/core/Azure.Core/src/Pipeline/HttpWebRequestTransport.cs#L131-L132

[scbedd]

@Mohit-Chakraborty when you get a couple minutes can you try your tests with the test-proxy version 1.0.0-dev.20220401.2? Pretty certain that should work now with the redirect disabled on proxy-side.

[Mohit-Chakraborty]

@JoshLove-msft updated the test proxy version in .NET.
I removed the workaround of not comparing the 'Accept' header.

Thanks for providing a fix @scbedd.

[scbedd]

Closing this issue, there is an issue covering how the test-proxy will address this long term in #3083