diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 01bb3da78..32464c2eb 100644 --- a/cmd/admin-handlers.go +++ b/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) diff --git a/cmd/api-response.go b/cmd/api-response.go index eb0c7940c..95c01982a 100644 --- a/cmd/api-response.go +++ b/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{ - Host: host, + u := &url.URL{ + 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() } diff --git a/cmd/api-response_test.go b/cmd/api-response_test.go index 6ad4dda10..bde1b1828 100644 --- a/cmd/api-response_test.go +++ b/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 - expectedLocation string + request *http.Request + bucket, object string + domain string + expectedLocation string }{ // Server binding to localhost IP with https. { - host: "127.0.0.1:9000", - proto: httpsScheme, + request: &http.Request{ + 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", - proto: httpScheme, + request: &http.Request{ + 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", - proto: "", + request: &http.Request{ + 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) } diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index ae632da4b..c469297d9 100644 --- a/cmd/auth-handler.go +++ b/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 diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 109e03593..58a577908 100644 --- a/cmd/bucket-handlers.go +++ b/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.Host, port, bucket, object) + location := getObjectLocation(r, globalDomainName, bucket, object) w.Header().Set("ETag", `"`+objInfo.ETag+`"`) w.Header().Set("Location", location) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index b5f8abe7d..367699526 100644 --- a/cmd/object-handlers.go +++ b/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 } diff --git a/cmd/object-handlers_test.go b/cmd/object-handlers_test.go index a3c5481e4..98546c9ba 100644 --- a/cmd/object-handlers_test.go +++ b/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) - } - } -} diff --git a/pkg/handlers/proxy.go b/pkg/handlers/proxy.go new file mode 100644 index 000000000..757ce64bc --- /dev/null +++ b/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 +} diff --git a/pkg/handlers/proxy_test.go b/pkg/handlers/proxy_test.go new file mode 100644 index 000000000..5456c1365 --- /dev/null +++ b/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) + } + } +}