From 473239bf36b3a71d19cae0515de3a86a8f3e4348 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 30 Nov 2023 11:25:05 +0300 Subject: [PATCH] [#257] Add policy checker Signed-off-by: Denis Kirillov --- CHANGELOG.md | 1 + api/middleware/constants.go | 105 +++++++++ api/middleware/metrics.go | 10 +- api/middleware/policy.go | 305 +++++++++++++++++++++++++++ api/router.go | 20 +- api/router_mock_test.go | 14 +- api/router_test.go | 96 +++++++-- cmd/s3-gw/app.go | 26 +-- cmd/s3-gw/app_settings.go | 2 +- go.mod | 2 +- go.sum | 4 +- internal/logs/logs.go | 1 + pkg/service/control/server/server.go | 38 ++-- 13 files changed, 563 insertions(+), 61 deletions(-) create mode 100644 api/middleware/constants.go create mode 100644 api/middleware/policy.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 638750a9..d5b7afad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ This document outlines major changes between releases. - Support per namespace placement policies configuration (see `namespaces.config` config param) (#266) - Support control api to manage policies. See `control` config section (#258) - Add `namespace` label to billing metrics (#271) +- Support policy-engine (#257) ### Changed - Update prometheus to v1.15.0 (#94) diff --git a/api/middleware/constants.go b/api/middleware/constants.go new file mode 100644 index 00000000..b13be8d9 --- /dev/null +++ b/api/middleware/constants.go @@ -0,0 +1,105 @@ +package middleware + +const ( + ListBucketsOperation = "ListBuckets" + + // bucket operations. + + OptionsOperation = "Options" + HeadBucketOperation = "HeadBucket" + ListMultipartUploadsOperation = "ListMultipartUploads" + GetBucketLocationOperation = "GetBucketLocation" + GetBucketPolicyOperation = "GetBucketPolicy" + GetBucketLifecycleOperation = "GetBucketLifecycle" + GetBucketEncryptionOperation = "GetBucketEncryption" + GetBucketCorsOperation = "GetBucketCors" + GetBucketACLOperation = "GetBucketACL" + GetBucketWebsiteOperation = "GetBucketWebsite" + GetBucketAccelerateOperation = "GetBucketAccelerate" + GetBucketRequestPaymentOperation = "GetBucketRequestPayment" + GetBucketLoggingOperation = "GetBucketLogging" + GetBucketReplicationOperation = "GetBucketReplication" + GetBucketTaggingOperation = "GetBucketTagging" + GetBucketObjectLockConfigOperation = "GetBucketObjectLockConfig" + GetBucketVersioningOperation = "GetBucketVersioning" + GetBucketNotificationOperation = "GetBucketNotification" + ListenBucketNotificationOperation = "ListenBucketNotification" + ListBucketObjectVersionsOperation = "ListBucketObjectVersions" + ListObjectsV2MOperation = "ListObjectsV2M" + ListObjectsV2Operation = "ListObjectsV2" + ListObjectsV1Operation = "ListObjectsV1" + PutBucketCorsOperation = "PutBucketCors" + PutBucketACLOperation = "PutBucketACL" + PutBucketLifecycleOperation = "PutBucketLifecycle" + PutBucketEncryptionOperation = "PutBucketEncryption" + PutBucketPolicyOperation = "PutBucketPolicy" + PutBucketObjectLockConfigOperation = "PutBucketObjectLockConfig" + PutBucketTaggingOperation = "PutBucketTagging" + PutBucketVersioningOperation = "PutBucketVersioning" + PutBucketNotificationOperation = "PutBucketNotification" + CreateBucketOperation = "CreateBucket" + DeleteMultipleObjectsOperation = "DeleteMultipleObjects" + PostObjectOperation = "PostObject" + DeleteBucketCorsOperation = "DeleteBucketCors" + DeleteBucketWebsiteOperation = "DeleteBucketWebsite" + DeleteBucketTaggingOperation = "DeleteBucketTagging" + DeleteBucketPolicyOperation = "DeleteBucketPolicy" + DeleteBucketLifecycleOperation = "DeleteBucketLifecycle" + DeleteBucketEncryptionOperation = "DeleteBucketEncryption" + DeleteBucketOperation = "DeleteBucket" + + // object operations. + + HeadObjectOperation = "HeadObject" + ListPartsOperation = "ListParts" + GetObjectACLOperation = "GetObjectACL" + GetObjectTaggingOperation = "GetObjectTagging" + GetObjectRetentionOperation = "GetObjectRetention" + GetObjectLegalHoldOperation = "GetObjectLegalHold" + GetObjectAttributesOperation = "GetObjectAttributes" + GetObjectOperation = "GetObject" + UploadPartCopyOperation = "UploadPartCopy" + UploadPartOperation = "UploadPart" + PutObjectACLOperation = "PutObjectACL" + PutObjectTaggingOperation = "PutObjectTagging" + CopyObjectOperation = "CopyObject" + PutObjectRetentionOperation = "PutObjectRetention" + PutObjectLegalHoldOperation = "PutObjectLegalHold" + PutObjectOperation = "PutObject" + CompleteMultipartUploadOperation = "CompleteMultipartUpload" + CreateMultipartUploadOperation = "CreateMultipartUpload" + SelectObjectContentOperation = "SelectObjectContent" + AbortMultipartUploadOperation = "AbortMultipartUpload" + DeleteObjectTaggingOperation = "DeleteObjectTagging" + DeleteObjectOperation = "DeleteObject" +) + +const ( + UploadsQuery = "uploads" + LocationQuery = "location" + PolicyQuery = "policy" + LifecycleQuery = "lifecycle" + EncryptionQuery = "encryption" + CorsQuery = "cors" + ACLQuery = "acl" + WebsiteQuery = "website" + AccelerateQuery = "accelerate" + RequestPaymentQuery = "requestPayment" + LoggingQuery = "logging" + ReplicationQuery = "replication" + TaggingQuery = "tagging" + ObjectLockQuery = "object-lock" + VersioningQuery = "versioning" + NotificationQuery = "notification" + EventsQuery = "events" + VersionsQuery = "versions" + ListTypeQuery = "list-type" + MetadataQuery = "metadata" + DeleteQuery = "delete" + UploadIDQuery = "uploadId" + RetentionQuery = "retention" + LegalQuery = "legal" + AttributesQuery = "attributes" + PartNumberQuery = "partNumber" + LegalHoldQuery = "legal-hold" +) diff --git a/api/middleware/metrics.go b/api/middleware/metrics.go index 74d990f7..cba21240 100644 --- a/api/middleware/metrics.go +++ b/api/middleware/metrics.go @@ -35,7 +35,7 @@ type ( startTime time.Time } - AliasResolver interface { + MetricsSettings interface { ResolveNamespaceAlias(namespace string) string } @@ -49,14 +49,14 @@ type ( const systemPath = "/system" // Metrics wraps http handler for api with basic statistics collection. -func Metrics(log *zap.Logger, resolveBucket BucketResolveFunc, appMetrics *metrics.AppMetrics, aliasResolver AliasResolver) Func { +func Metrics(log *zap.Logger, resolveBucket BucketResolveFunc, appMetrics *metrics.AppMetrics, settings MetricsSettings) Func { return func(h http.Handler) http.Handler { - return stats(h.ServeHTTP, resolveCID(log, resolveBucket), appMetrics, aliasResolver) + return stats(h.ServeHTTP, resolveCID(log, resolveBucket), appMetrics, settings) } } // Stats is a handler that update metrics. -func stats(f http.HandlerFunc, resolveCID cidResolveFunc, appMetrics *metrics.AppMetrics, aliasResolver AliasResolver) http.HandlerFunc { +func stats(f http.HandlerFunc, resolveCID cidResolveFunc, appMetrics *metrics.AppMetrics, settings MetricsSettings) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { reqInfo := GetReqInfo(r.Context()) @@ -82,7 +82,7 @@ func stats(f http.HandlerFunc, resolveCID cidResolveFunc, appMetrics *metrics.Ap user := resolveUser(r.Context()) cnrID := resolveCID(r.Context(), reqInfo) - appMetrics.Update(user, reqInfo.BucketName, cnrID, aliasResolver.ResolveNamespaceAlias(reqInfo.Namespace), + appMetrics.Update(user, reqInfo.BucketName, cnrID, settings.ResolveNamespaceAlias(reqInfo.Namespace), requestTypeFromAPI(reqInfo.API), in.countBytes, out.countBytes) code := statsWriter.statusCode diff --git a/api/middleware/policy.go b/api/middleware/policy.go new file mode 100644 index 00000000..f47613e4 --- /dev/null +++ b/api/middleware/policy.go @@ -0,0 +1,305 @@ +package middleware + +import ( + "crypto/elliptic" + "fmt" + "net/http" + "strings" + + apiErr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource/testutil" + "git.frostfs.info/TrueCloudLab/policy-engine/schema/s3" + "github.com/nspcc-dev/neo-go/pkg/crypto/keys" + "go.uber.org/zap" +) + +type PolicySettings interface { + ResolveNamespaceAlias(ns string) string +} + +func PolicyCheck(storage engine.ChainRouter, settings PolicySettings, domains []string, log *zap.Logger) Func { + return func(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + st, err := policyCheck(storage, settings, domains, r) + if err == nil { + if st != chain.Allow && st != chain.NoRuleFound { // todo drop 'st != chain.NoRuleFound' + err = apiErr.GetAPIErrorWithError(apiErr.ErrAccessDenied, fmt.Errorf("policy check: %s", st.String())) + } + } + if err != nil { + reqLogOrDefault(ctx, log).Error(logs.PolicyValidationFailed, zap.Error(err)) + WriteErrorResponse(w, GetReqInfo(ctx), err) + return + } + + h.ServeHTTP(w, r) + }) + } +} + +func policyCheck(storage engine.ChainRouter, settings PolicySettings, domains []string, r *http.Request) (chain.Status, error) { + req, err := getPolicyRequest(r, domains) + if err != nil { + return 0, err + } + + reqInfo := GetReqInfo(r.Context()) + target := engine.NewRequestTargetWithNamespace(settings.ResolveNamespaceAlias(reqInfo.Namespace)) + st, found, err := storage.IsAllowed(chain.Ingress, target, req) + if err != nil { + return 0, err + } + + if !found { + st = chain.NoRuleFound + } + + return st, nil +} + +func getPolicyRequest(r *http.Request, domains []string) (*testutil.Request, error) { + var owner string + ctx := r.Context() + bd, err := GetBoxData(ctx) + if err == nil && bd.Gate.BearerToken != nil { + pk, err := keys.NewPublicKeyFromBytes(bd.Gate.BearerToken.SigningKeyBytes(), elliptic.P256()) + if err != nil { + return nil, fmt.Errorf("parse pubclic key from btoken: %w", err) + } + owner = pk.Address() + } + + op, res := determineOperationAndResource(r, domains) + + return testutil.NewRequest(op, testutil.NewResource(res, nil), + map[string]string{s3.PropertyKeyOwner: owner}, + ), nil +} + +type ReqType int + +const ( + noneType ReqType = iota + bucketType + objectType +) + +func determineOperationAndResource(r *http.Request, domains []string) (operation string, resource string) { + reqType := noneType + + var matchDomain bool + for _, domain := range domains { + if ind := strings.Index(r.Host, "."+domain); ind != -1 { + matchDomain = true + reqType = bucketType + resource = r.Host[:ind] + trimmedObj := strings.TrimPrefix(r.URL.Path, "/") + if trimmedObj != "" { + reqType = objectType + resource += "/" + trimmedObj + } + } + } + + if !matchDomain { + resource = strings.TrimPrefix(r.URL.Path, "/") + if resource != "" { + if arr := strings.Split(resource, "/"); len(arr) == 1 { + reqType = bucketType + } else { + reqType = objectType + } + } + } + + switch reqType { + case objectType: + operation = determineObjectOperation(r) + case bucketType: + operation = determineBucketOperation(r) + default: + operation = determineGeneralOperation(r) + } + + return operation, resource +} + +func determineBucketOperation(r *http.Request) string { + query := r.URL.Query() + switch r.Method { + case http.MethodOptions: + return OptionsOperation + case http.MethodHead: + return HeadBucketOperation + case http.MethodGet: + switch { + case query.Has(UploadsQuery): + return ListMultipartUploadsOperation + case query.Has(LocationQuery): + return GetBucketLocationOperation + case query.Has(PolicyQuery): + return GetBucketPolicyOperation + case query.Has(LifecycleQuery): + return GetBucketLifecycleOperation + case query.Has(EncryptionQuery): + return GetBucketEncryptionOperation + case query.Has(CorsQuery): + return GetBucketCorsOperation + case query.Has(ACLQuery): + return GetBucketACLOperation + case query.Has(WebsiteQuery): + return GetBucketWebsiteOperation + case query.Has(AccelerateQuery): + return GetBucketAccelerateOperation + case query.Has(RequestPaymentQuery): + return GetBucketRequestPaymentOperation + case query.Has(LoggingQuery): + return GetBucketLoggingOperation + case query.Has(ReplicationQuery): + return GetBucketReplicationOperation + case query.Has(TaggingQuery): + return GetBucketTaggingOperation + case query.Has(ObjectLockQuery): + return GetBucketObjectLockConfigOperation + case query.Has(VersioningQuery): + return GetBucketVersioningOperation + case query.Has(NotificationQuery): + return GetBucketNotificationOperation + case query.Has(EventsQuery): + return ListenBucketNotificationOperation + case query.Has(VersionsQuery): + return ListBucketObjectVersionsOperation + case query.Get(ListTypeQuery) == "2" && query.Get(MetadataQuery) == "true": + return ListObjectsV2MOperation + case query.Get(ListTypeQuery) == "2": + return ListObjectsV2Operation + default: + return ListObjectsV1Operation + } + case http.MethodPut: + switch { + case query.Has(CorsQuery): + return PutBucketCorsOperation + case query.Has(ACLQuery): + return PutBucketACLOperation + case query.Has(LifecycleQuery): + return PutBucketLifecycleOperation + case query.Has(EncryptionQuery): + return PutBucketEncryptionOperation + case query.Has(PolicyQuery): + return PutBucketPolicyOperation + case query.Has(ObjectLockQuery): + return PutBucketObjectLockConfigOperation + case query.Has(TaggingQuery): + return PutBucketTaggingOperation + case query.Has(VersioningQuery): + return PutBucketVersioningOperation + case query.Has(NotificationQuery): + return PutBucketNotificationOperation + default: + return CreateBucketOperation + } + case http.MethodPost: + switch { + case query.Has(DeleteQuery): + return DeleteMultipleObjectsOperation + default: + return PostObjectOperation + } + case http.MethodDelete: + switch { + case query.Has(CorsQuery): + return DeleteBucketCorsOperation + case query.Has(WebsiteQuery): + return DeleteBucketWebsiteOperation + case query.Has(TaggingQuery): + return DeleteBucketTaggingOperation + case query.Has(PolicyQuery): + return DeleteBucketPolicyOperation + case query.Has(LifecycleQuery): + return DeleteBucketLifecycleOperation + case query.Has(EncryptionQuery): + return DeleteBucketEncryptionOperation + default: + return DeleteBucketOperation + } + } + + return "" +} + +func determineObjectOperation(r *http.Request) string { + query := r.URL.Query() + switch r.Method { + case http.MethodHead: + return HeadObjectOperation + case http.MethodGet: + switch { + case query.Has(UploadIDQuery): + return ListPartsOperation + case query.Has(ACLQuery): + return GetObjectACLOperation + case query.Has(TaggingQuery): + return GetObjectTaggingOperation + case query.Has(RetentionQuery): + return GetObjectRetentionOperation + case query.Has(LegalQuery): + return GetObjectLegalHoldOperation + case query.Has(AttributesQuery): + return GetObjectAttributesOperation + default: + return GetObjectOperation + } + case http.MethodPut: + switch { + case query.Has(PartNumberQuery) && query.Has(UploadIDQuery) && r.Header.Get("X-Amz-Copy-Source") != "": + return UploadPartCopyOperation + case query.Has(PartNumberQuery) && query.Has(UploadIDQuery): + return UploadPartOperation + case query.Has(ACLQuery): + return PutObjectACLOperation + case query.Has(TaggingQuery): + return PutObjectTaggingOperation + case r.Header.Get("X-Amz-Copy-Source") != "": + return CopyObjectOperation + case query.Has(RetentionQuery): + return PutObjectRetentionOperation + case query.Has(LegalHoldQuery): + return PutObjectLegalHoldOperation + default: + return PutObjectOperation + } + case http.MethodPost: + switch { + case query.Has(UploadIDQuery): + return CompleteMultipartUploadOperation + case query.Has(UploadsQuery): + return CreateMultipartUploadOperation + default: + return SelectObjectContentOperation + } + case http.MethodDelete: + switch { + case query.Has(UploadIDQuery): + return AbortMultipartUploadOperation + case query.Has(TaggingQuery): + return DeleteObjectTaggingOperation + default: + return DeleteObjectOperation + } + } + + return "" +} + +func determineGeneralOperation(r *http.Request) string { + if r.Method == http.MethodGet { + return ListBucketsOperation + } + return "" +} diff --git a/api/router.go b/api/router.go index 22070192..f6c76bfc 100644 --- a/api/router.go +++ b/api/router.go @@ -89,6 +89,12 @@ type ( } ) +type Settings interface { + s3middleware.RequestSettings + s3middleware.PolicySettings + s3middleware.MetricsSettings +} + type Config struct { Throttle middleware.ThrottleOpts Handler Handler @@ -96,25 +102,25 @@ type Config struct { Log *zap.Logger Metrics *metrics.AppMetrics - RequestMiddlewareSettings s3middleware.RequestSettings - - AliasResolver s3middleware.AliasResolver + MiddlewareSettings Settings // Domains optional. If empty no virtual hosted domains will be attached. Domains []string // FrostfsID optional. If nil middleware.FrostfsIDValidation won't be attached. FrostfsID s3middleware.FrostFSID + + PolicyStorage engine.LocalOverrideEngine } func NewRouter(cfg Config) *chi.Mux { api := chi.NewRouter() api.Use( - s3middleware.Request(cfg.Log, cfg.RequestMiddlewareSettings), + s3middleware.Request(cfg.Log, cfg.MiddlewareSettings), middleware.ThrottleWithOpts(cfg.Throttle), middleware.Recoverer, s3middleware.Tracing(), - s3middleware.Metrics(cfg.Log, cfg.Handler.ResolveBucket, cfg.Metrics, cfg.AliasResolver), + s3middleware.Metrics(cfg.Log, cfg.Handler.ResolveBucket, cfg.Metrics, cfg.MiddlewareSettings), s3middleware.LogSuccessResponse(cfg.Log), s3middleware.Auth(cfg.Center, cfg.Log), ) @@ -123,6 +129,10 @@ func NewRouter(cfg Config) *chi.Mux { api.Use(s3middleware.FrostfsIDValidation(cfg.FrostfsID, cfg.Log)) } + if cfg.PolicyStorage != nil { + api.Use(s3middleware.PolicyCheck(cfg.PolicyStorage, cfg.MiddlewareSettings, cfg.Domains, cfg.Log)) + } + defaultRouter := chi.NewRouter() defaultRouter.Mount(fmt.Sprintf("/{%s}", s3middleware.BucketURLPrm), bucketRouter(cfg.Handler, cfg.Log)) defaultRouter.Get("/", named("ListBuckets", cfg.Handler.ListBucketsHandler)) diff --git a/api/router_mock_test.go b/api/router_mock_test.go index 99c5d000..870c4679 100644 --- a/api/router_mock_test.go +++ b/api/router_mock_test.go @@ -11,6 +11,8 @@ import ( "github.com/stretchr/testify/require" ) +const FrostfsNamespaceHeader = "X-Frostfs-Namespace" + type centerMock struct { } @@ -18,10 +20,14 @@ func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) { return &middleware.Box{}, nil } -type requestSettingsMock struct{} +type middlewareSettingsMock struct{} -func (r *requestSettingsMock) NamespaceHeader() string { - return "X-Frostfs-Namespace" +func (r *middlewareSettingsMock) NamespaceHeader() string { + return FrostfsNamespaceHeader +} + +func (r *middlewareSettingsMock) ResolveNamespaceAlias(ns string) string { + return ns } type handlerMock struct { @@ -105,7 +111,7 @@ func (h *handlerMock) PutObjectLegalHoldHandler(http.ResponseWriter, *http.Reque func (h *handlerMock) PutObjectHandler(w http.ResponseWriter, r *http.Request) { res := &handlerResult{ - Method: "PutObject", + Method: middleware.PutObjectOperation, ReqInfo: middleware.GetReqInfo(r.Context()), } diff --git a/api/router_test.go b/api/router_test.go index 032333e7..a331e592 100644 --- a/api/router_test.go +++ b/api/router_test.go @@ -2,6 +2,7 @@ package api import ( "encoding/json" + "encoding/xml" "fmt" "io" "net/http" @@ -10,13 +11,46 @@ import ( "testing" "time" + 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" + "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" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) +type routerMock struct { + router *chi.Mux + cfg Config +} + +func (m *routerMock) ServeHTTP(w http.ResponseWriter, r *http.Request) { + m.router.ServeHTTP(w, r) +} + +func prepareRouter(t *testing.T) *routerMock { + cfg := Config{ + Throttle: middleware.ThrottleOpts{ + Limit: 10, + BacklogTimeout: 30 * time.Second, + }, + Handler: &handlerMock{t: t}, + Center: ¢erMock{}, + Log: zaptest.NewLogger(t), + Metrics: &metrics.AppMetrics{}, + MiddlewareSettings: &middlewareSettingsMock{}, + PolicyStorage: inmemory.NewInMemoryLocalOverrides(), + } + return &routerMock{ + router: NewRouter(cfg), + cfg: cfg, + } +} + func TestRouterUploadPart(t *testing.T) { chiRouter := prepareRouter(t) @@ -111,19 +145,42 @@ func TestRouterObjectEscaping(t *testing.T) { } } -func prepareRouter(t *testing.T) *chi.Mux { - cfg := Config{ - Throttle: middleware.ThrottleOpts{ - Limit: 10, - BacklogTimeout: 30 * time.Second, - }, - Handler: &handlerMock{t: t}, - Center: ¢erMock{}, - Log: zaptest.NewLogger(t), - Metrics: &metrics.AppMetrics{}, - RequestMiddlewareSettings: &requestSettingsMock{}, +func TestPolicyChecker(t *testing.T) { + chiRouter := prepareRouter(t) + namespace := "custom-ns" + bktName, objName := "bucket", "object" + target := fmt.Sprintf("/%s/%s", bktName, objName) + + ruleChain := &chain.Chain{ + ID: "id", + Rules: []chain.Rule{{ + Status: chain.AccessDenied, + Actions: chain.Actions{Names: []string{"*"}}, + Resources: chain.Resources{Names: []string{bktName + "/*"}}, + }}, } - return NewRouter(cfg) + + err := chiRouter.cfg.PolicyStorage.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.NamespaceTarget(namespace), ruleChain) + require.NoError(t, err) + + // check we can access 'bucket' in default namespace + w, r := httptest.NewRecorder(), httptest.NewRequest(http.MethodPut, target, nil) + chiRouter.ServeHTTP(w, r) + resp := readResponse(t, w) + require.Equal(t, s3middleware.PutObjectOperation, resp.Method) + + // check we can access 'other-bucket' in custom namespace + w, r = httptest.NewRecorder(), httptest.NewRequest(http.MethodPut, "/other-bucket/object", nil) + r.Header.Set(FrostfsNamespaceHeader, namespace) + chiRouter.ServeHTTP(w, r) + resp = readResponse(t, w) + require.Equal(t, s3middleware.PutObjectOperation, resp.Method) + + // check we cannot access 'bucket' in custom namespace + w, r = httptest.NewRecorder(), httptest.NewRequest(http.MethodPut, target, nil) + r.Header.Set(FrostfsNamespaceHeader, namespace) + chiRouter.ServeHTTP(w, r) + assertAPIError(t, w, apiErrors.ErrAccessDenied) } func readResponse(t *testing.T, w *httptest.ResponseRecorder) handlerResult { @@ -136,3 +193,18 @@ func readResponse(t *testing.T, w *httptest.ResponseRecorder) handlerResult { require.NoErrorf(t, err, "actual body: '%s'", string(resData)) return res } + +func assertAPIError(t *testing.T, w *httptest.ResponseRecorder, expectedErrorCode apiErrors.ErrorCode) { + actualErrorResponse := &s3middleware.ErrorResponse{} + err := xml.NewDecoder(w.Result().Body).Decode(actualErrorResponse) + require.NoError(t, err) + + expectedError := apiErrors.GetAPIError(expectedErrorCode) + + require.Equal(t, expectedError.HTTPStatusCode, w.Code) + require.Equal(t, expectedError.Code, actualErrorResponse.Code) + + if expectedError.ErrCode != apiErrors.ErrInternalError { + require.Contains(t, actualErrorResponse.Message, expectedError.Description) + } +} diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index 7bdf88b4..e739644c 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -348,16 +348,20 @@ func (s *appSettings) setNamespaceHeader(nsHeader string) { } func (s *appSettings) FormContainerZone(ns string) (zone string, isDefault bool) { - s.mu.RLock() - namespaces := s.defaultNamespaces - s.mu.RUnlock() - if slices.Contains(namespaces, ns) { + if s.IsDefaultNamespace(ns) { return v2container.SysAttributeZoneDefault, true } return ns + ".ns", false } +func (s *appSettings) IsDefaultNamespace(ns string) bool { + s.mu.RLock() + namespaces := s.defaultNamespaces + s.mu.RUnlock() + return slices.Contains(namespaces, ns) +} + func (s *appSettings) setDefaultNamespaces(namespaces []string) { for i := range namespaces { // to be set namespaces in env variable as `S3_GW_KLUDGE_DEFAULT_NAMESPACES="" "root"` namespaces[i] = strings.Trim(namespaces[i], "\"") @@ -386,12 +390,10 @@ func (s *appSettings) setAuthorizedControlAPIKeys(keys keys.PublicKeys) { } func (s *appSettings) ResolveNamespaceAlias(namespace string) string { - s.mu.RLock() - namespaces := s.defaultNamespaces - s.mu.RUnlock() - if slices.Contains(namespaces, namespace) { - return defaultMetricsNamespace + if s.IsDefaultNamespace(namespace) { + return defaultNamespace } + return namespace } @@ -404,7 +406,7 @@ func (a *App) initControlAPI() { a.policyStorage = inmemory.NewInMemoryLocalOverrides() svc := controlSvc.New( - controlSvc.WithAuthorizedKeysFetcher(a.settings), + controlSvc.WithSettings(a.settings), controlSvc.WithLogger(a.log), controlSvc.WithChainStorage(a.policyStorage), ) @@ -629,8 +631,8 @@ func (a *App) Serve(ctx context.Context) { Metrics: a.metrics, Domains: domains, - RequestMiddlewareSettings: a.settings, - AliasResolver: a.settings, + MiddlewareSettings: a.settings, + PolicyStorage: a.policyStorage, } // We cannot make direct assignment if frostfsid.FrostFSID is nil diff --git a/cmd/s3-gw/app_settings.go b/cmd/s3-gw/app_settings.go index dc482049..940e0b25 100644 --- a/cmd/s3-gw/app_settings.go +++ b/cmd/s3-gw/app_settings.go @@ -56,7 +56,7 @@ const ( defaultConstraintName = "default" - defaultMetricsNamespace = "" + defaultNamespace = "" ) var ( diff --git a/go.mod b/go.mod index 7d9c045d..4cdf8b89 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( git.frostfs.info/TrueCloudLab/frostfs-contract v0.18.1-0.20231109143925-dd5919348da9 git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20230531082742-c97d21411eb6 git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231003164722-60463871dbc2 - git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231121084541-5fa9d91903ba + git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231205092054-2d4a9fc6dcb3 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 diff --git a/go.sum b/go.sum index f1d3d1cb..691f8b57 100644 --- a/go.sum +++ b/go.sum @@ -48,8 +48,8 @@ git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231003164722-60463871dbc2 git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231003164722-60463871dbc2/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-20231121084541-5fa9d91903ba h1:VL3Nyz+C9Cwc+h3xAFUQBS62gneyGTULGTh+8NPP21g= -git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231121084541-5fa9d91903ba/go.mod h1:ekrDiIySdYhji5rBNAkxYMztFWMXyC9Q8LVz6gGVDu0= +git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231205092054-2d4a9fc6dcb3 h1:d4cCtg6vgQ101Qni9FqYaGPkmSJP1ZnEyHYMI+JaTIo= +git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231205092054-2d4a9fc6dcb3/go.mod h1:ekrDiIySdYhji5rBNAkxYMztFWMXyC9Q8LVz6gGVDu0= 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= diff --git a/internal/logs/logs.go b/internal/logs/logs.go index 9cf89e00..553211e9 100644 --- a/internal/logs/logs.go +++ b/internal/logs/logs.go @@ -133,4 +133,5 @@ const ( ControlAPIRemovePolicies = "remove policies request" ControlAPIGetPolicy = "get policy request" ControlAPIListPolicies = "list policies request" + PolicyValidationFailed = "policy validation failed" ) diff --git a/pkg/service/control/server/server.go b/pkg/service/control/server/server.go index c44bebca..87198730 100644 --- a/pkg/service/control/server/server.go +++ b/pkg/service/control/server/server.go @@ -25,13 +25,15 @@ type Server struct { *cfg } -type AuthorizedKeysFetcher interface { +type Settings interface { + ResolveNamespaceAlias(ns string) string FetchRawKeys() [][]byte } -type emptyKeysFetcher struct{} +type defaultSettings struct{} -func (f emptyKeysFetcher) FetchRawKeys() [][]byte { return nil } +func (f defaultSettings) FetchRawKeys() [][]byte { return nil } +func (f defaultSettings) ResolveNamespaceAlias(ns string) string { return ns } // Option of the Server's constructor. type Option func(*cfg) @@ -39,7 +41,7 @@ type Option func(*cfg) type cfg struct { log *zap.Logger - keysFetcher AuthorizedKeysFetcher + settings Settings chainStorage engine.LocalOverrideEngine } @@ -47,7 +49,7 @@ type cfg struct { func defaultCfg() *cfg { return &cfg{ log: zap.NewNop(), - keysFetcher: emptyKeysFetcher{}, + settings: defaultSettings{}, chainStorage: inmemory.NewInMemoryLocalOverrides(), } } @@ -67,11 +69,10 @@ func New(opts ...Option) *Server { } } -// WithAuthorizedKeysFetcher returns option to add list of public -// keys that have rights to use Control service. -func WithAuthorizedKeysFetcher(fetcher AuthorizedKeysFetcher) Option { +// WithSettings returns option to add settings to use Control service. +func WithSettings(settings Settings) Option { return func(c *cfg) { - c.keysFetcher = fetcher + c.settings = settings } } @@ -139,12 +140,8 @@ func (s *Server) putPolicy(data *control.PutPoliciesRequest_ChainData) error { return status.Error(codes.InvalidArgument, "missing chain id") } - err := s.chainStorage.LocalStorage().RemoveOverride(chain.Ingress, data.GetNamespace(), overrideChain.ID) - if err != nil && !isNotFoundError(err) { - return status.Error(codes.Internal, err.Error()) - } - - if _, err = s.chainStorage.LocalStorage().AddOverride(chain.Ingress, data.GetNamespace(), &overrideChain); err != nil { + ns := s.settings.ResolveNamespaceAlias(data.GetNamespace()) + if _, err := s.chainStorage.LocalStorage().AddOverride(chain.Ingress, engine.NamespaceTarget(ns), &overrideChain); err != nil { return status.Error(codes.Internal, err.Error()) } @@ -172,7 +169,8 @@ func (s *Server) RemovePolicies(_ context.Context, req *control.RemovePoliciesRe } func (s *Server) removePolicy(info *control.RemovePoliciesRequest_ChainInfo) error { - err := s.chainStorage.LocalStorage().RemoveOverride(chain.Ingress, info.GetNamespace(), chain.ID(info.GetChainID())) + ns := s.settings.ResolveNamespaceAlias(info.GetNamespace()) + err := s.chainStorage.LocalStorage().RemoveOverride(chain.Ingress, engine.NamespaceTarget(ns), chain.ID(info.GetChainID())) if err != nil { if isNotFoundError(err) { return status.Error(codes.NotFound, err.Error()) @@ -194,7 +192,8 @@ func (s *Server) GetPolicy(_ context.Context, req *control.GetPolicyRequest) (*c return nil, status.Error(codes.PermissionDenied, err.Error()) } - overrideChain, err := s.chainStorage.LocalStorage().GetOverride(chain.Ingress, req.GetBody().GetNamespace(), chain.ID(req.GetBody().GetChainID())) + ns := s.settings.ResolveNamespaceAlias(req.GetBody().GetNamespace()) + overrideChain, err := s.chainStorage.LocalStorage().GetOverride(chain.Ingress, engine.NamespaceTarget(ns), chain.ID(req.GetBody().GetChainID())) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -214,7 +213,8 @@ func (s *Server) ListPolicies(_ context.Context, req *control.ListPoliciesReques return nil, status.Error(codes.PermissionDenied, err.Error()) } - chains, err := s.chainStorage.LocalStorage().ListOverrides(chain.Ingress, req.GetBody().GetNamespace()) + ns := s.settings.ResolveNamespaceAlias(req.GetBody().GetNamespace()) + chains, err := s.chainStorage.LocalStorage().ListOverrides(chain.Ingress, engine.NamespaceTarget(ns)) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -250,7 +250,7 @@ func (s *Server) isValidRequest(req SignedMessage) error { ) // check if key is allowed - for _, authKey := range s.keysFetcher.FetchRawKeys() { + for _, authKey := range s.settings.FetchRawKeys() { if allowed = bytes.Equal(authKey, key); allowed { break }