diff --git a/cmd/admin-router.go b/cmd/admin-router.go index 81efe7e7d..d262a7775 100644 --- a/cmd/admin-router.go +++ b/cmd/admin-router.go @@ -110,10 +110,9 @@ func registerAdminRouter(router *mux.Router, enableConfigOps, enableIAMOps bool) // -- IAM APIs -- // Add policy IAM - adminRouter.Methods(http.MethodPut).Path(adminVersion+"/add-canned-policy").HandlerFunc(httpTraceHdrs(adminAPI.AddCannedPolicy)).Queries("name", "{name:.*}") + adminRouter.Methods(http.MethodPut).Path(adminVersion+"/add-canned-policy").HandlerFunc(httpTraceAll(adminAPI.AddCannedPolicy)).Queries("name", "{name:.*}") // Add user IAM - adminRouter.Methods(http.MethodGet).Path(adminVersion + "/accountinfo").HandlerFunc(httpTraceAll(adminAPI.AccountInfoHandler)) adminRouter.Methods(http.MethodPut).Path(adminVersion+"/add-user").HandlerFunc(httpTraceHdrs(adminAPI.AddUser)).Queries("accessKey", "{accessKey:.*}") diff --git a/pkg/bucket/policy/condition/key.go b/pkg/bucket/policy/condition/key.go index 39f8b554c..2538e3cb0 100644 --- a/pkg/bucket/policy/condition/key.go +++ b/pkg/bucket/policy/condition/key.go @@ -124,6 +124,7 @@ var AllSupportedKeys = append([]Key{ S3Prefix, S3Delimiter, S3MaxKeys, + S3VersionID, S3ObjectLockRemainingRetentionDays, S3ObjectLockMode, S3ObjectLockLegalHold, @@ -143,6 +144,8 @@ var AllSupportedKeys = append([]Key{ // CommonKeys - is list of all common condition keys. var CommonKeys = append([]Key{ + S3XAmzContentSha256, + S3LocationConstraint, AWSReferer, AWSSourceIP, AWSUserAgent, @@ -152,7 +155,6 @@ var CommonKeys = append([]Key{ AWSPrincipalType, AWSUserID, AWSUsername, - S3XAmzContentSha256, LDAPUser, }, JWTKeys...) @@ -241,6 +243,13 @@ func (set KeySet) Add(key Key) { set[key] = struct{}{} } +// Merge merges two key sets, duplicates are overwritten +func (set KeySet) Merge(mset KeySet) { + for k, v := range mset { + set[k] = v + } +} + // Difference - returns a key set contains difference of two keys. // Example: // keySet1 := ["one", "two", "three"] diff --git a/pkg/iam/policy/action.go b/pkg/iam/policy/action.go index 12a92a207..5f6c3a7f1 100644 --- a/pkg/iam/policy/action.go +++ b/pkg/iam/policy/action.go @@ -266,23 +266,45 @@ var supportedObjectActions = map[Action]struct{}{ // isObjectAction - returns whether action is object type or not. func (action Action) isObjectAction() bool { - _, ok := supportedObjectActions[action] - return ok + for supAction := range supportedObjectActions { + if action.Match(supAction) { + return true + } + } + return false } -// Match - matches object name with resource pattern. +// Match - matches action name with action patter. func (action Action) Match(a Action) bool { return wildcard.Match(string(action), string(a)) } // IsValid - checks if action is valid or not. func (action Action) IsValid() bool { - _, ok := supportedActions[action] - return ok + for supAction := range supportedActions { + if action.Match(supAction) { + return true + } + } + return false } -// actionConditionKeyMap - holds mapping of supported condition key for an action. -var actionConditionKeyMap = map[Action]condition.KeySet{ +type actionConditionKeyMap map[Action]condition.KeySet + +func (a actionConditionKeyMap) Lookup(action Action) (condition.KeySet, bool) { + var ckeysMerged = condition.KeySet{} + var found bool + for act, ckey := range a { + if action.Match(act) { + ckeysMerged.Merge(ckey) + found = true + } + } + return ckeysMerged, found +} + +// iamActionConditionKeyMap - holds mapping of supported condition key for an action. +var iamActionConditionKeyMap = actionConditionKeyMap{ AllActions: condition.NewKeySet(condition.AllSupportedKeys...), AbortMultipartUploadAction: condition.NewKeySet(condition.CommonKeys...), @@ -291,8 +313,6 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ DeleteBucketPolicyAction: condition.NewKeySet(condition.CommonKeys...), - DeleteObjectAction: condition.NewKeySet(condition.CommonKeys...), - GetBucketLocationAction: condition.NewKeySet(condition.CommonKeys...), GetBucketNotificationAction: condition.NewKeySet(condition.CommonKeys...), @@ -303,6 +323,7 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ append([]condition.Key{ condition.S3XAmzServerSideEncryption, condition.S3XAmzServerSideEncryptionCustomerAlgorithm, + condition.S3VersionID, }, condition.CommonKeys...)...), HeadBucketAction: condition.NewKeySet(condition.CommonKeys...), @@ -335,6 +356,11 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ PutBucketPolicyAction: condition.NewKeySet(condition.CommonKeys...), + DeleteObjectAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + PutObjectAction: condition.NewKeySet( append([]condition.Key{ condition.S3XAmzCopySource, @@ -342,6 +368,7 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ condition.S3XAmzServerSideEncryptionCustomerAlgorithm, condition.S3XAmzMetadataDirective, condition.S3XAmzStorageClass, + condition.S3VersionID, condition.S3ObjectLockRetainUntilDate, condition.S3ObjectLockMode, condition.S3ObjectLockLegalHold, @@ -351,21 +378,32 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ // LockLegalHold is not supported with PutObjectRetentionAction PutObjectRetentionAction: condition.NewKeySet( append([]condition.Key{ + condition.S3XAmzServerSideEncryption, + condition.S3XAmzServerSideEncryptionCustomerAlgorithm, condition.S3ObjectLockRemainingRetentionDays, condition.S3ObjectLockRetainUntilDate, condition.S3ObjectLockMode, + condition.S3VersionID, + }, condition.CommonKeys...)...), + GetObjectRetentionAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3XAmzServerSideEncryption, + condition.S3XAmzServerSideEncryptionCustomerAlgorithm, + condition.S3VersionID, }, condition.CommonKeys...)...), - - GetObjectRetentionAction: condition.NewKeySet(condition.CommonKeys...), PutObjectLegalHoldAction: condition.NewKeySet( append([]condition.Key{ + condition.S3XAmzServerSideEncryption, + condition.S3XAmzServerSideEncryptionCustomerAlgorithm, condition.S3ObjectLockLegalHold, + condition.S3VersionID, }, condition.CommonKeys...)...), GetObjectLegalHoldAction: condition.NewKeySet(condition.CommonKeys...), // https://docs.aws.amazon.com/AmazonS3/latest/dev/list_amazons3.html BypassGovernanceRetentionAction: condition.NewKeySet( append([]condition.Key{ + condition.S3VersionID, condition.S3ObjectLockRemainingRetentionDays, condition.S3ObjectLockRetainUntilDate, condition.S3ObjectLockMode, @@ -376,11 +414,24 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ PutBucketObjectLockConfigurationAction: condition.NewKeySet(condition.CommonKeys...), GetBucketTaggingAction: condition.NewKeySet(condition.CommonKeys...), PutBucketTaggingAction: condition.NewKeySet(condition.CommonKeys...), - PutObjectTaggingAction: condition.NewKeySet(condition.CommonKeys...), - GetObjectTaggingAction: condition.NewKeySet(condition.CommonKeys...), - DeleteObjectTaggingAction: condition.NewKeySet(condition.CommonKeys...), - PutObjectVersionTaggingAction: condition.NewKeySet(condition.CommonKeys...), + PutObjectTaggingAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + GetObjectTaggingAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + DeleteObjectTaggingAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + + PutObjectVersionTaggingAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), GetObjectVersionAction: condition.NewKeySet( append([]condition.Key{ condition.S3VersionID, @@ -397,10 +448,22 @@ var actionConditionKeyMap = map[Action]condition.KeySet{ append([]condition.Key{ condition.S3VersionID, }, condition.CommonKeys...)...), - GetReplicationConfigurationAction: condition.NewKeySet(condition.CommonKeys...), - PutReplicationConfigurationAction: condition.NewKeySet(condition.CommonKeys...), - ReplicateObjectAction: condition.NewKeySet(condition.CommonKeys...), - ReplicateDeleteAction: condition.NewKeySet(condition.CommonKeys...), - ReplicateTagsAction: condition.NewKeySet(condition.CommonKeys...), - GetObjectVersionForReplicationAction: condition.NewKeySet(condition.CommonKeys...), + GetReplicationConfigurationAction: condition.NewKeySet(condition.CommonKeys...), + PutReplicationConfigurationAction: condition.NewKeySet(condition.CommonKeys...), + ReplicateObjectAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + ReplicateDeleteAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + ReplicateTagsAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), + GetObjectVersionForReplicationAction: condition.NewKeySet( + append([]condition.Key{ + condition.S3VersionID, + }, condition.CommonKeys...)...), } diff --git a/pkg/iam/policy/actionset.go b/pkg/iam/policy/actionset.go index d780b28af..ac75d9879 100644 --- a/pkg/iam/policy/actionset.go +++ b/pkg/iam/policy/actionset.go @@ -43,6 +43,15 @@ func (actionSet ActionSet) Match(action Action) bool { if r.Match(action) { return true } + + // This is a special case where GetObjectVersion + // means GetObject is enabled implicitly. + switch r { + case GetObjectVersionAction: + if action == GetObjectAction { + return true + } + } } return false diff --git a/pkg/iam/policy/policy_test.go b/pkg/iam/policy/policy_test.go index 19fe8a120..ed41946f1 100644 --- a/pkg/iam/policy/policy_test.go +++ b/pkg/iam/policy/policy_test.go @@ -20,7 +20,9 @@ import ( "encoding/json" "net" "reflect" + "strings" "testing" + "time" "github.com/minio/minio-go/v7/pkg/set" "github.com/minio/minio/pkg/bucket/policy" @@ -432,6 +434,259 @@ func TestPolicyIsValid(t *testing.T) { } } +// Parse config with location constraints +func TestPolicyParseConfig(t *testing.T) { + policy1LocationConstraint := `{ + "Version":"2012-10-17", + "Statement":[ + { + "Sid":"statement1", + "Effect":"Allow", + "Action": "s3:CreateBucket", + "Resource": "arn:aws:s3:::*", + "Condition": { + "StringLike": { + "s3:LocationConstraint": "us-east-1" + } + } + }, + { + "Sid":"statement2", + "Effect":"Deny", + "Action": "s3:CreateBucket", + "Resource": "arn:aws:s3:::*", + "Condition": { + "StringNotLike": { + "s3:LocationConstraint": "us-east-1" + } + } + } + ] +}` + policy2Condition := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "statement1", + "Effect": "Allow", + "Action": "s3:GetObjectVersion", + "Resource": "arn:aws:s3:::test/HappyFace.jpg" + }, + { + "Sid": "statement2", + "Effect": "Deny", + "Action": "s3:GetObjectVersion", + "Resource": "arn:aws:s3:::test/HappyFace.jpg", + "Condition": { + "StringNotEquals": { + "s3:versionid": "AaaHbAQitwiL_h47_44lRO2DDfLlBO5e" + } + } + } + ] +}` + + policy3ConditionActionRegex := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "statement2", + "Effect": "Allow", + "Action": "s3:Get*", + "Resource": "arn:aws:s3:::test/HappyFace.jpg", + "Condition": { + "StringEquals": { + "s3:versionid": "AaaHbAQitwiL_h47_44lRO2DDfLlBO5e" + } + } + } + ] +}` + + policy4ConditionAction := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "statement2", + "Effect": "Allow", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::test/HappyFace.jpg", + "Condition": { + "StringEquals": { + "s3:versionid": "AaaHbAQitwiL_h47_44lRO2DDfLlBO5e" + } + } + } + ] +}` + + policy5ConditionCurrenTime := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:Get*", + "s3:Put*" + ], + "Resource": [ + "arn:aws:s3:::test/*" + ], + "Condition": { + "DateGreaterThan": { + "aws:CurrentTime": [ + "2017-02-28T00:00:00Z" + ] + } + } + } + ] +}` + + policy5ConditionCurrenTimeLesser := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:Get*", + "s3:Put*" + ], + "Resource": [ + "arn:aws:s3:::test/*" + ], + "Condition": { + "DateLessThan": { + "aws:CurrentTime": [ + "2017-02-28T00:00:00Z" + ] + } + } + } + ] +}` + + tests := []struct { + p string + args Args + allowed bool + }{ + { + p: policy1LocationConstraint, + allowed: true, + args: Args{ + AccountName: "allowed", + Action: CreateBucketAction, + BucketName: "test", + ConditionValues: map[string][]string{"LocationConstraint": {"us-east-1"}}, + }, + }, + { + p: policy1LocationConstraint, + allowed: false, + args: Args{ + AccountName: "disallowed", + Action: CreateBucketAction, + BucketName: "test", + ConditionValues: map[string][]string{"LocationConstraint": {"us-east-2"}}, + }, + }, + { + p: policy2Condition, + allowed: true, + args: Args{ + AccountName: "allowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{"versionid": {"AaaHbAQitwiL_h47_44lRO2DDfLlBO5e"}}, + }, + }, + { + p: policy2Condition, + allowed: false, + args: Args{ + AccountName: "disallowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{"versionid": {"AaaHbAQitwiL_h47_44lRO2DDfLlBO5f"}}, + }, + }, + { + p: policy3ConditionActionRegex, + allowed: true, + args: Args{ + AccountName: "allowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{"versionid": {"AaaHbAQitwiL_h47_44lRO2DDfLlBO5e"}}, + }, + }, + { + p: policy3ConditionActionRegex, + allowed: false, + args: Args{ + AccountName: "disallowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{"versionid": {"AaaHbAQitwiL_h47_44lRO2DDfLlBO5f"}}, + }, + }, + { + p: policy4ConditionAction, + allowed: true, + args: Args{ + AccountName: "allowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{"versionid": {"AaaHbAQitwiL_h47_44lRO2DDfLlBO5e"}}, + }, + }, + { + p: policy5ConditionCurrenTime, + allowed: true, + args: Args{ + AccountName: "allowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{ + "CurrentTime": {time.Now().Format(time.RFC3339)}, + }, + }, + }, + { + p: policy5ConditionCurrenTimeLesser, + allowed: false, + args: Args{ + AccountName: "disallowed", + Action: GetObjectAction, + BucketName: "test", + ObjectName: "HappyFace.jpg", + ConditionValues: map[string][]string{ + "CurrentTime": {time.Now().Format(time.RFC3339)}, + }, + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.args.AccountName, func(t *testing.T) { + ip, err := ParseConfig(strings.NewReader(test.p)) + if err != nil { + t.Error(err) + } + if got := ip.IsAllowed(test.args); got != test.allowed { + t.Errorf("Expected %t, got %t", test.allowed, got) + } + }) + } +} + func TestPolicyUnmarshalJSONAndValidate(t *testing.T) { case1Data := []byte(`{ "ID": "MyPolicyForMyBucket1", diff --git a/pkg/iam/policy/statement.go b/pkg/iam/policy/statement.go index 74f8874f3..024b197ed 100644 --- a/pkg/iam/policy/statement.go +++ b/pkg/iam/policy/statement.go @@ -114,8 +114,13 @@ func (statement Statement) isValid() error { return Errorf("unsupported Resource found %v for action %v", statement.Resources, action) } + condKeys, ok := iamActionConditionKeyMap.Lookup(action) + if !ok { + return Errorf("conditions are not supported for action %v", action) + } + keys := statement.Conditions.Keys() - keyDiff := keys.Difference(actionConditionKeyMap[action]) + keyDiff := keys.Difference(condKeys) if !keyDiff.IsEmpty() { return Errorf("unsupported condition keys '%v' used for action '%v'", keyDiff, action) }