Merge pull request #14319 from dmage/fwd-host

Automatic merge from submit-queue (batch tested with PRs 16052, 16105, 14319, 16106, 16104)

Use forwarded Host header without any changes

The Docker Server is sensitive to the presence of the port in a URL. If Docker Server is authorised to https://example.com/ and it is redirected to https://example.com:443, it might loose the Authorization header. So we should not remove nor add the port to the Host header.

Fixes bug 1439614

Author: openshift-merge-robot

pkg/util/httprequest/httprequest.go

@@ -66,32 +66,43 @@ func SchemeHost(req *http.Request) (string /*scheme*/, string /*host*/) {
 		return strings.TrimSpace(value)
 	}
 
-	forwardedProto := forwarded("Proto")
-	forwardedHost := forwarded("Host")
-	// If both X-Forwarded-Host and X-Forwarded-Port are sent, use the explicit port info
-	if forwardedPort := forwarded("Port"); len(forwardedHost) > 0 && len(forwardedPort) > 0 {
-		if h, _, err := net.SplitHostPort(forwardedHost); err == nil {
-			forwardedHost = net.JoinHostPort(h, forwardedPort)
-		} else {
-			forwardedHost = net.JoinHostPort(forwardedHost, forwardedPort)
-		}
+	hasExplicitHost := func(h string) bool {
+		_, _, err := net.SplitHostPort(h)
+		return err == nil
 	}
 
+	forwardedHost := forwarded("Host")
 	host := ""
+	hostHadExplicitPort := false
 	switch {
 	case len(forwardedHost) > 0:
 		host = forwardedHost
+		hostHadExplicitPort = hasExplicitHost(host)
+
+		// If both X-Forwarded-Host and X-Forwarded-Port are sent, use the explicit port info
+		if forwardedPort := forwarded("Port"); len(forwardedPort) > 0 {
+			if h, _, err := net.SplitHostPort(forwardedHost); err == nil {
+				host = net.JoinHostPort(h, forwardedPort)
+			} else {
+				host = net.JoinHostPort(forwardedHost, forwardedPort)
+			}
+		}
+
 	case len(req.Host) > 0:
 		host = req.Host
+		hostHadExplicitPort = hasExplicitHost(host)
+
 	case len(req.URL.Host) > 0:
 		host = req.URL.Host
+		hostHadExplicitPort = hasExplicitHost(host)
 	}
 
 	port := ""
 	if _, p, err := net.SplitHostPort(host); err == nil {
 		port = p
 	}
 
+	forwardedProto := forwarded("Proto")
 	scheme := ""
 	switch {
 	case len(forwardedProto) > 0:
@@ -106,5 +117,13 @@ func SchemeHost(req *http.Request) (string /*scheme*/, string /*host*/) {
 		scheme = "http"
 	}
 
+	if !hostHadExplicitPort {
+		if (scheme == "https" && port == "443") || (scheme == "http" && port == "80") {
+			if hostWithoutPort, _, err := net.SplitHostPort(host); err == nil {
+				host = hostWithoutPort
+			}
+		}
+	}
+
 	return scheme, host
 }

pkg/util/httprequest/httprequest_test.go

@@ -25,7 +25,7 @@ func TestSchemeHost(t *testing.T) {
 				},
 			},
 			expectedScheme: "https",
-			expectedHost:   "example.com:443",
+			expectedHost:   "example.com",
 		},
 		"X-Forwarded-Port overwrites X-Forwarded-Host port": {
 			req: &http.Request{
@@ -51,7 +51,32 @@ func TestSchemeHost(t *testing.T) {
 				},
 			},
 			expectedScheme: "https",
-			expectedHost:   "example.com:443",
+			expectedHost:   "example.com",
+		},
+		"stripped X-Forwarded-Host and X-Forwarded-Port with non-standard port": {
+			req: &http.Request{
+				URL:  &url.URL{Path: "/"},
+				Host: "127.0.0.1",
+				Header: http.Header{
+					"X-Forwarded-Host":  []string{"example.com"},
+					"X-Forwarded-Port":  []string{"80"},
+					"X-Forwarded-Proto": []string{"https"},
+				},
+			},
+			expectedScheme: "https",
+			expectedHost:   "example.com:80",
+		},
+		"detect scheme from X-Forwarded-Port": {
+			req: &http.Request{
+				URL:  &url.URL{Path: "/"},
+				Host: "127.0.0.1",
+				Header: http.Header{
+					"X-Forwarded-Host": []string{"example.com"},
+					"X-Forwarded-Port": []string{"443"},
+				},
+			},
+			expectedScheme: "https",
+			expectedHost:   "example.com",
 		},
 
 		"req host": {
@@ -156,7 +181,7 @@ func TestSchemeHost(t *testing.T) {
 				},
 			},
 			expectedScheme: "http",
-			expectedHost:   "route-namespace.router.default.svc.cluster.local:80",
+			expectedHost:   "route-namespace.router.default.svc.cluster.local",
 		},
 		"haproxy edge terminated route -> svc -> non-tls pod": {
 			req: &http.Request{
@@ -171,6 +196,21 @@ func TestSchemeHost(t *testing.T) {
 				},
 			},
 			expectedScheme: "https",
+			expectedHost:   "route-namespace.router.default.svc.cluster.local",
+		},
+		"haproxy edge terminated route -> svc -> non-tls pod with the explicit port": {
+			req: &http.Request{
+				URL:  &url.URL{Path: "/"},
+				Host: "route-namespace.router.default.svc.cluster.local:443",
+				Header: http.Header{
+					"X-Forwarded-Host":  []string{"route-namespace.router.default.svc.cluster.local:443"},
+					"X-Forwarded-Port":  []string{"443"},
+					"X-Forwarded-Proto": []string{"https"},
+					"Forwarded":         []string{"for=172.18.2.57;host=route-namespace.router.default.svc.cluster.local:443;proto=https"},
+					"X-Forwarded-For":   []string{"172.18.2.57"},
+				},
+			},
+			expectedScheme: "https",
 			expectedHost:   "route-namespace.router.default.svc.cluster.local:443",
 		},
 		"haproxy passthrough route -> svc -> tls pod": {