[#257] Support flag to deny access if policy rules not found

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

api/middleware/policy.go

@@ -18,6 +18,7 @@ import (
 
 type PolicySettings interface {
 	ResolveNamespaceAlias(ns string) string
+	PolicyDenyByDefault() bool
 }
 
 func PolicyCheck(storage engine.ChainRouter, settings PolicySettings, domains []string, log *zap.Logger) Func {
@@ -27,7 +28,7 @@ func PolicyCheck(storage engine.ChainRouter, settings PolicySettings, domains []
 
 			st, err := policyCheck(storage, settings, domains, r)
 			if err == nil {
-				if st != chain.Allow && st != chain.NoRuleFound { // todo drop 'st != chain.NoRuleFound'
+				if st != chain.Allow && (st != chain.NoRuleFound || settings.PolicyDenyByDefault()) {
 					err = apiErr.GetAPIErrorWithError(apiErr.ErrAccessDenied, fmt.Errorf("policy check: %s", st.String()))
 				}
 			}

api/router_mock_test.go

@@ -20,7 +20,9 @@ func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) {
 	return &middleware.Box{}, nil
 }
 
-type middlewareSettingsMock struct{}
+type middlewareSettingsMock struct {
+	denyByDefault bool
+}
 
 func (r *middlewareSettingsMock) NamespaceHeader() string {
 	return FrostfsNamespaceHeader
@@ -30,6 +32,10 @@ func (r *middlewareSettingsMock) ResolveNamespaceAlias(ns string) string {
 	return ns
 }
 
+func (r *middlewareSettingsMock) PolicyDenyByDefault() bool {
+	return r.denyByDefault
+}
+
 type handlerMock struct {
 	t *testing.T
 }

api/router_test.go

@@ -24,8 +24,9 @@ import (
 )
 
 type routerMock struct {
-	router *chi.Mux
+	router             *chi.Mux
-	cfg    Config
+	cfg                Config
+	middlewareSettings *middlewareSettingsMock
 }
 
 func (m *routerMock) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -33,6 +34,8 @@ func (m *routerMock) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 func prepareRouter(t *testing.T) *routerMock {
+	middlewareSettings := &middlewareSettingsMock{}
+
 	cfg := Config{
 		Throttle: middleware.ThrottleOpts{
 			Limit:          10,
@@ -42,12 +45,13 @@ func prepareRouter(t *testing.T) *routerMock {
 		Center:             &centerMock{},
 		Log:                zaptest.NewLogger(t),
 		Metrics:            &metrics.AppMetrics{},
-		MiddlewareSettings: &middlewareSettingsMock{},
+		MiddlewareSettings: middlewareSettings,
 		PolicyStorage:      inmemory.NewInMemoryLocalOverrides(),
 	}
 	return &routerMock{
-		router: NewRouter(cfg),
+		router:             NewRouter(cfg),
-		cfg:    cfg,
+		cfg:                cfg,
+		middlewareSettings: middlewareSettings,
 	}
 }
 
@@ -183,6 +187,24 @@ func TestPolicyChecker(t *testing.T) {
 	assertAPIError(t, w, apiErrors.ErrAccessDenied)
 }
 
+func TestDefaultBehaviorPolicyChecker(t *testing.T) {
+	chiRouter := prepareRouter(t)
+	bktName, objName := "bucket", "object"
+	target := fmt.Sprintf("/%s/%s", bktName, objName)
+
+	// check we can access bucket if rules not found
+	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 cannot access if rules not found when settings is enabled
+	chiRouter.middlewareSettings.denyByDefault = true
+	w, r = httptest.NewRecorder(), httptest.NewRequest(http.MethodPut, target, nil)
+	chiRouter.ServeHTTP(w, r)
+	assertAPIError(t, w, apiErrors.ErrAccessDenied)
+}
+
 func readResponse(t *testing.T, w *httptest.ResponseRecorder) handlerResult {
 	var res handlerResult
 

cmd/s3-gw/app.go

@@ -102,6 +102,7 @@ type (
 		namespaceHeader               string
 		defaultNamespaces             []string
 		authorizedControlAPIKeys      [][]byte
+		policyDenyByDefault           bool
 	}
 
 	maxClientsConfig struct {
@@ -212,6 +213,7 @@ func newAppSettings(log *Logger, v *viper.Viper, key *keys.PrivateKey) *appSetti
 	settings.setBufferMaxSizeForPut(v.GetUint64(cfgBufferMaxSizeForPut))
 	settings.setMD5Enabled(v.GetBool(cfgMD5Enabled))
 	settings.setAuthorizedControlAPIKeys(append(fetchAuthorizedKeys(log.logger, v), key.PublicKey()))
+	settings.setPolicyDenyByDefault(v.GetBool(cfgPolicyDenyByDefault))
 
 	return settings
 }
@@ -397,6 +399,18 @@ func (s *appSettings) ResolveNamespaceAlias(namespace string) string {
 	return namespace
 }
 
+func (s *appSettings) PolicyDenyByDefault() bool {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.policyDenyByDefault
+}
+
+func (s *appSettings) setPolicyDenyByDefault(policyDenyByDefault bool) {
+	s.mu.Lock()
+	s.policyDenyByDefault = policyDenyByDefault
+	s.mu.Unlock()
+}
+
 func (a *App) initAPI(ctx context.Context) {
 	a.initLayer(ctx)
 	a.initHandler()
@@ -775,6 +789,7 @@ func (a *App) updateSettings() {
 	a.settings.setMD5Enabled(a.cfg.GetBool(cfgMD5Enabled))
 	a.settings.setDefaultNamespaces(a.cfg.GetStringSlice(cfgKludgeDefaultNamespaces))
 	a.settings.setAuthorizedControlAPIKeys(append(fetchAuthorizedKeys(a.log, a.cfg), a.key.PublicKey()))
+	a.settings.setPolicyDenyByDefault(a.cfg.GetBool(cfgPolicyDenyByDefault))
 }
 
 func (a *App) startServices() {

cmd/s3-gw/app_settings.go

@@ -198,7 +198,8 @@ const ( // Settings.
 	cfgSoftMemoryLimit = "runtime.soft_memory_limit"
 
 	// Enable return MD5 checksum in ETag.
-	cfgMD5Enabled = "features.md5.enabled"
+	cfgMD5Enabled          = "features.md5.enabled"
+	cfgPolicyDenyByDefault = "features.policy.deny_by_default"
 
 	// FrostfsID.
 	cfgFrostfsIDEnabled  = "frostfsid.enabled"

config/config.env

@@ -160,6 +160,8 @@ S3_GW_TRACING_EXPORTER="otlp_grpc"
 S3_GW_RUNTIME_SOFT_MEMORY_LIMIT=1073741824
 
 S3_GW_FEATURES_MD5_ENABLED=false
+# Enable denying access for request that doesn't match any policy chain rules.
+S3_GW_FEATURES_POLICY_DENY_BY_DEFAULT=false
 
 # ReadTimeout is the maximum duration for reading the entire
 # request, including the body. A zero or negative value means

config/config.yaml

@@ -189,6 +189,9 @@ runtime:
   soft_memory_limit: 1gb
 
 features:
+  policy:
+    # Enable denying access for request that doesn't match any policy chain rules.
+    deny_by_default: false
   md5:
     enabled: false
 

docs/configuration.md

@@ -595,13 +595,16 @@ Contains parameters for enabling features.
 
 ```yaml
 features:
+   policy:
+      deny_by_default: false
    md5:
       enabled: false
 ```
 
-| Parameter     | Type   | SIGHUP reload | Default value | Description                                                    |
+| Parameter                | Type   | SIGHUP reload | Default value | Description                                                                  |
-|---------------|--------|---------------|---------------|----------------------------------------------------------------|
+|--------------------------|--------|---------------|---------------|------------------------------------------------------------------------------|
-| `md5.enabled` | `bool` | yes           | false         | Flag to enable return MD5 checksum in ETag headers and fields. |
+| `md5.enabled`            | `bool` | yes           | false         | Flag to enable return MD5 checksum in ETag headers and fields.               |
+| `policy.deny_by_default` | `bool` | yes           | false         | Enable denying access for request that doesn't match any policy chain rules. |
 
 # `web` section
 Contains web server configuration parameters.