From 5698d5844e22b16dacc787b1fb563193d33d4e31 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Wed, 13 Dec 2023 17:44:18 +0300 Subject: [PATCH] [#283] Support frostfsid groups in policy request checking Signed-off-by: Denis Kirillov --- api/middleware/auth.go | 6 ++--- api/middleware/policy.go | 32 ++++++++++++++++++------ api/router.go | 16 +++++++----- cmd/s3-gw/app.go | 33 +++++++++++-------------- cmd/s3-gw/app_settings.go | 5 ++-- config/config.env | 4 +-- config/config.yaml | 7 +++--- docs/configuration.md | 12 ++++----- go.mod | 2 +- go.sum | 4 +-- internal/frostfs/frostfsid/frostfsid.go | 27 +++++++++++++++++--- 11 files changed, 92 insertions(+), 56 deletions(-) diff --git a/api/middleware/auth.go b/api/middleware/auth.go index 9684f09..b35ef12 100644 --- a/api/middleware/auth.go +++ b/api/middleware/auth.go @@ -71,11 +71,11 @@ func Auth(center Center, log *zap.Logger) Func { } } -type FrostFSID interface { +type FrostFSIDValidator interface { ValidatePublicKey(key *keys.PublicKey) error } -func FrostfsIDValidation(frostfsID FrostFSID, log *zap.Logger) Func { +func FrostfsIDValidation(frostfsID FrostFSIDValidator, log *zap.Logger) Func { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -97,7 +97,7 @@ func FrostfsIDValidation(frostfsID FrostFSID, log *zap.Logger) Func { } } -func validateBearerToken(frostfsID FrostFSID, bt *bearer.Token) error { +func validateBearerToken(frostfsID FrostFSIDValidator, bt *bearer.Token) error { m := new(acl.BearerToken) bt.WriteToV2(m) diff --git a/api/middleware/policy.go b/api/middleware/policy.go index be99a95..da69a0e 100644 --- a/api/middleware/policy.go +++ b/api/middleware/policy.go @@ -11,8 +11,10 @@ import ( "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/common" "git.frostfs.info/TrueCloudLab/policy-engine/schema/s3" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" + "github.com/nspcc-dev/neo-go/pkg/util" "go.uber.org/zap" ) @@ -21,12 +23,16 @@ type PolicySettings interface { PolicyDenyByDefault() bool } -func PolicyCheck(storage engine.ChainRouter, settings PolicySettings, domains []string, log *zap.Logger) Func { +type FrostFSIDInformer interface { + GetUserGroupIDs(userHash util.Uint160) ([]string, error) +} + +func PolicyCheck(storage engine.ChainRouter, frostfsid FrostFSIDInformer, 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) + st, err := policyCheck(storage, frostfsid, settings, domains, r) if err == nil { if st != chain.Allow && (st != chain.NoRuleFound || settings.PolicyDenyByDefault()) { err = apiErr.GetAPIErrorWithError(apiErr.ErrAccessDenied, fmt.Errorf("policy check: %s", st.String())) @@ -43,8 +49,8 @@ func PolicyCheck(storage engine.ChainRouter, settings PolicySettings, domains [] } } -func policyCheck(storage engine.ChainRouter, settings PolicySettings, domains []string, r *http.Request) (chain.Status, error) { - req, err := getPolicyRequest(r, domains) +func policyCheck(storage engine.ChainRouter, frostfsid FrostFSIDInformer, settings PolicySettings, domains []string, r *http.Request) (chain.Status, error) { + req, err := getPolicyRequest(r, frostfsid, domains) if err != nil { return 0, err } @@ -63,8 +69,12 @@ func policyCheck(storage engine.ChainRouter, settings PolicySettings, domains [] return st, nil } -func getPolicyRequest(r *http.Request, domains []string) (*testutil.Request, error) { - var owner string +func getPolicyRequest(r *http.Request, frostfsid FrostFSIDInformer, domains []string) (*testutil.Request, error) { + var ( + owner string + groups []string + ) + ctx := r.Context() bd, err := GetBoxData(ctx) if err == nil && bd.Gate.BearerToken != nil { @@ -73,12 +83,20 @@ func getPolicyRequest(r *http.Request, domains []string) (*testutil.Request, err return nil, fmt.Errorf("parse pubclic key from btoken: %w", err) } owner = pk.Address() + + groups, err = frostfsid.GetUserGroupIDs(pk.GetScriptHash()) + if err != nil { + return nil, fmt.Errorf("get group ids: %w", err) + } } op, res := determineOperationAndResource(r, domains) return testutil.NewRequest(op, testutil.NewResource(res, nil), - map[string]string{s3.PropertyKeyOwner: owner}, + map[string]string{ + s3.PropertyKeyOwner: owner, + common.PropertyKeyFrostFSIDGroupID: chain.FormCondSliceContainsValue(groups), + }, ), nil } diff --git a/api/router.go b/api/router.go index 315a45e..1e7cf80 100644 --- a/api/router.go +++ b/api/router.go @@ -96,6 +96,11 @@ type Settings interface { s3middleware.MetricsSettings } +type FrostFSID interface { + s3middleware.FrostFSIDValidator + s3middleware.FrostFSIDInformer +} + type Config struct { Throttle middleware.ThrottleOpts Handler Handler @@ -108,8 +113,9 @@ type Config struct { // 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 + FrostfsID FrostFSID + + FrostFSIDValidation bool PolicyChecker engine.ChainRouter } @@ -126,13 +132,11 @@ func NewRouter(cfg Config) *chi.Mux { s3middleware.Auth(cfg.Center, cfg.Log), ) - if cfg.FrostfsID != nil { + if cfg.FrostFSIDValidation { api.Use(s3middleware.FrostfsIDValidation(cfg.FrostfsID, cfg.Log)) } - if cfg.PolicyChecker != nil { - api.Use(s3middleware.PolicyCheck(cfg.PolicyChecker, cfg.MiddlewareSettings, cfg.Domains, cfg.Log)) - } + api.Use(s3middleware.PolicyCheck(cfg.PolicyChecker, cfg.FrostfsID, cfg.MiddlewareSettings, cfg.Domains, cfg.Log)) defaultRouter := chi.NewRouter() defaultRouter.Mount(fmt.Sprintf("/{%s}", s3middleware.BucketURLPrm), bucketRouter(cfg.Handler, cfg.Log)) diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index f68965a..08d7478 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -84,12 +84,13 @@ type ( } appSettings struct { - logLevel zap.AtomicLevel - maxClient maxClientsConfig - defaultMaxAge int - notificatorEnabled bool - resolveZoneList []string - isResolveListAllow bool // True if ResolveZoneList contains allowed zones + logLevel zap.AtomicLevel + maxClient maxClientsConfig + defaultMaxAge int + notificatorEnabled bool + resolveZoneList []string + isResolveListAllow bool // True if ResolveZoneList contains allowed zones + frostfsidValidation bool mu sync.RWMutex namespaces Namespaces @@ -192,10 +193,11 @@ func (a *App) initLayer(ctx context.Context) { func newAppSettings(log *Logger, v *viper.Viper, key *keys.PrivateKey) *appSettings { settings := &appSettings{ - logLevel: log.lvl, - maxClient: newMaxClients(v), - defaultMaxAge: fetchDefaultMaxAge(v, log.logger), - notificatorEnabled: v.GetBool(cfgEnableNATS), + logLevel: log.lvl, + maxClient: newMaxClients(v), + defaultMaxAge: fetchDefaultMaxAge(v, log.logger), + notificatorEnabled: v.GetBool(cfgEnableNATS), + frostfsidValidation: v.GetBool(cfgFrostfsIDValidationEnabled), } settings.resolveZoneList = v.GetStringSlice(cfgResolveBucketAllow) @@ -434,10 +436,6 @@ func (a *App) initMetrics() { } func (a *App) initFrostfsID(ctx context.Context) { - if !a.cfg.GetBool(cfgFrostfsIDEnabled) { - return - } - var err error a.frostfsid, err = frostfsid.New(ctx, frostfsid.Config{ RPCAddress: a.cfg.GetString(cfgRPCEndpoint), @@ -673,12 +671,9 @@ func (a *App) Serve(ctx context.Context) { MiddlewareSettings: a.settings, PolicyChecker: a.policyStorage, - } - // We cannot make direct assignment if frostfsid.FrostFSID is nil - // because in that case the interface won't be nil, it will just contain nil value. - if a.frostfsid != nil { - cfg.FrostfsID = a.frostfsid + FrostfsID: a.frostfsid, + FrostFSIDValidation: a.settings.frostfsidValidation, } chiRouter := api.NewRouter(cfg) diff --git a/cmd/s3-gw/app_settings.go b/cmd/s3-gw/app_settings.go index 86a091c..fcc43c2 100644 --- a/cmd/s3-gw/app_settings.go +++ b/cmd/s3-gw/app_settings.go @@ -206,8 +206,8 @@ const ( // Settings. cfgPolicyDenyByDefault = "features.policy.deny_by_default" // FrostfsID. - cfgFrostfsIDEnabled = "frostfsid.enabled" - cfgFrostfsIDContract = "frostfsid.contract" + cfgFrostfsIDContract = "frostfsid.contract" + cfgFrostfsIDValidationEnabled = "frostfsid.validation.enabled" // Policy. cfgPolicyEnabled = "policy.enabled" @@ -697,7 +697,6 @@ func newSettings() *viper.Viper { // frostfsid v.SetDefault(cfgFrostfsIDContract, "frostfsid.frostfs") - v.SetDefault(cfgFrostfsIDEnabled, true) // policy v.SetDefault(cfgPolicyContract, "policy.frostfs") diff --git a/config/config.env b/config/config.env index 1288142..d179025 100644 --- a/config/config.env +++ b/config/config.env @@ -193,10 +193,10 @@ S3_GW_WEB_WRITE_TIMEOUT=0 S3_GW_WEB_IDLE_TIMEOUT=30s # FrostfsID contract configuration. To enable this functionality the `rpc_endpoint` param must be also set. -# Enables check that allow requests only users that is registered in FrostfsID contract. -S3_GW_FROSTFSID_ENABLED=true # FrostfsID contract hash (LE) or name in NNS. S3_GW_FROSTFSID_CONTRACT=frostfsid.frostfs +# Enables a check to only allow requests to users registered in the FrostfsID contract. +S3_GW_FROSTFSID_VALIDATION_ENABLED=true # Policy contract configuration. To enable this functionality the `rpc_endpoint` param must be also set. # Enables using policies from Policy contract. diff --git a/config/config.yaml b/config/config.yaml index 9255d3d..abbc27d 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -163,7 +163,7 @@ cors: frostfs: # Numbers of the object copies (for each replica) to consider PUT to FrostFS successful. # `[0]` or empty list means that object will be processed according to the container's placement policy - set_copies_number: [0] + set_copies_number: [ 0 ] # This flag enables client side object preparing. client_cut: false # Sets max buffer size for read payload in put operations. @@ -228,10 +228,11 @@ web: # FrostfsID contract configuration. To enable this functionality the `rpc_endpoint` param must be also set. frostfsid: - # Enables check that allow requests only users that is registered in FrostfsID contract. - enabled: true # FrostfsID contract hash (LE) or name in NNS. contract: frostfsid.frostfs + validation: + # Enables a check to only allow requests to users registered in the FrostfsID contract. + enabled: true # Policy contract configuration. To enable this functionality the `rpc_endpoint` param must be also set. policy: diff --git a/docs/configuration.md b/docs/configuration.md index 9ebacf3..ec6b34f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -637,14 +637,15 @@ FrostfsID contract configuration. To enable this functionality the `rpc_endpoint ```yaml frostfsid: - enabled: false contract: frostfsid.frostfs + validation: + enabled: false ``` -| Parameter | Type | SIGHUP reload | Default value | Description | -|------------|----------|---------------|-------------------|----------------------------------------------------------------------------------------| -| `enabled` | `bool` | no | true | Enables check that allow requests only users that is registered in FrostfsID contract. | -| `contract` | `string` | no | frostfsid.frostfs | FrostfsID contract hash (LE) or name in NNS. | +| Parameter | Type | SIGHUP reload | Default value | Description | +|----------------------|----------|---------------|---------------------|---------------------------------------------------------------------------------------| +| `contract` | `string` | no | `frostfsid.frostfs` | FrostfsID contract hash (LE) or name in NNS. | +| `validation.enabled` | `bool` | no | `false` | Enables a check to only allow requests to users registered in the FrostfsID contract. | # `policy` section @@ -697,4 +698,3 @@ To override config values for default namespaces use namespace names that are pr } } ``` - diff --git a/go.mod b/go.mod index 1630b61..0d46604 100644 --- a/go.mod +++ b/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-20231220070831-3128352693fc + git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231221111352-06e9c910142d 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 d70ba9d..58e7b8d 100644 --- a/go.sum +++ b/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-20231220070831-3128352693fc h1:ZBZkWBbDmqSdMoq7igIg4EYMIgbyFaLGcpHcU3urDnI= -git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231220070831-3128352693fc/go.mod h1:v43imcuSmDwSNrePe4UTQh8jaE8FmsiKN3FcaEzmRzc= +git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231221111352-06e9c910142d h1:mIq3GcGDoiSTBN4lYqQkLP3NgGK6izNuyvAysR2G/LI= +git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20231221111352-06e9c910142d/go.mod h1:ps6oKO0mxaPJzK3admTB3iwoBXKkHnS73n4PCrqpHBg= 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/frostfs/frostfsid/frostfsid.go b/internal/frostfs/frostfsid/frostfsid.go index 263dac7..420f29c 100644 --- a/internal/frostfs/frostfsid/frostfsid.go +++ b/internal/frostfs/frostfsid/frostfsid.go @@ -3,15 +3,17 @@ package frostfsid import ( "context" "fmt" + "strconv" "strings" "git.frostfs.info/TrueCloudLab/frostfs-contract/frostfsid/client" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/handler" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/authmate" frostfsutil "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/util" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/rpcclient" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/wallet" ) @@ -32,9 +34,9 @@ type Config struct { } var ( - _ middleware.FrostFSID = (*FrostFSID)(nil) - _ authmate.FrostFSID = (*FrostFSID)(nil) - _ handler.FrostFSID = (*FrostFSID)(nil) + _ api.FrostFSID = (*FrostFSID)(nil) + _ authmate.FrostFSID = (*FrostFSID)(nil) + _ handler.FrostFSID = (*FrostFSID)(nil) ) // New creates new FrostfsID contract wrapper that implements auth.FrostFSID interface. @@ -88,3 +90,20 @@ func (f *FrostFSID) GetUserAddress(namespace, name string) (string, error) { return key.Address(), nil } + +func (f *FrostFSID) GetUserGroupIDs(userHash util.Uint160) ([]string, error) { + subjExt, err := f.cli.GetSubjectExtended(userHash) + if err != nil { + if strings.Contains(err.Error(), "not found") { + return nil, nil + } + return nil, err + } + + res := make([]string, len(subjExt.Groups)) + for i, group := range subjExt.Groups { + res[i] = strconv.FormatInt(group.ID, 10) + } + + return res, nil +}