Don't override Host when forward-proxying - effectively preserving the transparent proxy approach

[similark]

Provide a description of what has been changed
This small change solves #331 - when building the forward proxy request, there's no need to override the Host field, which trashes the Host header altogether. The request has the proper value in the URL, making it reach the target svc and still preserving the original Host. This makes it possible to treat the request properly down the road (at the application level)

Checklist

• Commits are signed with Developer Certificate of Origin (DCO)
• Changelog has been updated and is aligned with our changelog requirements
• Any necessary documentation is added, such as:
  • README.md
  • The docs/ directory
  • The docs repo

Fixes #331

[similark]

Opinions, anyone? @JorTurFer

[JorTurFer]

is this correct @t0rr3sp3dr0 ?

[similark]

Just making note that this change is covered in the TestImmediatelySuccessfulProxy test.. meaning the request reaches its target as expected. Only difference is that now the Host header is kept as it was, and not updated. In a running cluster, the target pod will get the request with the actual Host in place, and not service_name.svc.NAMESPACE.....

[t0rr3sp3dr0]

Yes, it makes sense that we don't override the Host header on incoming requests.

Looking at the entire function, we shouldn't override the Director from httputil.NewSingleHostReverseProxy the way we are doing. We should call the original Director implementation and then make our changes, otherwise we are just using a blank httputil.ReverseProxy and not httputil.NewSingleHostReverseProxy.

[t0rr3sp3dr0]

I would do something like this:

	superDirector := proxy.Director
	proxy.Director = func(req *http.Request) {
		host := req.URL.Host
		superDirector(req)
		req.URL.Host = host

		// Strip client-provided forwarding headers to prevent IP spoofing.
		req.Header.Del("Forwarded")
		req.Header.Del("X-Forwarded-For")
		req.Header.Del("X-Forwarded-Host")
		req.Header.Del("X-Forwarded-Proto")
	}

For reference:

• https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/net/http/httputil/reverseproxy.go;l=142
• https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/net/http/httputil/reverseproxy.go;l=262
• https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/net/http/httputil/reverseproxy.go;l=421

[similark]

@t0rr3sp3dr0 how about this? (I omitted the req.URL.Host blanking here)

[similark]

Opinions, anyone? @tomkerkhove @JorTurFer @t0rr3sp3dr0
This is kind of a show-stopper for us, as any downstream application can't figure the original request.

[t0rr3sp3dr0]

@similark, I’m ok with the changes, but you need to rebase your branch. The file you patched moved to interceptor/handler/upstream.go.

[t0rr3sp3dr0]

Also, update CHANGELOG.md to reflect this change.