Make plugins.security.dfm_empty_overrides_all dynamically toggleable

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Make security plugin aware of FIPS build param (-Pcrypto.standard=FIPS-140-3) ([#5952](https://github.com/opensearch-project/security/pull/5952))
 - Hardens input validation for resource sharing APIs ([#5831](https://github.com/opensearch-project/security/pull/5831)
 - Optimize getFieldFilter to only return a predicate when index has FLS restrictions for user ([#5777](https://github.com/opensearch-project/security/pull/5777))
+- Make `plugins.security.dfm_empty_overrides_all` dynamically toggleable ([#6016](https://github.com/opensearch-project/security/pull/6016)
 - Performance optimizations for building internal authorization data structures upon config updates ([#5988](https://github.com/opensearch-project/security/pull/5988))
 - Make encryption_key optional for obo token authenticator ([#6017](https://github.com/opensearch-project/security/pull/6017)
 - [Resource Sharing] Using custom action prefixes for sample resource plugin ([#6020](https://github.com/opensearch-project/security/pull/6020)

src/integrationTest/java/org/opensearch/security/dlsfls/DfmEmptyOverridesAllDynamicSettingTest.java

@@ -0,0 +1,151 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.security.dlsfls;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import org.opensearch.test.framework.TestSecurityConfig.Role;
+import org.opensearch.test.framework.TestSecurityConfig.User;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.TestRestClient;
+import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+
+/**
+ * Integration tests verifying that plugins.security.dfm_empty_overrides_all can be toggled
+ * dynamically via the cluster settings API without requiring a node restart.
+ *
+ * <p>The setting controls whether a role without a DLS/FLS rule overrides roles that do have
+ * restrictions. When true, a role with no restriction on an index grants full access even if
+ * another mapped role restricts it.
+ */
+public class DfmEmptyOverridesAllDynamicSettingTest {
+
+    static final String INDEX = "dfm_test_index";
+
+    static final User ADMIN_USER = new User("admin").roles(ALL_ACCESS);
+
+    /**
+     * User with two roles:
+     *  - one role that applies a DLS filter (only docs where dept=sales)
+     *  - one role with a wildcard index pattern and NO DLS rule (unrestricted)
+     *
+     * When dfm_empty_overrides_all=true the unrestricted role wins and the user sees all docs.
+     * When dfm_empty_overrides_all=false the restricted role wins and the user sees only sales docs.
+     */
+    static final User MIXED_ROLE_USER = new User("mixed_role_user").roles(
+        new Role("dls_restricted_role").clusterPermissions("cluster_composite_ops_ro")
+            .indexPermissions("read")
+            .dls("{\"term\":{\"dept\":\"sales\"}}")
+            .on(INDEX),
+        new Role("unrestricted_wildcard_role").clusterPermissions("cluster_composite_ops_ro").indexPermissions("read").on("*")
+    );
+
+    @ClassRule
+    public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
+        .anonymousAuth(false)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(ADMIN_USER, MIXED_ROLE_USER)
+        .nodeSettings(Map.of(SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName())))
+        .build();
+
+    @BeforeClass
+    public static void createTestData() throws IOException {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            client.putJson(INDEX + "/_doc/1?refresh=true", "{\"dept\":\"sales\",\"value\":1}");
+            client.putJson(INDEX + "/_doc/2?refresh=true", "{\"dept\":\"engineering\",\"value\":2}");
+            client.putJson(INDEX + "/_doc/3?refresh=true", "{\"dept\":\"marketing\",\"value\":3}");
+        }
+    }
+
+    private void setDfmEmptyOverridesAll(boolean enabled) throws IOException {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            HttpResponse response = client.putJson(
+                "_cluster/settings",
+                String.format("{\"persistent\":{\"plugins.security.dfm_empty_overrides_all\":%b}}", enabled)
+            );
+            assertThat("Failed to update cluster setting", response.getStatusCode(), is(200));
+        }
+    }
+
+    private int countHits(TestRestClient client) throws IOException {
+        HttpResponse response = client.get(INDEX + "/_search?size=10");
+        assertThat("Search failed", response.getStatusCode(), is(200));
+        return response.getIntFromJsonBody("/hits/total/value");
+    }
+
+    @Test
+    public void testSettingFalse_restrictedRoleWins_userSeesOnlySalesDocs() throws IOException {
+        setDfmEmptyOverridesAll(false);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            int hits = countHits(client);
+            assertThat("With dfm_empty_overrides_all=false, DLS filter should apply and only sales doc visible", hits, is(1));
+        }
+    }
+
+    @Test
+    public void testSettingTrue_unrestrictedRoleWins_userSeesAllDocs() throws IOException {
+        setDfmEmptyOverridesAll(true);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            int hits = countHits(client);
+            assertThat("With dfm_empty_overrides_all=true, unrestricted role should override DLS and all docs visible", hits, is(3));
+        }
+    }
+
+    @Test
+    public void testDynamicToggle_fromTrueToFalse_restrictionApplied() throws IOException {
+        setDfmEmptyOverridesAll(true);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            assertThat("Expected all docs visible when setting is true", countHits(client), is(3));
+        }
+
+        setDfmEmptyOverridesAll(false);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            assertThat("Expected only sales doc visible after toggling setting to false", countHits(client), is(1));
+        }
+    }
+
+    @Test
+    public void testDynamicToggle_fromFalseToTrue_restrictionLifted() throws IOException {
+        setDfmEmptyOverridesAll(false);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            assertThat("Expected only sales doc visible when setting is false", countHits(client), is(1));
+        }
+
+        setDfmEmptyOverridesAll(true);
+        try (TestRestClient client = cluster.getRestClient(MIXED_ROLE_USER)) {
+            assertThat("Expected all docs visible after toggling setting to true", countHits(client), is(3));
+        }
+    }
+
+    @Test
+    public void testAdminUser_alwaysSeesAllDocs_regardlessOfSetting() throws IOException {
+        setDfmEmptyOverridesAll(false);
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            assertThat("Admin should always see all docs", countHits(client), is(3));
+        }
+
+        setDfmEmptyOverridesAll(true);
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            assertThat("Admin should always see all docs", countHits(client), is(3));
+        }
+    }
+}

src/integrationTest/java/org/opensearch/security/dlsfls/SensitiveClusterSettingsAccessTest.java

@@ -0,0 +1,101 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.security.dlsfls;
+
+import java.util.List;
+import java.util.Map;
+
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import org.opensearch.test.framework.TestSecurityConfig.Role;
+import org.opensearch.test.framework.TestSecurityConfig.User;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.TestRestClient;
+import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+
+/**
+ * Verifies that plugins.security.dfm_empty_overrides_all (a Sensitive setting) can only be
+ * updated by users whose role is listed in plugins.security.restapi.roles_enabled, and that
+ * a user with only cluster:admin/settings/put is denied with 403.
+ */
+public class SensitiveClusterSettingsAccessTest {
+
+    static final String DFM_SETTING_BODY = "{\"persistent\":{\"plugins.security.dfm_empty_overrides_all\":true}}";
+
+    static final User SECURITY_ADMIN = new User("security_admin").roles(ALL_ACCESS);
+
+    static final Role SETTINGS_ONLY_ROLE = new Role("settings_only_role").clusterPermissions(
+        "cluster:admin/settings/put",
+        "cluster:admin/settings/update"
+    );
+    static final User SETTINGS_USER = new User("settings_user").roles(SETTINGS_ONLY_ROLE);
+
+    @ClassRule
+    public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
+        .anonymousAuth(false)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(SECURITY_ADMIN, SETTINGS_USER)
+        .nodeSettings(Map.of(SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_" + SECURITY_ADMIN.getName() + "__" + ALL_ACCESS.getName())))
+        .build();
+
+    @Test
+    public void adminCertUser_canUpdateSensitiveSetting() {
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
+            HttpResponse response = client.putJson("_cluster/settings", DFM_SETTING_BODY);
+            assertThat(response.getStatusCode(), is(200));
+        }
+    }
+
+    @Test
+    public void securityAdmin_canUpdateSensitiveSetting() {
+        try (TestRestClient client = cluster.getRestClient(SECURITY_ADMIN)) {
+            HttpResponse response = client.putJson("_cluster/settings", DFM_SETTING_BODY);
+            assertThat(response.getStatusCode(), is(200));
+        }
+    }
+
+    @Test
+    public void userWithOnlyClusterSettingsPerm_cannotUpdateSensitiveSetting() {
+        try (TestRestClient client = cluster.getRestClient(SETTINGS_USER)) {
+            HttpResponse response = client.putJson("_cluster/settings", DFM_SETTING_BODY);
+            assertThat(response.getStatusCode(), is(403));
+        }
+    }
+
+    @Test
+    public void userWithOnlyClusterSettingsPerm_cannotUpdateMixedPayloadContainingSensitiveSetting() {
+        try (TestRestClient client = cluster.getRestClient(SETTINGS_USER)) {
+            HttpResponse response = client.putJson(
+                "_cluster/settings",
+                "{\"persistent\":{\"indices.recovery.max_bytes_per_sec\":\"50mb\",\"plugins.security.dfm_empty_overrides_all\":true}}"
+            );
+            assertThat(response.getStatusCode(), is(403));
+        }
+    }
+
+    @Test
+    public void userWithOnlyClusterSettingsPerm_canStillUpdateNonSensitiveSetting() {
+        try (TestRestClient client = cluster.getRestClient(SETTINGS_USER)) {
+            // indices.recovery.max_bytes_per_sec is a core Dynamic setting with no Sensitive property
+            HttpResponse response = client.putJson(
+                "_cluster/settings",
+                "{\"transient\":{\"indices.recovery.max_bytes_per_sec\":\"50mb\"}}"
+            );
+            assertThat(response.getStatusCode(), is(200));
+        }
+    }
+}

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -1646,9 +1646,7 @@ public List<Setting<?>> getSettings() {
                     Property.Filtered
                 )
             );
-            settings.add(
-                Setting.boolSetting(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false, Property.NodeScope, Property.Filtered)
-            );
+            settings.add(SecuritySettings.DFM_EMPTY_OVERRIDES_ALL_SETTING);
             settings.add(Setting.groupSetting(ConfigConstants.SECURITY_AUTHCZ_REST_IMPERSONATION_USERS + ".", Property.NodeScope)); // not
                                                                                                                                     // filtered
                                                                                                                                     // here

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

@@ -146,6 +146,15 @@ public DlsFlsValveImpl(
             clusterService.getClusterSettings().addSettingsUpdateConsumer(SecuritySettings.DLS_WRITE_BLOCKED, newDlsWriteBlockedEnabled -> {
                 dlsWriteBlockedEnabled = newDlsWriteBlockedEnabled;
             });
+            clusterService.getClusterSettings()
+                .addSettingsUpdateConsumer(SecuritySettings.DFM_EMPTY_OVERRIDES_ALL_SETTING, newDfmEmptyOverridesAll -> {
+                    DlsFlsProcessedConfig config = dlsFlsBaseContext.config();
+                    if (config != null) {
+                        config.getDocumentPrivileges().setDfmEmptyOverridesAll(newDfmEmptyOverridesAll);
+                        config.getFieldPrivileges().setDfmEmptyOverridesAll(newDfmEmptyOverridesAll);
+                        config.getFieldMasking().setDfmEmptyOverridesAll(newDfmEmptyOverridesAll);
+                    }
+                });
         }
         this.resourceSharingEnabledSetting = resourceSharingEnabledSetting;
     }

src/main/java/org/opensearch/security/filter/SecurityFilter.java

@@ -31,6 +31,7 @@
 import java.util.UUID;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSet;
@@ -43,6 +44,8 @@
 import org.opensearch.ResourceAlreadyExistsException;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.DocWriteRequest.OpType;
+import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsAction;
+import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
 import org.opensearch.action.admin.indices.alias.Alias;
 import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
@@ -123,6 +126,7 @@ public class SecurityFilter implements ActionFilter {
     private final UserInjector userInjector;
     private final ResourceAccessEvaluator resourceAccessEvaluator;
     private final ThreadContextUserInfo threadContextUserInfo;
+    private final Set<String> restApiAllowedRoles;
 
     public SecurityFilter(
         final Settings settings,
@@ -153,6 +157,7 @@ public SecurityFilter(
         this.rolesInjector = new RolesInjector(auditLog);
         this.userInjector = new UserInjector(settings, threadPool, auditLog, xffResolver);
         this.resourceAccessEvaluator = resourceAccessEvaluator;
+        this.restApiAllowedRoles = Set.copyOf(settings.getAsList(ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED));
         this.threadContextUserInfo = new ThreadContextUserInfo(
             threadPool.getThreadContext(),
             privilegesConfiguration,
@@ -435,6 +440,11 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
                 return;
             }
 
+            // Block cluster-settings updates that touch Sensitive settings unless the user holds a restapi-allowed role
+            if (handleBlockedSensitiveSettingsUpdate(action, request, context, listener)) {
+                return;
+            }
+
             // not a resource‐sharing request → fall back into the normal PrivilegesEvaluator
             PrivilegesEvaluatorResponse pres = eval.evaluate(context);
 
@@ -520,6 +530,37 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
         }
     }
 
+    private <Request extends ActionRequest, Response extends ActionResponse> boolean handleBlockedSensitiveSettingsUpdate(
+        String action,
+        Request request,
+        PrivilegesEvaluationContext context,
+        ActionListener<Response> listener
+    ) {
+        if (!ClusterUpdateSettingsAction.NAME.equals(action)) {
+            return false;
+        }
+        ClusterUpdateSettingsRequest settingsRequest = (ClusterUpdateSettingsRequest) request;
+        boolean touchesSensitiveSetting = Stream.concat(
+            settingsRequest.transientSettings().keySet().stream(),
+            settingsRequest.persistentSettings().keySet().stream()
+        ).anyMatch(key -> cs.getClusterSettings().isSensitiveSetting(key));
+
+        if (!touchesSensitiveSetting) {
+            return false;
+        }
+        if (Collections.disjoint(restApiAllowedRoles, context.getMappedRoles())) {
+            log.debug("User {} does not have a role permitted to update sensitive cluster settings", context.getUser().getName());
+            listener.onFailure(
+                new OpenSearchSecurityException(
+                    "User " + context.getUser().getName() + " does not have permission to update sensitive cluster settings",
+                    RestStatus.FORBIDDEN
+                )
+            );
+            return true;
+        }
+        return false;
+    }
+
     private static boolean isUserAdmin(User user, final AdminDNs adminDns) {
         return user != null && adminDns.isAdmin(user);
     }

src/main/java/org/opensearch/security/privileges/dlsfls/AbstractRuleBasedPrivileges.java

@@ -90,7 +90,7 @@ abstract class AbstractRuleBasedPrivileges<SingleRule, JoinedRule extends Abstra
     /**
      * Corresponds to the settings flag plugins.security.dfm_empty_overrides_all.
      */
-    private final boolean dfmEmptyOverridesAll;
+    private volatile boolean dfmEmptyOverridesAll;
 
     /**
      * Corresponds to the setting plugins.security.privileges_evaluation.precomputed_privileges.enabled
@@ -111,6 +111,10 @@ public AbstractRuleBasedPrivileges(
         this.statefulRules = this.statefulIndexEnabled ? new StatefulRules<>(compiledRoles, indexMetadata, roleToRuleFunction) : null;
     }
 
+    public void setDfmEmptyOverridesAll(boolean dfmEmptyOverridesAll) {
+        this.dfmEmptyOverridesAll = dfmEmptyOverridesAll;
+    }
+
     /**
      * Returns true if the user identified in the PrivilegesEvaluationContext does not have any restrictions in any case,
      * independently of the indices they are requesting.