From 80fc3b4ac3fdbd6c2884373d563a6bd4b4287106 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Wed, 9 Nov 2022 17:25:02 +0300 Subject: [PATCH] [#740] Fix forming policy by ast Signed-off-by: Denis Kirillov --- api/handler/acl.go | 9 +++--- api/handler/acl_test.go | 72 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/api/handler/acl.go b/api/handler/acl.go index 0c644e3f0..8e74cd201 100644 --- a/api/handler/acl.go +++ b/api/handler/acl.go @@ -1001,6 +1001,10 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) { trimmedResource := strings.TrimPrefix(resource, arnAwsPrefix) r, ok := rr[trimmedResource] if !ok { + if !strings.HasPrefix(trimmedResource, bktPolicy.Bucket) { + return nil, fmt.Errorf("resource '%s' must be in the same bucket '%s'", trimmedResource, bktPolicy.Bucket) + } + r = &astResource{ resourceInfo: resourceInfoFromName(trimmedResource, bktPolicy.Bucket), } @@ -1046,9 +1050,6 @@ func astToPolicy(ast *ast) *bucketPolicy { bktPolicy := &bucketPolicy{} for _, resource := range ast.Resources { - if len(resource.Version) == 0 { - continue - } allowed, denied := triageOperations(resource.Operations) handleResourceOperations(bktPolicy, allowed, eacl.ActionAllow, resource.Name()) handleResourceOperations(bktPolicy, denied, eacl.ActionDeny, resource.Name()) @@ -1080,7 +1081,7 @@ func handleResourceOperations(bktPolicy *bucketPolicy, list []*astOperation, eac for action, ops := range actionToOpMap { for _, op := range ops { if !contains(userOps, op) { - break LOOP + continue LOOP } } actions = append(actions, action) diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go index 40f17ba12..42e2e4966 100644 --- a/api/handler/acl_test.go +++ b/api/handler/acl_test.go @@ -1,11 +1,13 @@ package handler import ( + "bytes" "context" "crypto/ecdsa" "crypto/rand" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" "io" "net/http" @@ -1297,7 +1299,7 @@ func TestPutBucketACL(t *testing.T) { tc := prepareHandlerContext(t) bktName := "bucket-for-acl" - box := createAccessBox(t) + box, _ := createAccessBox(t) bktInfo := createBucket(t, tc, bktName, box) header := map[string]string{api.AmzACL: "public-read"} @@ -1308,6 +1310,70 @@ func TestPutBucketACL(t *testing.T) { checkLastRecords(t, tc, bktInfo, eacl.ActionDeny) } +func TestBucketPolicy(t *testing.T) { + hc := prepareHandlerContext(t) + bktName := "bucket-for-policy" + + box, key := createAccessBox(t) + createBucket(t, hc, bktName, box) + + bktPolicy := getBucketPolicy(hc, bktName) + 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) + } else { + require.Equal(t, allUsersWildcard, st.Principal.AWS) + require.Equal(t, "Deny", st.Effect) + require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource) + } + } + + newPolicy := &bucketPolicy{ + Statement: []statement{{ + Effect: "Allow", + Principal: principal{AWS: allUsersWildcard}, + Action: []string{s3GetObject}, + Resource: []string{arnAwsPrefix + "dummy"}, + }}, + } + + putBucketPolicy(hc, bktName, newPolicy, box, http.StatusInternalServerError) + + newPolicy.Statement[0].Resource[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) + } + } +} + +func getBucketPolicy(hc *handlerContext, bktName string) *bucketPolicy { + w, r := prepareTestRequest(hc, bktName, "", nil) + hc.Handler().GetBucketPolicyHandler(w, r) + + assertStatus(hc.t, w, http.StatusOK) + policy := &bucketPolicy{} + err := json.NewDecoder(w.Result().Body).Decode(policy) + require.NoError(hc.t, err) + return policy +} + +func putBucketPolicy(hc *handlerContext, bktName string, bktPolicy *bucketPolicy, box *accessbox.Box, status int) { + body, err := json.Marshal(bktPolicy) + require.NoError(hc.t, err) + + w, r := prepareTestPayloadRequest(hc, bktName, "", bytes.NewReader(body)) + ctx := context.WithValue(r.Context(), api.BoxData, box) + r = r.WithContext(ctx) + hc.Handler().PutBucketPolicyHandler(w, r) + assertStatus(hc.t, w, status) +} + func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo, action eacl.Action) { bktACL, err := tc.Layer().GetBucketACL(tc.Context(), bktInfo) require.NoError(t, err) @@ -1325,7 +1391,7 @@ func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo } } -func createAccessBox(t *testing.T) *accessbox.Box { +func createAccessBox(t *testing.T) (*accessbox.Box, *keys.PrivateKey) { key, err := keys.NewPrivateKey() require.NoError(t, err) @@ -1345,7 +1411,7 @@ func createAccessBox(t *testing.T) *accessbox.Box { }, } - return box + return box, key } func createBucket(t *testing.T, tc *handlerContext, bktName string, box *accessbox.Box) *data.BucketInfo {