Handle incoming proxy requests ip, scheme (#5591)

This PR implements functions to get the right ip, scheme
from the incoming proxied requests.

cmd/admin-handlers.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/minio/minio/pkg/auth"
+	"github.com/minio/minio/pkg/handlers"
 	"github.com/minio/minio/pkg/madmin"
 )
 
@@ -472,17 +473,6 @@ func (a adminAPIHandlers) HealHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Helper function to fetch client address - we use the
-	// X-forwarded-for header if one is present.
-	getClientAddress := func() string {
-		addr := r.RemoteAddr
-		fwdFor := r.Header.Get("X-Forwarded-For")
-		if fwdFor == "" {
-			return addr
-		}
-		return fwdFor
-	}
-
 	type healResp struct {
 		respBytes []byte
 		errCode   APIErrorCode
@@ -530,7 +520,7 @@ func (a adminAPIHandlers) HealHandler(w http.ResponseWriter, r *http.Request) {
 
 	if clientToken == "" {
 		// Not a status request
-		nh := newHealSequence(bucket, objPrefix, getClientAddress(),
+		nh := newHealSequence(bucket, objPrefix, handlers.GetSourceIP(r),
 			numDisks, hs, forceStart)
 
 		respCh := make(chan healResp)

cmd/api-response.go

@@ -22,6 +22,8 @@ import (
 	"net/url"
 	"path"
 	"time"
+
+	"github.com/minio/minio/pkg/handlers"
 )
 
 const (
@@ -281,15 +283,21 @@ func getURLScheme(tls bool) string {
 }
 
 // getObjectLocation gets the fully qualified URL of an object.
-func getObjectLocation(host, proto, bucket, object string) string {
+func getObjectLocation(r *http.Request, domain, bucket, object string) string {
+	proto := handlers.GetSourceScheme(r)
 	if proto == "" {
 		proto = getURLScheme(globalIsSSL)
 	}
-	u := url.URL{
+	u := &url.URL{
-		Host:   host,
+		Host:   r.Host,
 		Path:   path.Join(slashSeparator, bucket, object),
 		Scheme: proto,
 	}
+	// If domain is set then we need to use bucket DNS style.
+	if domain != "" {
+		u.Host = bucket + "." + domain
+		u.Path = path.Join(slashSeparator, object)
+	}
 	return u.String()
 }
 

cmd/api-response_test.go

@@ -17,42 +17,89 @@
 package cmd
 
 import (
+	"net/http"
 	"testing"
 )
 
 // Tests object location.
 func TestObjectLocation(t *testing.T) {
 	testCases := []struct {
-		host, proto, bucket, object string
+		request          *http.Request
+		bucket, object   string
+		domain           string
 		expectedLocation string
 	}{
 		// Server binding to localhost IP with https.
 		{
-			host:             "127.0.0.1:9000",
+			request: &http.Request{
-			proto:            httpsScheme,
+				Host: "127.0.0.1:9000",
+				Header: map[string][]string{
+					"X-Forwarded-Scheme": {httpScheme},
+				},
+			},
+			bucket:           "testbucket1",
+			object:           "test/1.txt",
+			expectedLocation: "http://127.0.0.1:9000/testbucket1/test/1.txt",
+		},
+		{
+			request: &http.Request{
+				Host: "127.0.0.1:9000",
+				Header: map[string][]string{
+					"X-Forwarded-Scheme": {httpsScheme},
+				},
+			},
 			bucket:           "testbucket1",
 			object:           "test/1.txt",
 			expectedLocation: "https://127.0.0.1:9000/testbucket1/test/1.txt",
 		},
 		// Server binding to fqdn.
 		{
-			host:             "s3.mybucket.org",
+			request: &http.Request{
-			proto:            httpScheme,
+				Host: "s3.mybucket.org",
+				Header: map[string][]string{
+					"X-Forwarded-Scheme": {httpScheme},
+				},
+			},
 			bucket:           "mybucket",
 			object:           "test/1.txt",
 			expectedLocation: "http://s3.mybucket.org/mybucket/test/1.txt",
 		},
 		// Server binding to fqdn.
 		{
-			host:             "mys3.mybucket.org",
+			request: &http.Request{
-			proto:            "",
+				Host:   "mys3.mybucket.org",
+				Header: map[string][]string{},
+			},
 			bucket:           "mybucket",
 			object:           "test/1.txt",
 			expectedLocation: "http://mys3.mybucket.org/mybucket/test/1.txt",
 		},
+		// Server with virtual domain name.
+		{
+			request: &http.Request{
+				Host:   "mys3.bucket.org",
+				Header: map[string][]string{},
+			},
+			domain:           "mys3.bucket.org",
+			bucket:           "mybucket",
+			object:           "test/1.txt",
+			expectedLocation: "http://mybucket.mys3.bucket.org/test/1.txt",
+		},
+		{
+			request: &http.Request{
+				Host: "mys3.bucket.org",
+				Header: map[string][]string{
+					"X-Forwarded-Scheme": {httpsScheme},
+				},
+			},
+			domain:           "mys3.bucket.org",
+			bucket:           "mybucket",
+			object:           "test/1.txt",
+			expectedLocation: "https://mybucket.mys3.bucket.org/test/1.txt",
+		},
 	}
 	for i, testCase := range testCases {
-		gotLocation := getObjectLocation(testCase.host, testCase.proto, testCase.bucket, testCase.object)
+		gotLocation := getObjectLocation(testCase.request, testCase.domain, testCase.bucket, testCase.object)
 		if testCase.expectedLocation != gotLocation {
 			t.Errorf("Test %d: expected %s, got %s", i+1, testCase.expectedLocation, gotLocation)
 		}

cmd/auth-handler.go

@@ -22,6 +22,8 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strings"
+
+	"github.com/minio/minio/pkg/handlers"
 )
 
 // Verify if request has JWT.
@@ -136,13 +138,12 @@ func checkRequestAuthType(r *http.Request, bucket, policyAction, region string)
 
 	if reqAuthType == authTypeAnonymous && policyAction != "" {
 		// http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html
-		sourceIP := getSourceIPAddress(r)
 		resource, err := getResource(r.URL.Path, r.Host, globalDomainName)
 		if err != nil {
 			return ErrInternalError
 		}
 		return enforceBucketPolicy(bucket, policyAction, resource,
-			r.Referer(), sourceIP, r.URL.Query())
+			r.Referer(), handlers.GetSourceIP(r), r.URL.Query())
 	}
 
 	// By default return ErrAccessDenied

cmd/bucket-handlers.go

@@ -572,8 +572,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
 		return
 	}
 
-	port := r.Header.Get("X-Forward-Proto")
+	location := getObjectLocation(r, globalDomainName, bucket, object)
-	location := getObjectLocation(r.Host, port, bucket, object)
 	w.Header().Set("ETag", `"`+objInfo.ETag+`"`)
 	w.Header().Set("Location", location)
 

cmd/object-handlers.go

@@ -32,6 +32,7 @@ import (
 
 	mux "github.com/gorilla/mux"
 	"github.com/minio/minio/pkg/errors"
+	"github.com/minio/minio/pkg/handlers"
 	"github.com/minio/minio/pkg/hash"
 	"github.com/minio/minio/pkg/ioutil"
 	sha256 "github.com/minio/sha256-simd"
@@ -57,22 +58,6 @@ func setHeadGetRespHeaders(w http.ResponseWriter, reqParams url.Values) {
 	}
 }
 
-// getSourceIPAddress - get the source ip address of the request.
-func getSourceIPAddress(r *http.Request) string {
-	var ip string
-	// Attempt to get ip from standard headers.
-	// Do not support X-Forwarded-For because it is easy to spoof.
-	ip = r.Header.Get("X-Real-Ip")
-	parsedIP := net.ParseIP(ip)
-	// Skip non valid IP address.
-	if parsedIP != nil {
-		return ip
-	}
-	// Default to remote address if headers not set.
-	ip, _, _ = net.SplitHostPort(r.RemoteAddr)
-	return ip
-}
-
 // errAllowableNotFound - For an anon user, return 404 if have ListBucket, 403 otherwise
 // this is in keeping with the permissions sections of the docs of both:
 //   HEAD Object: http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectHEAD.html
@@ -81,7 +66,7 @@ func errAllowableObjectNotFound(bucket string, r *http.Request) APIErrorCode {
 	if getRequestAuthType(r) == authTypeAnonymous {
 		// We care about the bucket as a whole, not a particular resource.
 		resource := "/" + bucket
-		sourceIP := getSourceIPAddress(r)
+		sourceIP := handlers.GetSourceIP(r)
 		if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", resource,
 			r.Referer(), sourceIP, r.URL.Query()); s3Error != ErrNone {
 			return ErrAccessDenied
@@ -631,7 +616,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 		return
 	case authTypeAnonymous:
 		// http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html
-		sourceIP := getSourceIPAddress(r)
+		sourceIP := handlers.GetSourceIP(r)
 		if s3Err = enforceBucketPolicy(bucket, "s3:PutObject", r.URL.Path, r.Referer(), sourceIP, r.URL.Query()); s3Err != ErrNone {
 			writeErrorResponse(w, s3Err, r.URL)
 			return
@@ -1049,7 +1034,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
 	case authTypeAnonymous:
 		// http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html
 		if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL.Path,
-			r.Referer(), getSourceIPAddress(r), r.URL.Query()); s3Error != ErrNone {
+			r.Referer(), handlers.GetSourceIP(r), r.URL.Query()); s3Error != ErrNone {
 			writeErrorResponse(w, s3Error, r.URL)
 			return
 		}

cmd/object-handlers_test.go

@@ -3563,56 +3563,3 @@ func testAPIListObjectPartsHandler(obj ObjectLayer, instanceType, bucketName str
 	// `ExecObjectLayerAPINilTest` sets the Object Layer to `nil` and calls the handler.
 	ExecObjectLayerAPINilTest(t, nilBucket, nilObject, instanceType, apiRouter, nilReq)
 }
-
-// TestGetSourceIPAddress - check the source ip of a request is parsed correctly.
-func TestGetSourceIPAddress(t *testing.T) {
-	testCases := []struct {
-		request    *http.Request
-		expectedIP string
-	}{
-		{
-			// Test Case 1. Use only RemoteAddr as host and port.
-			request: &http.Request{
-				RemoteAddr: "127.0.0.1:9000",
-			},
-			expectedIP: "127.0.0.1",
-		},
-		{
-			// Test Case 2. Use both RemoteAddr and single header.
-			request: &http.Request{
-				RemoteAddr: "127.0.0.1:9000",
-				Header: map[string][]string{
-					"X-Real-Ip": {"54.240.143.0"},
-				},
-			},
-			expectedIP: "54.240.143.0", // Use headers before RemoteAddr.
-		},
-		{
-			// Test Case 3. Use both RemoteAddr and several header vals.
-			// Check that first val in header is used.
-			request: &http.Request{
-				RemoteAddr: "127.0.0.1:9000",
-				Header: map[string][]string{
-					"X-Real-Ip": {"54.240.143.0", "54.240.143.188"},
-				},
-			},
-			expectedIP: "54.240.143.0",
-		},
-		{
-			// Test Case 4. Use header and corrupt header value.
-			request: &http.Request{
-				RemoteAddr: "127.0.0.1:9000",
-				Header: map[string][]string{
-					"X-Real-Ip": {"54.240.143.188", "corrupt"},
-				},
-			},
-			expectedIP: "54.240.143.188",
-		},
-	}
-	for i, test := range testCases {
-		receivedIP := getSourceIPAddress(test.request)
-		if test.expectedIP != receivedIP {
-			t.Fatalf("Case %d: Expected the IP to be `%s`, but instead got `%s`", i+1, test.expectedIP, receivedIP)
-		}
-	}
-}

pkg/handlers/proxy.go

@@ -0,0 +1,115 @@
+/*
+ * Minio Cloud Storage, (C) 2018 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package handlers
+
+import (
+	"net"
+	"net/http"
+	"regexp"
+	"strings"
+)
+
+var (
+	// De-facto standard header keys.
+	xForwardedFor    = http.CanonicalHeaderKey("X-Forwarded-For")
+	xForwardedHost   = http.CanonicalHeaderKey("X-Forwarded-Host")
+	xForwardedProto  = http.CanonicalHeaderKey("X-Forwarded-Proto")
+	xForwardedScheme = http.CanonicalHeaderKey("X-Forwarded-Scheme")
+	xRealIP          = http.CanonicalHeaderKey("X-Real-IP")
+)
+
+var (
+	// RFC7239 defines a new "Forwarded: " header designed to replace the
+	// existing use of X-Forwarded-* headers.
+	// e.g. Forwarded: for=192.0.2.60;proto=https;by=203.0.113.43
+	forwarded = http.CanonicalHeaderKey("Forwarded")
+	// Allows for a sub-match of the first value after 'for=' to the next
+	// comma, semi-colon or space. The match is case-insensitive.
+	forRegex = regexp.MustCompile(`(?i)(?:for=)([^(;|,| )]+)(.*)`)
+	// Allows for a sub-match for the first instance of scheme (http|https)
+	// prefixed by 'proto='. The match is case-insensitive.
+	protoRegex = regexp.MustCompile(`(?i)^(;|,| )+(?:proto=)(https|http)`)
+)
+
+// GetSourceScheme retrieves the scheme from the X-Forwarded-Proto and RFC7239
+// Forwarded headers (in that order).
+func GetSourceScheme(r *http.Request) string {
+	var scheme string
+
+	// Retrieve the scheme from X-Forwarded-Proto.
+	if proto := r.Header.Get(xForwardedProto); proto != "" {
+		scheme = strings.ToLower(proto)
+	} else if proto = r.Header.Get(xForwardedScheme); proto != "" {
+		scheme = strings.ToLower(proto)
+	} else if proto := r.Header.Get(forwarded); proto != "" {
+		// match should contain at least two elements if the protocol was
+		// specified in the Forwarded header. The first element will always be
+		// the 'for=', which we ignore, subsequently we proceed to look for
+		// 'proto=' which should precede right after `for=` if not
+		// we simply ignore the values and return empty. This is in line
+		// with the approach we took for returning first ip from multiple
+		// params.
+		if match := forRegex.FindStringSubmatch(proto); len(match) > 1 {
+			if match = protoRegex.FindStringSubmatch(match[2]); len(match) > 1 {
+				scheme = strings.ToLower(match[2])
+			}
+		}
+	}
+
+	return scheme
+}
+
+// GetSourceIP retrieves the IP from the X-Forwarded-For, X-Real-IP and RFC7239
+// Forwarded headers (in that order), falls back to r.RemoteAddr when all
+// else fails.
+func GetSourceIP(r *http.Request) string {
+	var addr string
+
+	if fwd := r.Header.Get(xForwardedFor); fwd != "" {
+		// Only grab the first (client) address. Note that '192.168.0.1,
+		// 10.1.1.1' is a valid key for X-Forwarded-For where addresses after
+		// the first may represent forwarding proxies earlier in the chain.
+		s := strings.Index(fwd, ", ")
+		if s == -1 {
+			s = len(fwd)
+		}
+		addr = fwd[:s]
+	} else if fwd := r.Header.Get(xRealIP); fwd != "" {
+		// X-Real-IP should only contain one IP address (the client making the
+		// request).
+		addr = fwd
+	} else if fwd := r.Header.Get(forwarded); fwd != "" {
+		// match should contain at least two elements if the protocol was
+		// specified in the Forwarded header. The first element will always be
+		// the 'for=' capture, which we ignore. In the case of multiple IP
+		// addresses (for=8.8.8.8, 8.8.4.4, 172.16.1.20 is valid) we only
+		// extract the first, which should be the client IP.
+		if match := forRegex.FindStringSubmatch(fwd); len(match) > 1 {
+			// IPv6 addresses in Forwarded headers are quoted-strings. We strip
+			// these quotes.
+			addr = strings.Trim(match[1], `"`)
+		}
+	}
+
+	if addr != "" {
+		return addr
+	}
+
+	// Default to remote address if headers not set.
+	addr, _, _ = net.SplitHostPort(r.RemoteAddr)
+	return addr
+}

pkg/handlers/proxy_test.go

@@ -0,0 +1,83 @@
+/*
+ * Minio Cloud Storage, (C) 2018 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package handlers
+
+import (
+	"net/http"
+	"testing"
+)
+
+type headerTest struct {
+	key      string // header key
+	val      string // header val
+	expected string // expected result
+}
+
+func TestGetScheme(t *testing.T) {
+	headers := []headerTest{
+		{xForwardedProto, "https", "https"},
+		{xForwardedProto, "http", "http"},
+		{xForwardedProto, "HTTP", "http"},
+		{xForwardedScheme, "https", "https"},
+		{xForwardedScheme, "http", "http"},
+		{xForwardedScheme, "HTTP", "http"},
+		{forwarded, `For="[2001:db8:cafe::17]:4711`, ""},                    // No proto
+		{forwarded, `for=192.0.2.43, for=198.51.100.17;proto=https`, ""},    // Multiple params, will be empty.
+		{forwarded, `for=172.32.10.15; proto=https;by=127.0.0.1;`, "https"}, // Space before proto
+		{forwarded, `for=192.0.2.60;proto=http;by=203.0.113.43`, "http"},    // Multiple params
+	}
+	for _, v := range headers {
+		req := &http.Request{
+			Header: http.Header{
+				v.key: []string{v.val},
+			}}
+		res := GetSourceScheme(req)
+		if res != v.expected {
+			t.Errorf("wrong header for %s: got %s want %s", v.key, res,
+				v.expected)
+		}
+	}
+}
+
+// TestGetSourceIP - check the source ip of a request is parsed correctly.
+func TestGetSourceIP(t *testing.T) {
+	headers := []headerTest{
+		{xForwardedFor, "8.8.8.8", "8.8.8.8"},                                         // Single address
+		{xForwardedFor, "8.8.8.8, 8.8.4.4", "8.8.8.8"},                                // Multiple
+		{xForwardedFor, "", ""},                                                       // None
+		{xRealIP, "8.8.8.8", "8.8.8.8"},                                               // Single address
+		{xRealIP, "[2001:db8:cafe::17]:4711", "[2001:db8:cafe::17]:4711"},             // IPv6 address
+		{xRealIP, "", ""},                                                             // None
+		{forwarded, `for="_gazonk"`, "_gazonk"},                                       // Hostname
+		{forwarded, `For="[2001:db8:cafe::17]:4711`, `[2001:db8:cafe::17]:4711`},      // IPv6 address
+		{forwarded, `for=192.0.2.60;proto=http;by=203.0.113.43`, `192.0.2.60`},        // Multiple params
+		{forwarded, `for=192.0.2.43, for=198.51.100.17`, "192.0.2.43"},                // Multiple params
+		{forwarded, `for="workstation.local",for=198.51.100.17`, "workstation.local"}, // Hostname
+	}
+
+	for _, v := range headers {
+		req := &http.Request{
+			Header: http.Header{
+				v.key: []string{v.val},
+			}}
+		res := GetSourceIP(req)
+		if res != v.expected {
+			t.Errorf("wrong header for %s: got %s want %s", v.key, res,
+				v.expected)
+		}
+	}
+}