diff --git a/api/handler/acl.go b/api/handler/acl.go index 4ba2f8cb..780b2127 100644 --- a/api/handler/acl.go +++ b/api/handler/acl.go @@ -1,6 +1,7 @@ package handler import ( + "bytes" "context" "crypto/ecdsa" "crypto/elliptic" @@ -56,6 +57,8 @@ var ( errInvalidPublicKey = errors.New("invalid public key") ) +var rawWildcardJSON = []byte(`"*"`) + const ( arnAwsPrefix = "arn:aws:s3:::" allUsersWildcard = "*" @@ -102,11 +105,11 @@ type bucketPolicy struct { } type statement struct { - Sid string `json:"Sid"` - Effect string `json:"Effect"` - Principal principal `json:"Principal"` - Action []string `json:"Action"` - Resource []string `json:"Resource"` + Sid string `json:"Sid"` + Effect string `json:"Effect"` + Principal principal `json:"Principal"` + Action stringOrSlice `json:"Action"` + Resource stringOrSlice `json:"Resource"` } type principal struct { @@ -114,6 +117,29 @@ type principal struct { CanonicalUser string `json:"CanonicalUser,omitempty"` } +// principalModel is a copy of principal to avoid infinity recursion in principal.UnmarshalJSON. +type principalModel struct { + AWS string `json:"AWS,omitempty"` + CanonicalUser string `json:"CanonicalUser,omitempty"` +} + +func (s *principal) UnmarshalJSON(bts []byte) error { + if len(bts) == 3 && bytes.Equal(bts, rawWildcardJSON) { + s.AWS = allUsersWildcard + return nil + } + + var pm principalModel + if err := json.Unmarshal(bts, &pm); err != nil { + return err + } + + s.AWS = pm.AWS + s.CanonicalUser = pm.CanonicalUser + + return nil +} + type orderedAstResource struct { Index int Resource *astResource @@ -1009,8 +1035,7 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) { if state.Principal.AWS == allUsersWildcard { groupGrantee = true } - - for _, resource := range state.Resource { + for _, resource := range state.Resource.values { trimmedResource := strings.TrimPrefix(resource, arnAwsPrefix) r, ok := rr[trimmedResource] if !ok { @@ -1022,7 +1047,7 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) { resourceInfo: resourceInfoFromName(trimmedResource, bktPolicy.Bucket), } } - for _, action := range state.Action { + for _, action := range state.Action.values { for _, op := range actionToOpMap[action] { toAction := effectToAction(state.Effect) r.Operations = addTo(r.Operations, state.Principal.CanonicalUser, op, groupGrantee, toAction) @@ -1103,8 +1128,8 @@ func handleResourceOperations(bktPolicy *bucketPolicy, list []*astOperation, eac state := statement{ Effect: actionToEffect(eaclAction), Principal: principal{CanonicalUser: user}, - Action: actions, - Resource: []string{arnAwsPrefix + resourceName}, + Action: stringOrSlice{values: actions}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resourceName}}, } if user == allUsersGroup { state.Principal = principal{AWS: allUsersWildcard} @@ -1247,8 +1272,8 @@ func getAllowStatement(resInfo *resourceInfo, id string, permission amazonS3Perm Principal: principal{ CanonicalUser: id, }, - Action: getActions(permission, resInfo.IsBucket()), - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: getActions(permission, resInfo.IsBucket())}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, } if id == allUsersWildcard { @@ -1264,8 +1289,8 @@ func getDenyStatement(resInfo *resourceInfo, id string, permission amazonS3Permi Principal: principal{ CanonicalUser: id, }, - Action: getActions(permission, resInfo.IsBucket()), - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: getActions(permission, resInfo.IsBucket())}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, } if id == allUsersWildcard { diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go index ed0bf4fe..4c19cc91 100644 --- a/api/handler/acl_test.go +++ b/api/handler/acl_test.go @@ -103,16 +103,16 @@ func TestPolicyToAst(t *testing.T) { { Effect: "Allow", Principal: principal{AWS: allUsersWildcard}, - Action: []string{"s3:PutObject"}, - Resource: []string{"arn:aws:s3:::bucketName"}, + Action: stringOrSlice{values: []string{"s3:PutObject"}}, + Resource: stringOrSlice{values: []string{"arn:aws:s3:::bucketName"}}, }, { Effect: "Deny", Principal: principal{ CanonicalUser: hex.EncodeToString(key.PublicKey().Bytes()), }, - Action: []string{"s3:GetObject"}, - Resource: []string{"arn:aws:s3:::bucketName/object"}, + Action: stringOrSlice{values: []string{"s3:GetObject"}}, + Resource: stringOrSlice{values: []string{"arn:aws:s3:::bucketName/object"}}, }}, } policy.Bucket = "bucketName" @@ -746,22 +746,22 @@ func TestBucketAclToPolicy(t *testing.T) { Principal: principal{ CanonicalUser: id, }, - Action: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads", "s3:PutObject", "s3:DeleteObject"}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads", "s3:PutObject", "s3:DeleteObject"}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, { Effect: "Allow", Principal: principal{AWS: allUsersWildcard}, - Action: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads"}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads"}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, { Effect: "Allow", Principal: principal{ CanonicalUser: id2, }, - Action: []string{"s3:PutObject", "s3:DeleteObject"}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{"s3:PutObject", "s3:DeleteObject"}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, }, } @@ -819,22 +819,22 @@ func TestObjectAclToPolicy(t *testing.T) { Principal: principal{ CanonicalUser: id, }, - Action: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, { Effect: "Allow", Principal: principal{ CanonicalUser: id2, }, - Action: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, { Effect: "Allow", Principal: principal{AWS: allUsersWildcard}, - Action: []string{s3GetObject, s3GetObjectVersion}, - Resource: []string{arnAwsPrefix + resInfo.Name()}, + Action: stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}}, }, }, } @@ -1343,11 +1343,11 @@ func TestBucketPolicy(t *testing.T) { for _, st := range bktPolicy.Statement { if st.Effect == "Allow" { require.Equal(t, hex.EncodeToString(key.PublicKey().Bytes()), st.Principal.CanonicalUser) - require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource) + require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values) } else { require.Equal(t, allUsersWildcard, st.Principal.AWS) require.Equal(t, "Deny", st.Effect) - require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource) + require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values) } } @@ -1355,21 +1355,21 @@ func TestBucketPolicy(t *testing.T) { Statement: []statement{{ Effect: "Allow", Principal: principal{AWS: allUsersWildcard}, - Action: []string{s3GetObject}, - Resource: []string{arnAwsPrefix + "dummy"}, + Action: stringOrSlice{values: []string{s3GetObject}}, + Resource: stringOrSlice{values: []string{arnAwsPrefix + "dummy"}}, }}, } putBucketPolicy(hc, bktName, newPolicy, box, http.StatusInternalServerError) - newPolicy.Statement[0].Resource[0] = arnAwsPrefix + bktName + newPolicy.Statement[0].Resource.values[0] = arnAwsPrefix + bktName putBucketPolicy(hc, bktName, newPolicy, box, http.StatusOK) bktPolicy = getBucketPolicy(hc, bktName) for _, st := range bktPolicy.Statement { if st.Effect == "Allow" && st.Principal.AWS == allUsersWildcard { - require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource) - require.ElementsMatch(t, []string{s3GetObject, s3ListBucket}, st.Action) + require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values) + require.ElementsMatch(t, []string{s3GetObject, s3ListBucket}, st.Action.values) } } } @@ -1546,3 +1546,36 @@ func TestEACLEncode(t *testing.T) { require.Contains(t, acp.AccessControlList, g) } } + +func TestPrincipal(t *testing.T) { + type testCase struct { + Principal principal `json:"p"` + } + + t.Run("wildcard", func(t *testing.T) { + payload := []byte(`{"p":"*"}`) + + var testObj testCase + require.NoError(t, json.Unmarshal(payload, &testObj)) + require.Equal(t, allUsersWildcard, testObj.Principal.AWS) + + bts, err := json.Marshal(testObj) + require.NoError(t, err) + + // we should be able to unmarshal string and structs, but marshals always a struct. + payload = []byte(`{"p":{"AWS":"*"}}`) + require.Equal(t, payload, bts) + }) + + t.Run("wildcard in struct", func(t *testing.T) { + payload := []byte(`{"p":{"AWS":"*"}}`) + + var testObj testCase + require.NoError(t, json.Unmarshal(payload, &testObj)) + require.Equal(t, allUsersWildcard, testObj.Principal.AWS) + + bts, err := json.Marshal(testObj) + require.NoError(t, err) + require.Equal(t, payload, bts) + }) +} diff --git a/api/handler/json_types.go b/api/handler/json_types.go new file mode 100644 index 00000000..6795bca7 --- /dev/null +++ b/api/handler/json_types.go @@ -0,0 +1,39 @@ +package handler + +import ( + "encoding/json" +) + +type ( + stringOrSlice struct { + values []string + } +) + +func (s *stringOrSlice) UnmarshalJSON(bytes []byte) error { + if err := json.Unmarshal(bytes, &s.values); err == nil { + return nil + } + + var str string + if err := json.Unmarshal(bytes, &str); err != nil { + return err + } + + s.values = []string{str} + return nil +} + +func (s stringOrSlice) MarshalJSON() ([]byte, error) { + values := s.values + if values == nil { + values = []string{} + } + + if len(s.values) == 1 { + str := s.values[0] + return json.Marshal(&str) + } + + return json.Marshal(values) +} diff --git a/api/handler/json_types_test.go b/api/handler/json_types_test.go new file mode 100644 index 00000000..a515426e --- /dev/null +++ b/api/handler/json_types_test.go @@ -0,0 +1,108 @@ +package handler + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +type ( + testCase struct { + Action stringOrSlice `json:"action"` + } +) + +func TestStringOrSlice(t *testing.T) { + t.Run("string", func(t *testing.T) { + var ( + payload = []byte(`{"action":"s3:PutObject"}`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Equal(t, []string{"s3:PutObject"}, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + require.Equal(t, payload, marshaled) + }) + + t.Run("wildcard", func(t *testing.T) { + var ( + payload = []byte(`{"action":"*"}`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Equal(t, []string{"*"}, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + require.Equal(t, payload, marshaled) + }) + + t.Run("slice", func(t *testing.T) { + var ( + payload = []byte(`{"action":["s3:PutObject","s3:GetObject"]}`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Equal(t, []string{"s3:PutObject", "s3:GetObject"}, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + require.Equal(t, payload, marshaled) + }) + + t.Run("single element slice", func(t *testing.T) { + var ( + payload = []byte(`{"action":["s3:PutObject"]}`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Equal(t, []string{"s3:PutObject"}, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + + // if only one element, marshalling makes it as a string, not a slice. + singleElementPayload := []byte(`{"action":"s3:PutObject"}`) + require.Equal(t, singleElementPayload, marshaled) + }) + + t.Run("single element slice, with spaces", func(t *testing.T) { + var ( + payload = []byte(`{ "action" : [ "s3:PutObject" ] }`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Equal(t, []string{"s3:PutObject"}, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + + // if only one element, marshalling makes it as a string, not a slice. + singleElementPayload := []byte(`{"action":"s3:PutObject"}`) + require.Equal(t, singleElementPayload, marshaled) + }) + + t.Run("nil slice", func(t *testing.T) { + var ( + payload = []byte(`{}`) + obj testCase + ) + + require.NoError(t, json.Unmarshal(payload, &obj)) + require.Nil(t, obj.Action.values) + + marshaled, err := json.Marshal(obj) + require.NoError(t, err) + + singleElementPayload := []byte(`{"action":[]}`) + require.Equal(t, singleElementPayload, marshaled) + }) +} diff --git a/docs/aws_s3_compat.md b/docs/aws_s3_compat.md index e731bd21..6fd89168 100644 --- a/docs/aws_s3_compat.md +++ b/docs/aws_s3_compat.md @@ -36,8 +36,37 @@ Reference: For now there are some limitations: * [Bucket policy](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucket-policies.html) supports only one `Principal` per `Statement`. -Principal must be `"AWS": "*"` (to refer all users) or `"CanonicalUser": "0313b1ac3a8076e155a7e797b24f0b650cccad5941ea59d7cfd51a024a8b2a06bf"` (hex encoded public key of desired user). -* Resource in bucket policy is an array. Each item MUST contain bucket name, CAN contain object name (wildcards are not supported): +Principal must be `"AWS": "*"` or `"*"` (to refer all users) or `"CanonicalUser": "0313b1ac3a8076e155a7e797b24f0b650cccad5941ea59d7cfd51a024a8b2a06bf"` (hex encoded public key of desired user). +```json +{ + "Statement": [ + { + "Principal": "*" + } + ] +} +``` +```json +{ + "Statement": [ + { + "Principal": { + "AWS": "*" + } + } + ] +} +``` +* Resource in bucket policy is a string value or array of strings. Each item MUST contain bucket name, CAN contain object name (wildcards are not supported): +```json +{ + "Statement": [ + { + "Resource": "arn:aws:s3:::bucket" + } + ] +} +``` ```json { "Statement": [ @@ -49,6 +78,26 @@ Principal must be `"AWS": "*"` (to refer all users) or `"CanonicalUser": "0313b1 } ] } + +``` +* Action is a string value or array of strings: +```json +{ + "Statement": [ + { + "Action": "s3:PutObject" + } + ] +} +``` +```json +{ + "Statement": [ + { + "Action": ["s3:PutObject", "s3:PutObjectAcl"] + } + ] +} ``` * AWS conditions and wildcard are not supported in [resources](https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-arn-format.html) * Only `CanonicalUser` (with hex encoded public key) and `All Users Group` are supported in [ACL](https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html).