bucket-policy: Add IPAddress/NotIPAddress conditions support (#4736)

cmd/auth-handler.go

@@ -126,8 +126,9 @@ 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)
 		return enforceBucketPolicy(bucket, policyAction, r.URL.Path,
-			r.Referer(), r.URL.Query())
+			r.Referer(), sourceIP, r.URL.Query())
 	}
 
 	// By default return ErrAccessDenied

cmd/bucket-handlers.go

@@ -33,7 +33,7 @@ import (
 
 // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html
 // Enforces bucket policies for a bucket for a given tatusaction.
-func enforceBucketPolicy(bucket, action, resource, referer string, queryParams url.Values) (s3Error APIErrorCode) {
+func enforceBucketPolicy(bucket, action, resource, referer, sourceIP string, queryParams url.Values) (s3Error APIErrorCode) {
 	// Verify if bucket actually exists
 	if err := checkBucketExist(bucket, newObjectLayerFn()); err != nil {
 		err = errorCause(err)
@@ -73,6 +73,8 @@ func enforceBucketPolicy(bucket, action, resource, referer string, queryParams u
 	if referer != "" {
 		conditionKeyMap["referer"] = set.CreateStringSet(referer)
 	}
+	// Add request source Ip to conditionKeyMap.
+	conditionKeyMap["ip"] = set.CreateStringSet(sourceIP)
 
 	// Validate action, resource and conditions with current policy statements.
 	if !bucketPolicyEvalStatements(action, arn, conditionKeyMap, policy.Statements) {

cmd/bucket-policy-handlers.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"runtime"
 	"strings"
@@ -81,6 +82,18 @@ func refererMatch(pattern, referer string) bool {
 	return wildcard.MatchSimple(pattern, referer)
 }
 
+// isIPInCIDR - checks if a given a IP address is a member of the given subnet.
+func isIPInCIDR(cidr, ip string) bool {
+	// AWS S3 spec says IPs must use standard CIDR notation.
+	// http://docs.aws.amazon.com/AmazonS3/latest/dev/example-bucket-policies.html#example-bucket-policies-use-case-3.
+	_, cidrNet, err := net.ParseCIDR(cidr)
+	if err != nil {
+		return false // If provided CIDR can't be parsed no IP will be in the subnet.
+	}
+	addr := net.ParseIP(ip)
+	return cidrNet.Contains(addr)
+}
+
 // Verify if given resource matches with policy statement.
 func bucketPolicyResourceMatch(resource string, statement policyStatement) bool {
 	// the resource rule for object could contain "*" wild card.
@@ -97,11 +110,14 @@ func bucketPolicyConditionMatch(conditions map[string]set.StringSet, statement p
 	// - StringNotEquals
 	// - StringLike
 	// - StringNotLike
+	// - IpAddress
+	// - NotIpAddress
 	//
 	// Supported applicable condition keys for each conditions.
 	// - s3:prefix
 	// - s3:max-keys
 	// - s3:aws-Referer
+	// - s3:aws-SourceIp
 
 	// The following loop evaluates the logical AND of all the
 	// conditions in the statement. Note: we can break out of the
@@ -159,6 +175,37 @@ func bucketPolicyConditionMatch(conditions map[string]set.StringSet, statement p
 					return false
 				}
 			}
+		} else if condition == "IpAddress" {
+			awsIps := conditionKeyVal["aws:SourceIp"]
+			// Skip empty condition, it is trivially satisfied.
+			if awsIps.IsEmpty() {
+				continue
+			}
+			// wildcard match of ip if statement was not empty.
+			// Find a valid ip.
+			ipFound := false
+			for ip := range conditions["ip"] {
+				if !awsIps.FuncMatch(isIPInCIDR, ip).IsEmpty() {
+					ipFound = true
+					break
+				}
+			}
+			if !ipFound {
+				return false
+			}
+		} else if condition == "NotIpAddress" {
+			awsIps := conditionKeyVal["aws:SourceIp"]
+			// Skip empty condition, it is trivially satisfied.
+			if awsIps.IsEmpty() {
+				continue
+			}
+			// wildcard match of ip if statement was not empty.
+			// Find if nothing matches.
+			for ip := range conditions["ip"] {
+				if !awsIps.FuncMatch(isIPInCIDR, ip).IsEmpty() {
+					return false
+				}
+			}
 		}
 	}
 

cmd/bucket-policy-handlers_test.go

@@ -843,7 +843,7 @@ func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName str
 
 // TestBucketPolicyConditionMatch - Tests to validate whether bucket policy conditions match.
 func TestBucketPolicyConditionMatch(t *testing.T) {
-	// obtain the inner map[string]set.StringSet for policyStatement.Conditions .
+	// obtain the inner map[string]set.StringSet for policyStatement.Conditions.
 	getInnerMap := func(key2, value string) map[string]set.StringSet {
 		innerMap := make(map[string]set.StringSet)
 		innerMap[key2] = set.CreateStringSet(value)
@@ -970,6 +970,34 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			condition:          getInnerMap("referer", "http://somethingelse.com/"),
 			expectedMatch:      true,
 		},
+		// Test case 13.
+		// IpAddress condition evaluates to true.
+		{
+			statementCondition: getStatementWithCondition("IpAddress", "aws:SourceIp", "54.240.143.0/24"),
+			condition:          getInnerMap("ip", "54.240.143.2"),
+			expectedMatch:      true,
+		},
+		// Test case 14.
+		// IpAddress condition evaluates to false.
+		{
+			statementCondition: getStatementWithCondition("IpAddress", "aws:SourceIp", "54.240.143.0/24"),
+			condition:          getInnerMap("ip", "127.240.143.224"),
+			expectedMatch:      false,
+		},
+		// Test case 15.
+		// NotIpAddress condition evaluates to true.
+		{
+			statementCondition: getStatementWithCondition("NotIpAddress", "aws:SourceIp", "54.240.143.0/24"),
+			condition:          getInnerMap("ip", "54.240.144.188"),
+			expectedMatch:      true,
+		},
+		// Test case 16.
+		// NotIpAddress condition evaluates to false.
+		{
+			statementCondition: getStatementWithCondition("NotIpAddress", "aws:SourceIp", "54.240.143.0/24"),
+			condition:          getInnerMap("ip", "54.240.143.243"),
+			expectedMatch:      false,
+		},
 	}
 
 	for i, tc := range testCases {

cmd/bucket-policy-parser.go

@@ -40,11 +40,11 @@ var supportedActionMap = set.CreateStringSet("*", "s3:*", "s3:GetObject",
 	"s3:AbortMultipartUpload", "s3:ListBucketMultipartUploads", "s3:ListMultipartUploadParts")
 
 // supported Conditions type.
-var supportedConditionsType = set.CreateStringSet("StringEquals", "StringNotEquals", "StringLike", "StringNotLike")
+var supportedConditionsType = set.CreateStringSet("StringEquals", "StringNotEquals", "StringLike", "StringNotLike", "IpAddress", "NotIpAddress")
 
 // Validate s3:prefix, s3:max-keys are present if not
 // supported keys for the conditions.
-var supportedConditionsKey = set.CreateStringSet("s3:prefix", "s3:max-keys", "aws:Referer")
+var supportedConditionsKey = set.CreateStringSet("s3:prefix", "s3:max-keys", "aws:Referer", "aws:SourceIp")
 
 // supportedEffectMap - supported effects.
 var supportedEffectMap = set.CreateStringSet("Allow", "Deny")

cmd/bucket-policy-parser_test.go

@@ -374,6 +374,12 @@ func TestIsValidConditions(t *testing.T) {
 	// returns map with the "StringNotLike" set to empty map.
 	setEmptyStringNotLike := getEmptyConditionKeyMap("StringNotLike")
 
+	// returns map with the "IpAddress" set to empty map.
+	setEmptyIPAddress := getEmptyConditionKeyMap("IpAddress")
+
+	// returns map with "NotIpAddress" set to empty map.
+	setEmptyNotIPAddress := getEmptyConditionKeyMap("NotIpAddress")
+
 	// Generate conditions.
 	generateConditions := func(key1, key2, value string) map[string]map[string]set.StringSet {
 		innerMap := make(map[string]set.StringSet)
@@ -427,6 +433,8 @@ func TestIsValidConditions(t *testing.T) {
 		setEmptyStringNotEquals(),
 		setEmptyStringLike(),
 		setEmptyStringNotLike(),
+		setEmptyIPAddress(),
+		setEmptyNotIPAddress(),
 		generateConditions("StringEquals", "s3:prefix", "Asia/"),
 		generateConditions("StringEquals", "s3:max-keys", "100"),
 		generateConditions("StringNotEquals", "s3:prefix", "Asia/"),
@@ -482,7 +490,13 @@ func TestIsValidConditions(t *testing.T) {
 		// Test case - 12.
 		{roBucketActionSet, testConditions[11], nil, true},
 		// Test case - 13.
-		{getObjectActionSet, testConditions[11], maxKeysConditionErr, false},
+		{roBucketActionSet, testConditions[12], nil, true},
+		// Test case - 11.
+		{roBucketActionSet, testConditions[13], nil, true},
+		// Test case - 12.
+		{roBucketActionSet, testConditions[14], nil, true},
+		// Test case - 13.
+		{getObjectActionSet, testConditions[15], maxKeysConditionErr, false},
 	}
 	for i, testCase := range testCases {
 		actualErr := isValidConditions(testCase.inputActions, testCase.inputCondition)
@@ -787,3 +801,65 @@ func TestAWSRefererCondition(t *testing.T) {
 		}
 	}
 }
+
+func TestAWSSourceIPCondition(t *testing.T) {
+	resource := set.CreateStringSet([]string{
+		fmt.Sprintf("%s%s", bucketARNPrefix, "minio-bucket"+"/"+"Asia"+"*"),
+	}...)
+
+	conditionsKeyMap := make(policy.ConditionKeyMap)
+	// Test both IPv4 and IPv6 addresses.
+	conditionsKeyMap.Add("aws:SourceIp",
+		set.CreateStringSet("54.240.143.0/24",
+			"2001:DB8:1234:5678::/64"))
+
+	requestConditionKeyMap := make(map[string]set.StringSet)
+	requestConditionKeyMap["ip"] = set.CreateStringSet("54.240.143.2")
+
+	testCases := []struct {
+		effect       string
+		conditionKey string
+		match        bool
+	}{
+		{
+			effect:       "Allow",
+			conditionKey: "IpAddress",
+			match:        true,
+		},
+		{
+			effect:       "Allow",
+			conditionKey: "NotIpAddress",
+			match:        false,
+		},
+		{
+			effect:       "Deny",
+			conditionKey: "IpAddress",
+			match:        true,
+		},
+		{
+			effect:       "Deny",
+			conditionKey: "NotIpAddress",
+			match:        false,
+		},
+	}
+
+	for i, test := range testCases {
+		conditions := make(map[string]map[string]set.StringSet)
+		conditions[test.conditionKey] = conditionsKeyMap
+
+		allowStatement := policyStatement{
+			Sid:    "Testing AWS referer condition",
+			Effect: test.effect,
+			Principal: map[string]interface{}{
+				"AWS": "*",
+			},
+			Resources:  resource,
+			Conditions: conditions,
+		}
+
+		if result := bucketPolicyConditionMatch(requestConditionKeyMap, allowStatement); result != test.match {
+			t.Errorf("Test %d -  Expected conditons to evaluate to %v but got %v",
+				i+1, test.match, result)
+		}
+	}
+}

cmd/object-handlers.go

@@ -48,16 +48,33 @@ func setGetRespHeaders(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
 //   GET Object: http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html
 func errAllowableObjectNotFound(bucket string, r *http.Request) APIErrorCode {
 	if getRequestAuthType(r) == authTypeAnonymous {
-		//we care about the bucket as a whole, not a particular resource
+		// We care about the bucket as a whole, not a particular resource.
 		resource := "/" + bucket
+		sourceIP := getSourceIPAddress(r)
 		if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", resource,
-			r.Referer(), r.URL.Query()); s3Error != ErrNone {
+			r.Referer(), sourceIP, r.URL.Query()); s3Error != ErrNone {
 			return ErrAccessDenied
 		}
 	}
@@ -505,8 +522,9 @@ 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)
 		if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL.Path,
-			r.Referer(), r.URL.Query()); s3Error != ErrNone {
+			r.Referer(), sourceIP, r.URL.Query()); s3Error != ErrNone {
 			writeErrorResponse(w, s3Error, r.URL)
 			return
 		}
@@ -791,8 +809,9 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
 		return
 	case authTypeAnonymous:
 		// http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html
+		sourceIP := getSourceIPAddress(r)
 		if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL.Path,
-			r.Referer(), r.URL.Query()); s3Error != ErrNone {
+			r.Referer(), sourceIP, r.URL.Query()); s3Error != ErrNone {
 			writeErrorResponse(w, s3Error, r.URL)
 			return
 		}

cmd/object-handlers_test.go

@@ -3552,3 +3552,56 @@ 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)
+		}
+	}
+}

docs/bucket/policy/README.md

@@ -24,12 +24,15 @@ This package implements parsing and validating bucket access policies based on A
     StringNotEquals
     StringLike
     StringNotLike
+    IpAddress
+    NotIpAddress
 
 Supported applicable condition keys for each conditions.
 
     s3:prefix
     s3:max-keys
     aws:Referer
+    aws:SourceIp
 
 ### Nested policy support.