Fix policy bugs Null conditions and canonical names (#7021)

This PR fixes two different issues

- Null condition implementation
- HTTP Canonical request value names

This PR fixes handling of null conditions and
handle HTTP canonical names in request values.

This PR was tested with policies mentioned in the following blog
https://aws.amazon.com/blogs/security/how-to-prevent-uploads-of-unencrypted-objects-to-amazon-s3/

Fixes #6955

pkg/policy/condition/func_test.go

@@ -53,12 +53,12 @@ func TestFunctionsEvaluate(t *testing.T) {
 		{case1Function, map[string][]string{
 			"x-amz-copy-source": {"mybucket/myobject"},
 			"SourceIp":          {"192.168.1.10"},
-		}, true},
+		}, false},
 		{case1Function, map[string][]string{
 			"x-amz-copy-source": {"mybucket/myobject"},
 			"SourceIp":          {"192.168.1.10"},
 			"Refer":             {"http://example.org/"},
-		}, true},
+		}, false},
 		{case1Function, map[string][]string{"x-amz-copy-source": {"mybucket/myobject"}}, false},
 		{case1Function, map[string][]string{"SourceIp": {"192.168.1.10"}}, false},
 		{case1Function, map[string][]string{
@@ -79,7 +79,7 @@ func TestFunctionsEvaluate(t *testing.T) {
 		result := testCase.functions.Evaluate(testCase.values)
 
 		if result != testCase.expectedResult {
-			t.Fatalf("case %v: expected: %v, got: %v\n", i+1, testCase.expectedResult, result)
+			t.Errorf("case %v: expected: %v, got: %v\n", i+1, testCase.expectedResult, result)
 		}
 	}
 }

pkg/policy/condition/ipaddressfunc.go

@@ -19,6 +19,7 @@ package condition
 import (
 	"fmt"
 	"net"
+	"net/http"
 	"sort"
 )
 
@@ -46,7 +47,12 @@ type ipAddressFunc struct {
 // falls in one of network or not.
 func (f ipAddressFunc) evaluate(values map[string][]string) bool {
 	IPs := []net.IP{}
-	for _, s := range values[f.k.Name()] {
+	requestValue, ok := values[http.CanonicalHeaderKey(f.k.Name())]
+	if !ok {
+		requestValue = values[f.k.Name()]
+	}
+
+	for _, s := range requestValue {
 		IP := net.ParseIP(s)
 		if IP == nil {
 			panic(fmt.Errorf("invalid IP address '%v'", s))

pkg/policy/condition/nullfunc.go

@@ -18,17 +18,19 @@ package condition
 
 import (
 	"fmt"
+	"net/http"
 	"reflect"
 	"strconv"
 )
 
-// nullFunc - Null condition function. It checks whether Key is present in given
+// nullFunc - Null condition function. It checks whether Key is not present in given
 // values or not.
 // For example,
 //   1. if Key = S3XAmzCopySource and Value = true, at evaluate() it returns whether
-//      S3XAmzCopySource is in given value map or not.
-//   2. if Key = S3XAmzCopySource and Value = false, at evaluate() it returns whether
 //      S3XAmzCopySource is NOT in given value map or not.
+//   2. if Key = S3XAmzCopySource and Value = false, at evaluate() it returns whether
+//      S3XAmzCopySource is in given value map or not.
+// https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition_operators.html#Conditions_Null
 type nullFunc struct {
 	k     Key
 	value bool
@@ -37,13 +39,16 @@ type nullFunc struct {
 // evaluate() - evaluates to check whether Key is present in given values or not.
 // Depending on condition boolean value, this function returns true or false.
 func (f nullFunc) evaluate(values map[string][]string) bool {
-	requestValue := values[f.k.Name()]
+	requestValue, ok := values[http.CanonicalHeaderKey(f.k.Name())]
+	if !ok {
+		requestValue = values[f.k.Name()]
+	}
 
 	if f.value {
-		return len(requestValue) != 0
+		return len(requestValue) == 0
 	}
 
-	return len(requestValue) == 0
+	return len(requestValue) != 0
 }
 
 // key() - returns condition key which is used by this condition function.

pkg/policy/condition/nullfunc_test.go

@@ -37,23 +37,23 @@ func TestNullFuncEvaluate(t *testing.T) {
 		values         map[string][]string
 		expectedResult bool
 	}{
-		{case1Function, map[string][]string{"prefix": {"true"}}, true},
-		{case1Function, map[string][]string{"prefix": {"false"}}, true},
-		{case1Function, map[string][]string{"prefix": {"mybucket/foo"}}, true},
-		{case1Function, map[string][]string{}, false},
-		{case1Function, map[string][]string{"delimiter": {"/"}}, false},
-		{case2Function, map[string][]string{"prefix": {"true"}}, false},
-		{case2Function, map[string][]string{"prefix": {"false"}}, false},
-		{case2Function, map[string][]string{"prefix": {"mybucket/foo"}}, false},
-		{case2Function, map[string][]string{}, true},
-		{case2Function, map[string][]string{"delimiter": {"/"}}, true},
+		{case1Function, map[string][]string{"prefix": {"true"}}, false},
+		{case1Function, map[string][]string{"prefix": {"false"}}, false},
+		{case1Function, map[string][]string{"prefix": {"mybucket/foo"}}, false},
+		{case1Function, map[string][]string{}, true},
+		{case1Function, map[string][]string{"delimiter": {"/"}}, true},
+		{case2Function, map[string][]string{"prefix": {"true"}}, true},
+		{case2Function, map[string][]string{"prefix": {"false"}}, true},
+		{case2Function, map[string][]string{"prefix": {"mybucket/foo"}}, true},
+		{case2Function, map[string][]string{}, false},
+		{case2Function, map[string][]string{"delimiter": {"/"}}, false},
 	}
 
 	for i, testCase := range testCases {
 		result := testCase.function.evaluate(testCase.values)
 
 		if result != testCase.expectedResult {
-			t.Fatalf("case %v: expected: %v, got: %v\n", i+1, testCase.expectedResult, result)
+			t.Errorf("case %v: expected: %v, got: %v\n", i+1, testCase.expectedResult, result)
 		}
 	}
 }

pkg/policy/condition/stringequalsfunc.go

@@ -18,6 +18,7 @@ package condition
 
 import (
 	"fmt"
+	"net/http"
 	"sort"
 	"strings"
 
@@ -44,7 +45,11 @@ type stringEqualsFunc struct {
 // evaluate() - evaluates to check whether value by Key in given values is in
 // condition values.
 func (f stringEqualsFunc) evaluate(values map[string][]string) bool {
-	requestValue := values[f.k.Name()]
+	requestValue, ok := values[http.CanonicalHeaderKey(f.k.Name())]
+	if !ok {
+		requestValue = values[f.k.Name()]
+	}
+
 	return !f.values.Intersection(set.CreateStringSet(requestValue...)).IsEmpty()
 }
 

pkg/policy/condition/stringlikefunc.go

@@ -18,6 +18,7 @@ package condition
 
 import (
 	"fmt"
+	"net/http"
 	"sort"
 	"strings"
 
@@ -45,7 +46,12 @@ type stringLikeFunc struct {
 // evaluate() - evaluates to check whether value by Key in given values is wildcard
 // matching in condition values.
 func (f stringLikeFunc) evaluate(values map[string][]string) bool {
-	for _, v := range values[f.k.Name()] {
+	requestValue, ok := values[http.CanonicalHeaderKey(f.k.Name())]
+	if !ok {
+		requestValue = values[f.k.Name()]
+	}
+
+	for _, v := range requestValue {
 		if !f.values.FuncMatch(wildcard.Match, v).IsEmpty() {
 			return true
 		}