[#282] policy: Use prefixes to distinguish s3/iam actions/resources
All checks were successful
/ DCO (pull_request) Successful in 1m37s
/ Vulncheck (pull_request) Successful in 1m50s
/ Builds (1.20) (pull_request) Successful in 2m24s
/ Builds (1.21) (pull_request) Successful in 2m2s
/ Lint (pull_request) Successful in 4m26s
/ Tests (1.20) (pull_request) Successful in 2m28s
/ Tests (1.21) (pull_request) Successful in 1m58s
All checks were successful
/ DCO (pull_request) Successful in 1m37s
/ Vulncheck (pull_request) Successful in 1m50s
/ Builds (1.20) (pull_request) Successful in 2m24s
/ Builds (1.21) (pull_request) Successful in 2m2s
/ Lint (pull_request) Successful in 4m26s
/ Tests (1.20) (pull_request) Successful in 2m28s
/ Tests (1.21) (pull_request) Successful in 1m58s
Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
This commit is contained in:
parent
38c5503a02
commit
a17ff66975
6 changed files with 93 additions and 29 deletions
|
@ -574,7 +574,7 @@ func (h *handler) PutBucketPolicyHandler(w http.ResponseWriter, r *http.Request)
|
|||
|
||||
for _, rule := range s3Chain.Rules {
|
||||
for _, resource := range rule.Resources.Names {
|
||||
if reqInfo.BucketName != strings.Split(resource, "/")[0] {
|
||||
if reqInfo.BucketName != strings.Split(strings.TrimPrefix(resource, arnAwsPrefix), "/")[0] {
|
||||
h.logAndSendError(w, "policy resource mismatched bucket", reqInfo, errors.GetAPIError(errors.ErrMalformedPolicy))
|
||||
return
|
||||
}
|
||||
|
|
|
@ -91,30 +91,41 @@ const (
|
|||
)
|
||||
|
||||
func determineOperationAndResource(r *http.Request, domains []string) (operation string, resource string) {
|
||||
reqType := noneType
|
||||
var (
|
||||
reqType ReqType
|
||||
matchDomain bool
|
||||
)
|
||||
|
||||
var matchDomain bool
|
||||
for _, domain := range domains {
|
||||
if ind := strings.Index(r.Host, "."+domain); ind != -1 {
|
||||
ind := strings.Index(r.Host, "."+domain)
|
||||
if ind == -1 {
|
||||
continue
|
||||
}
|
||||
|
||||
matchDomain = true
|
||||
reqType = bucketType
|
||||
resource = r.Host[:ind]
|
||||
trimmedObj := strings.TrimPrefix(r.URL.Path, "/")
|
||||
if trimmedObj != "" {
|
||||
bkt := r.Host[:ind]
|
||||
if obj := strings.TrimPrefix(r.URL.Path, "/"); obj != "" {
|
||||
reqType = objectType
|
||||
resource += "/" + trimmedObj
|
||||
}
|
||||
resource = fmt.Sprintf(s3.ResourceFormatS3BucketObject, bkt, obj)
|
||||
} else {
|
||||
resource = fmt.Sprintf(s3.ResourceFormatS3Bucket, bkt)
|
||||
}
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
if !matchDomain {
|
||||
resource = strings.TrimPrefix(r.URL.Path, "/")
|
||||
if resource != "" {
|
||||
if arr := strings.Split(resource, "/"); len(arr) == 1 {
|
||||
bktObj := strings.TrimPrefix(r.URL.Path, "/")
|
||||
if ind := strings.IndexByte(bktObj, '/'); ind == -1 {
|
||||
reqType = bucketType
|
||||
resource = fmt.Sprintf(s3.ResourceFormatS3Bucket, bktObj)
|
||||
if bktObj == "" {
|
||||
reqType = noneType
|
||||
}
|
||||
} else {
|
||||
reqType = objectType
|
||||
}
|
||||
resource = fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktObj[:ind], bktObj[ind+1:])
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -127,7 +138,7 @@ func determineOperationAndResource(r *http.Request, domains []string) (operation
|
|||
operation = determineGeneralOperation(r)
|
||||
}
|
||||
|
||||
return operation, resource
|
||||
return "s3:" + operation, resource
|
||||
}
|
||||
|
||||
func determineBucketOperation(r *http.Request) string {
|
||||
|
|
|
@ -302,9 +302,13 @@ func (h *handlerMock) CreateBucketHandler(http.ResponseWriter, *http.Request) {
|
|||
panic("implement me")
|
||||
}
|
||||
|
||||
func (h *handlerMock) HeadBucketHandler(http.ResponseWriter, *http.Request) {
|
||||
//TODO implement me
|
||||
panic("implement me")
|
||||
func (h *handlerMock) HeadBucketHandler(w http.ResponseWriter, r *http.Request) {
|
||||
res := &handlerResult{
|
||||
Method: middleware.HeadBucketOperation,
|
||||
ReqInfo: middleware.GetReqInfo(r.Context()),
|
||||
}
|
||||
|
||||
h.writeResponse(w, res)
|
||||
}
|
||||
|
||||
func (h *handlerMock) PostObject(http.ResponseWriter, *http.Request) {
|
||||
|
@ -337,9 +341,13 @@ func (h *handlerMock) DeleteBucketHandler(http.ResponseWriter, *http.Request) {
|
|||
panic("implement me")
|
||||
}
|
||||
|
||||
func (h *handlerMock) ListBucketsHandler(http.ResponseWriter, *http.Request) {
|
||||
//TODO implement me
|
||||
panic("implement me")
|
||||
func (h *handlerMock) ListBucketsHandler(w http.ResponseWriter, r *http.Request) {
|
||||
res := &handlerResult{
|
||||
Method: middleware.ListBucketsOperation,
|
||||
ReqInfo: middleware.GetReqInfo(r.Context()),
|
||||
}
|
||||
|
||||
h.writeResponse(w, res)
|
||||
}
|
||||
|
||||
func (h *handlerMock) Preflight(http.ResponseWriter, *http.Request) {
|
||||
|
|
|
@ -14,9 +14,11 @@ import (
|
|||
apiErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
|
||||
s3middleware "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics"
|
||||
engineiam "git.frostfs.info/TrueCloudLab/policy-engine/iam"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/schema/s3"
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
@ -49,6 +51,7 @@ func prepareRouter(t *testing.T) *routerMock {
|
|||
Metrics: &metrics.AppMetrics{},
|
||||
MiddlewareSettings: middlewareSettings,
|
||||
PolicyChecker: policyChecker,
|
||||
Domains: []string{"domain1", "domain2"},
|
||||
}
|
||||
return &routerMock{
|
||||
router: NewRouter(cfg),
|
||||
|
@ -163,7 +166,7 @@ func TestPolicyChecker(t *testing.T) {
|
|||
Rules: []chain.Rule{{
|
||||
Status: chain.AccessDenied,
|
||||
Actions: chain.Actions{Names: []string{"*"}},
|
||||
Resources: chain.Resources{Names: []string{bktName + "/*"}},
|
||||
Resources: chain.Resources{Names: []string{fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName)}},
|
||||
}},
|
||||
}
|
||||
|
||||
|
@ -190,6 +193,48 @@ func TestPolicyChecker(t *testing.T) {
|
|||
assertAPIError(t, w, apiErrors.ErrAccessDenied)
|
||||
}
|
||||
|
||||
func TestPolicyCheckerReqTypeDetermination(t *testing.T) {
|
||||
chiRouter := prepareRouter(t)
|
||||
bktName, objName := "bucket", "object"
|
||||
|
||||
policy := engineiam.Policy{
|
||||
Statement: []engineiam.Statement{{
|
||||
Principal: map[engineiam.PrincipalType][]string{engineiam.Wildcard: {}},
|
||||
Effect: engineiam.AllowEffect,
|
||||
Action: engineiam.Action{"s3:*"},
|
||||
Resource: engineiam.Resource{fmt.Sprintf(s3.ResourceFormatS3All)},
|
||||
}},
|
||||
}
|
||||
|
||||
ruleChain, err := engineiam.ConvertToS3Chain(policy, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, _, err = chiRouter.policyChecker.MorphRuleChainStorage().AddMorphRuleChain(chain.S3, engine.NamespaceTarget(""), ruleChain)
|
||||
require.NoError(t, err)
|
||||
|
||||
chiRouter.middlewareSettings.denyByDefault = true
|
||||
t.Run("can list buckets", func(t *testing.T) {
|
||||
w, r := httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
chiRouter.ServeHTTP(w, r)
|
||||
resp := readResponse(t, w)
|
||||
require.Equal(t, s3middleware.ListBucketsOperation, resp.Method)
|
||||
})
|
||||
|
||||
t.Run("can head 'bucket'", func(t *testing.T) {
|
||||
w, r := httptest.NewRecorder(), httptest.NewRequest(http.MethodHead, "/"+bktName, nil)
|
||||
chiRouter.ServeHTTP(w, r)
|
||||
resp := readResponse(t, w)
|
||||
require.Equal(t, s3middleware.HeadBucketOperation, resp.Method)
|
||||
})
|
||||
|
||||
t.Run("can put object into 'bucket'", func(t *testing.T) {
|
||||
w, r := httptest.NewRecorder(), httptest.NewRequest(http.MethodPut, fmt.Sprintf("/%s/%s", bktName, objName), nil)
|
||||
chiRouter.ServeHTTP(w, r)
|
||||
resp := readResponse(t, w)
|
||||
require.Equal(t, s3middleware.PutObjectOperation, resp.Method)
|
||||
})
|
||||
}
|
||||
|
||||
func TestDefaultBehaviorPolicyChecker(t *testing.T) {
|
||||
chiRouter := prepareRouter(t)
|
||||
bktName, objName := "bucket", "object"
|
||||
|
|
2
go.mod
2
go.mod
|
@ -7,7 +7,7 @@ require (
|
|||
git.frostfs.info/TrueCloudLab/frostfs-contract v0.18.1-0.20231129062201-a1b61d394958
|
||||
git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20230531082742-c97d21411eb6
|
||||
git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231107114540-ab75edd70939
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231213132038-1d07331f5df5
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231220070831-3128352693fc
|
||||
git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20231018083019-2b6d84de9a3d
|
||||
github.com/aws/aws-sdk-go v1.44.6
|
||||
github.com/bluele/gcache v0.0.2
|
||||
|
|
4
go.sum
4
go.sum
|
@ -48,8 +48,8 @@ git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231107114540-ab75edd70939
|
|||
git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231107114540-ab75edd70939/go.mod h1:t1akKcUH7iBrFHX8rSXScYMP17k2kYQXMbZooiL5Juw=
|
||||
git.frostfs.info/TrueCloudLab/hrw v1.2.1 h1:ccBRK21rFvY5R1WotI6LNoPlizk7qSvdfD8lNIRudVc=
|
||||
git.frostfs.info/TrueCloudLab/hrw v1.2.1/go.mod h1:C1Ygde2n843yTZEQ0FP69jYiuaYV0kriLvP4zm8JuvM=
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231213132038-1d07331f5df5 h1:vNDlTalmXHL4jVbDfquBdXeoevglOOFImOM/yanH14A=
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231213132038-1d07331f5df5/go.mod h1:iJMX6qk9aIHIu3WVSd4puF5CHsNk5eOi++MaJJfNbXM=
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231220070831-3128352693fc h1:ZBZkWBbDmqSdMoq7igIg4EYMIgbyFaLGcpHcU3urDnI=
|
||||
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231220070831-3128352693fc/go.mod h1:v43imcuSmDwSNrePe4UTQh8jaE8FmsiKN3FcaEzmRzc=
|
||||
git.frostfs.info/TrueCloudLab/rfc6979 v0.4.0 h1:M2KR3iBj7WpY3hP10IevfIB9MURr4O9mwVfJ+SjT3HA=
|
||||
git.frostfs.info/TrueCloudLab/rfc6979 v0.4.0/go.mod h1:okpbKfVYf/BpejtfFTfhZqFP+sZ8rsHrP8Rr/jYPNRc=
|
||||
git.frostfs.info/TrueCloudLab/tzhash v1.8.0 h1:UFMnUIk0Zh17m8rjGHJMqku2hCgaXDqjqZzS4gsb4UA=
|
||||
|
|
Loading…
Reference in a new issue