From 97943ec07231e31643a5c77df5a3fbdf50321ee8 Mon Sep 17 00:00:00 2001 From: Samuel Herman Date: Thu, 27 Mar 2025 11:42:32 -0700 Subject: [PATCH 1/4] make tests to pass Signed-off-by: Samuel Herman --- config/roles.yml | 24 + .../security/OpenSearchSecurityPlugin.java | 15 +- .../SettingsPermissionValve.java | 16 + .../SettingsPermissionValveImpl.java | 178 ++++++ .../dlic/rest/api/RolesApiAction.java | 1 + .../security/filter/SecurityFilter.java | 13 +- .../security/securityconf/impl/v7/RoleV7.java | 21 + .../SettingsPermissionValveTests.java | 190 +++++++ .../security/filter/SecurityFilterTests.java | 7 +- src/test/resources/log4j2-test.properties | 3 + .../settings_valve/action_groups.yml | 160 ++++++ .../resources/settings_valve/allowlist.yml | 17 + src/test/resources/settings_valve/audit.yml | 46 ++ src/test/resources/settings_valve/config.yml | 98 ++++ .../settings_valve/internal_users.yml | 29 + .../settings_valve/kirk-keystore.jks | Bin 0 -> 4525 bytes .../settings_valve/node-0-keystore.jks | Bin 0 -> 4593 bytes .../settings_valve/node-1-keystore.jks | Bin 0 -> 4595 bytes .../settings_valve/node-2-keystore.jks | Bin 0 -> 4593 bytes .../resources/settings_valve/nodes_dn.yml | 4 + src/test/resources/settings_valve/roles.yml | 40 ++ .../settings_valve/roles_mapping.yml | 525 ++++++++++++++++++ .../settings_valve/roles_tenants.yml | 28 + src/test/resources/settings_valve/tenants.yml | 11 + .../resources/settings_valve/truststore.jks | Bin 0 -> 1096 bytes .../resources/settings_valve/whitelist.yml | 17 + 26 files changed, 1431 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java create mode 100644 src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java create mode 100644 src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java create mode 100644 src/test/resources/settings_valve/action_groups.yml create mode 100644 src/test/resources/settings_valve/allowlist.yml create mode 100644 src/test/resources/settings_valve/audit.yml create mode 100644 src/test/resources/settings_valve/config.yml create mode 100644 src/test/resources/settings_valve/internal_users.yml create mode 100644 src/test/resources/settings_valve/kirk-keystore.jks create mode 100644 src/test/resources/settings_valve/node-0-keystore.jks create mode 100644 src/test/resources/settings_valve/node-1-keystore.jks create mode 100644 src/test/resources/settings_valve/node-2-keystore.jks create mode 100644 src/test/resources/settings_valve/nodes_dn.yml create mode 100644 src/test/resources/settings_valve/roles.yml create mode 100644 src/test/resources/settings_valve/roles_mapping.yml create mode 100644 src/test/resources/settings_valve/roles_tenants.yml create mode 100644 src/test/resources/settings_valve/tenants.yml create mode 100644 src/test/resources/settings_valve/truststore.jks create mode 100644 src/test/resources/settings_valve/whitelist.yml diff --git a/config/roles.yml b/config/roles.yml index ae8ec30b89..4567cea4f8 100644 --- a/config/roles.yml +++ b/config/roles.yml @@ -2,6 +2,30 @@ _meta: type: "roles" config_version: 2 +# Allows users to update cluster routing and blocks settings +settings_admin: + cluster_permissions: + - "cluster:admin/settings/*" + allowed_settings: + cluster: + - "cluster.routing.*" + - "cluster.blocks.*" + index: + - "index.number_of_replicas" + - "index.refresh_interval" + + +# Allows users to update index routing and blocks settings +index_manager: + index_permissions: + - index_patterns: + - "*" + allowed_actions: + - "indices:admin/settings/*" + allowed_settings: + - "index.number_of_replicas" + - "index.refresh_interval" + # Restrict users so they can only view visualization and dashboard on OpenSearchDashboards kibana_read_only: reserved: true diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 0802cb856c..cf2ed1967c 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -145,14 +145,7 @@ import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.compliance.ComplianceIndexingOperationListener; import org.opensearch.security.compliance.ComplianceIndexingOperationListenerImpl; -import org.opensearch.security.configuration.AdminDNs; -import org.opensearch.security.configuration.ClusterInfoHolder; -import org.opensearch.security.configuration.CompatConfig; -import org.opensearch.security.configuration.ConfigurationRepository; -import org.opensearch.security.configuration.DlsFlsRequestValve; -import org.opensearch.security.configuration.DlsFlsValveImpl; -import org.opensearch.security.configuration.PrivilegesInterceptorImpl; -import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper; +import org.opensearch.security.configuration.*; import org.opensearch.security.dlic.rest.api.Endpoint; import org.opensearch.security.dlic.rest.api.SecurityRestApiActions; import org.opensearch.security.dlic.rest.api.ssl.CertificatesActionType; @@ -272,6 +265,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile IndexResolverReplacer irr; private final AtomicReference namedXContentRegistry = new AtomicReference<>(NamedXContentRegistry.EMPTY);; private volatile DlsFlsRequestValve dlsFlsValve = null; + private volatile SettingsPermissionValve settingsPermissionValve = null; private volatile OpensearchDynamicSetting transportPassiveAuthSetting; private volatile PasswordHasher passwordHasher; private volatile DlsFlsBaseContext dlsFlsBaseContext; @@ -1127,6 +1121,7 @@ public Collection createComponents( if (SSLConfig.isSslOnlyMode()) { dlsFlsValve = new DlsFlsRequestValve.NoopDlsFlsRequestValve(); + settingsPermissionValve = new SettingsPermissionValve.NoopSettingsPermissionValve(); } else { dlsFlsValve = new DlsFlsValveImpl( settings, @@ -1137,10 +1132,12 @@ public Collection createComponents( threadPool, dlsFlsBaseContext ); + settingsPermissionValve = new SettingsPermissionValveImpl(clusterService, adminDns, auditLog); + cr.subscribeOnChange(configMap -> { ((SettingsPermissionValveImpl) settingsPermissionValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); } - sf = new SecurityFilter(settings, evaluator, adminDns, dlsFlsValve, auditLog, threadPool, cs, compatConfig, irr, xffResolver); + sf = new SecurityFilter(settings, evaluator, adminDns, dlsFlsValve, auditLog, threadPool, cs, compatConfig, irr, xffResolver, settingsPermissionValve); final String principalExtractorClass = settings.get(SSLConfigConstants.SECURITY_SSL_TRANSPORT_PRINCIPAL_EXTRACTOR_CLASS, null); diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java new file mode 100644 index 0000000000..08efb17eab --- /dev/null +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java @@ -0,0 +1,16 @@ +package org.opensearch.security.configuration; + +import org.opensearch.action.ActionRequest; +import org.opensearch.core.action.ActionListener; +import org.opensearch.security.privileges.PrivilegesEvaluationContext; + +public interface SettingsPermissionValve { + boolean invoke(PrivilegesEvaluationContext context, ActionListener listener); + + class NoopSettingsPermissionValve implements SettingsPermissionValve { + @Override + public boolean invoke(PrivilegesEvaluationContext context, ActionListener listener) { + return true; + } + } +} \ No newline at end of file diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java new file mode 100644 index 0000000000..e3e39cee54 --- /dev/null +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java @@ -0,0 +1,178 @@ +package org.opensearch.security.configuration; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.action.ActionListener; +import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.privileges.PrivilegesEvaluationContext; +import org.opensearch.security.securityconf.DynamicConfigModel; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.threadpool.ThreadPool; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +public class SettingsPermissionValveImpl implements SettingsPermissionValve { + private static final Logger log = LogManager.getLogger(SettingsPermissionValveImpl.class); + + private final AtomicReference> rolesConfiguration = new AtomicReference<>(); + private final ClusterService clusterService; + private final AdminDNs adminDNs; + private final AuditLog auditLog; + + public SettingsPermissionValveImpl( + ClusterService clusterService, + AdminDNs adminDNs, + AuditLog auditLog + ) { + this.clusterService = clusterService; + this.adminDNs = adminDNs; + this.auditLog = auditLog; + + // Add listener for configuration updates + clusterService.addListener(event -> { + SecurityDynamicConfiguration config = rolesConfiguration.get(); + if (config != null) { + // Handle any cluster state related updates if needed + } + }); + } + + @Override + public boolean invoke(PrivilegesEvaluationContext context, ActionListener listener) { + final ActionRequest request = context.getRequest(); + + // Skip validation for admin users + if (adminDNs.isAdmin(context.getUser())) { + return true; + } + + try { + if (request instanceof ClusterUpdateSettingsRequest) { + return validateClusterSettings(context, (ClusterUpdateSettingsRequest) request, listener); + } else if (request instanceof UpdateSettingsRequest) { + return validateIndexSettings(context, (UpdateSettingsRequest) request, listener); + } + return true; + } catch (Exception e) { + log.error("Error while evaluating settings permissions", e); + listener.onFailure(new SecurityException("Error while evaluating settings permissions: " + e.getMessage())); + return false; + } + } + + private boolean validateClusterSettings( + PrivilegesEvaluationContext context, + ClusterUpdateSettingsRequest request, + ActionListener listener + ) { + // Get allowed settings patterns from user's roles + Set allowedSettings = getAllowedSettingsFromRoles(context); + + // Validate persistent settings + if (!validateSettingsMap(request.persistentSettings(), allowedSettings)) { + auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); + listener.onFailure( + new SecurityException("User not authorized to modify these cluster settings: " + request.persistentSettings().keySet()) + ); + return false; + } + + // Validate transient settings + if (!validateSettingsMap(request.transientSettings(), allowedSettings)) { + auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); + listener.onFailure( + new SecurityException("User not authorized to modify these cluster settings: " + request.transientSettings().keySet()) + ); + return false; + } + + return true; + } + + private boolean validateIndexSettings( + PrivilegesEvaluationContext context, + UpdateSettingsRequest request, + ActionListener listener + ) { + // Get allowed settings patterns from user's roles + Set allowedSettings = getAllowedSettingsFromRoles(context); + + if (!validateSettingsMap(request.settings(), allowedSettings)) { + auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); + listener.onFailure( + new SecurityException("User not authorized to modify these index settings: " + request.settings().keySet()) + ); + return false; + } + + return true; + } + + private Set getAllowedSettingsFromRoles(PrivilegesEvaluationContext context) { + Set allowedSettings = new HashSet<>(); + SecurityDynamicConfiguration roles = rolesConfiguration.get(); + + if (roles != null && roles.getCEntries() != null) { + for (String role : context.getUser().getRoles()) { + RoleV7 roleConfig = roles.getCEntries().get(role); + if (roleConfig != null) { + // Get cluster-level settings permissions + Set clusterSettings = roleConfig.getAllowed_cluster_settings(); + if (clusterSettings != null) { + allowedSettings.addAll(clusterSettings); + } + + // Get index-level settings from index permissions + List indexPermissions = roleConfig.getIndex_permissions(); + if (indexPermissions != null) { + for (RoleV7.Index indexPermission : indexPermissions) { + Set indexSettings = indexPermission.getAllowed_settings(); + if (indexSettings != null) { + allowedSettings.addAll(indexSettings); + } + } + } + } + } + } + + return allowedSettings; + } + + private boolean validateSettingsMap(Settings settingsToValidate, Set allowedPatterns) { + if (settingsToValidate.isEmpty() || allowedPatterns.isEmpty()) { + return true; + } + + for (String key : settingsToValidate.keySet()) { + boolean matched = false; + for (String pattern : allowedPatterns) { + if (WildcardMatcher.from(pattern).test(key)) { + matched = true; + break; + } + } + if (!matched) { + log.debug("Setting {} not allowed for current user", key); + return false; + } + } + return true; + } + + public void updateConfiguration(SecurityDynamicConfiguration rolesConfig) { + if (rolesConfig != null) { + rolesConfiguration.set(rolesConfig.clone()); + } + } +} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java index 4339a11d96..10f62396ae 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java @@ -187,6 +187,7 @@ public Map allowedKeys() { return allowedKeys.put("cluster_permissions", DataType.ARRAY) .put("tenant_permissions", DataType.ARRAY) .put("index_permissions", DataType.ARRAY) + .put("allowed_cluster_settings", DataType.ARRAY) .put("description", DataType.STRING) .build(); } diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index 6b23fb6b53..da030ce0e4 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -81,6 +81,7 @@ import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.CompatConfig; import org.opensearch.security.configuration.DlsFlsRequestValve; +import org.opensearch.security.configuration.SettingsPermissionValve; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.privileges.PrivilegesEvaluator; @@ -113,6 +114,7 @@ public class SecurityFilter implements ActionFilter { private final WildcardMatcher immutableIndicesMatcher; private final RolesInjector rolesInjector; private final UserInjector userInjector; + private final SettingsPermissionValve settingsPermissionValve; public SecurityFilter( final Settings settings, @@ -124,7 +126,8 @@ public SecurityFilter( ClusterService cs, final CompatConfig compatConfig, final IndexResolverReplacer indexResolverReplacer, - final XFFResolver xffResolver + final XFFResolver xffResolver, + final SettingsPermissionValve settingsPermissionValve ) { this.evalp = evalp; this.adminDns = adminDns; @@ -140,6 +143,7 @@ public SecurityFilter( ); this.rolesInjector = new RolesInjector(auditLog); this.userInjector = new UserInjector(settings, threadPool, auditLog, xffResolver); + this.settingsPermissionValve = settingsPermissionValve; log.info("{} indices are made immutable.", immutableIndicesMatcher); } @@ -383,9 +387,16 @@ private void ap if (pres.isAllowed()) { auditLog.logGrantedPrivileges(action, request, task); auditLog.logIndexEvent(action, request, task); + + // Add settings permission check + if (!settingsPermissionValve.invoke(context, listener)) { + return; + } + if (!dlsFlsValve.invoke(context, listener)) { return; } + final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder(); if (createIndexRequestBuilder == null) { chain.proceed(task, action, request, listener); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 2b2da40927..6a62e62065 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -29,6 +29,7 @@ import java.util.Collections; import java.util.List; +import java.util.Set; import com.fasterxml.jackson.annotation.JsonProperty; @@ -44,6 +45,7 @@ public class RoleV7 implements Hideable, StaticDefinable { private String description; private List cluster_permissions = Collections.emptyList(); private List index_permissions = Collections.emptyList(); + private Set allowed_cluster_settings = Collections.emptySet(); private List tenant_permissions = Collections.emptyList(); public RoleV7() { @@ -57,6 +59,7 @@ public static class Index { private List fls = Collections.emptyList(); private List masked_fields = Collections.emptyList(); private List allowed_actions = Collections.emptyList(); + private Set allowed_settings = Collections.emptySet(); public Index() { super(); @@ -102,6 +105,14 @@ public void setAllowed_actions(List allowed_actions) { this.allowed_actions = allowed_actions; } + public Set getAllowed_settings() { + return allowed_settings; + } + + public void setAllowed_settings(Set allowed_settings) { + this.allowed_settings = allowed_settings; + } + @Override public String toString() { return "Index [index_patterns=" @@ -114,6 +125,8 @@ public String toString() { + masked_fields + ", allowed_actions=" + allowed_actions + + ", allowed_settings=" + + allowed_settings + "]"; } } @@ -221,6 +234,14 @@ public void setStatic(boolean _static) { this._static = _static; } + public Set getAllowed_cluster_settings() { + return allowed_cluster_settings; + } + + public void setAllowed_cluster_settings(Set allowed_cluster_settings) { + this.allowed_cluster_settings = allowed_cluster_settings; + } + @Override public String toString() { return "RoleV7 [reserved=" diff --git a/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java new file mode 100644 index 0000000000..06b6967e7f --- /dev/null +++ b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java @@ -0,0 +1,190 @@ +package org.opensearch.security.configuration; + +import org.junit.Assert; +import org.junit.Test; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.test.SingleClusterTest; +import org.opensearch.security.test.helper.rest.RestHelper; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class SettingsPermissionValveTests extends SingleClusterTest { + private static final String STRONG_PASSWORD = "p@sSVVVAA##@worD!"; + private static final String ADMIN_PASSWORD = "nagilum"; + + @Test + public void testClusterSettingsPermissions() throws Exception { + setupTestUsers(); + + // Test admin user can update any settings + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.routing.allocation.enable\":\"none\"}}", + encodeBasicHeader("admin", ADMIN_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user with specific cluster settings permission + response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.routing.allocation.enable\":\"none\"}}", + encodeBasicHeader("cluster_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user without cluster settings permission + response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.routing.allocation.enable\":\"none\"}}", + encodeBasicHeader("no_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + } + + @Test + public void testIndexSettingsPermissions() throws Exception { + setupTestUsers(); + createTestIndex(); + + // Test admin user can update any index settings + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "test-index/_settings", + "{\"index\":{\"number_of_replicas\":2}}", + encodeBasicHeader("admin", ADMIN_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user with specific index settings permission + response = nonSslRestHelper().executePutRequest( + "test-index/_settings", + "{\"index\":{\"number_of_replicas\":1}}", + encodeBasicHeader("index_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user without index settings permission + response = nonSslRestHelper().executePutRequest( + "test-index/_settings", + "{\"index\":{\"number_of_replicas\":3}}", + encodeBasicHeader("no_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + } + + @Test + public void testWildcardSettingsPermissions() throws Exception { + setupTestUsers(); + + // Test user with wildcard cluster settings permission + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.routing.rebalance.enable\":\"none\"}}", + encodeBasicHeader("wildcard_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test non-matching wildcard pattern + response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.fault_detection.enable\":false}}", + encodeBasicHeader("wildcard_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + } + + private void setupTestUsers() throws Exception { + Settings settings = Settings.builder() + .put("plugins.security.restapi.roles_enabled", "admin") + .build(); + setup(settings); + + // Define roles with different settings permissions + Map clusterSettingsRole = new HashMap<>(); + clusterSettingsRole.put("cluster_permissions", Collections.singletonList("cluster:admin/settings/*")); + clusterSettingsRole.put("allowed_cluster_settings", Collections.singletonList("cluster.routing.*")); + updateSecurityConfig("roles", "cluster_settings_role", clusterSettingsRole); + + Map indexSettingsRole = new HashMap<>(); + Map indexPermission = new HashMap<>(); + indexPermission.put("index_patterns", Collections.singletonList("*")); + indexPermission.put("allowed_actions", Collections.singletonList("indices:admin/settings/*")); + indexPermission.put("allowed_settings", Collections.singletonList("index.number_of_replicas")); + indexSettingsRole.put("index_permissions", Collections.singletonList(indexPermission)); + updateSecurityConfig("roles", "index_settings_role", indexSettingsRole); + + Map wildcardSettingsRole = new HashMap<>(); + wildcardSettingsRole.put("cluster_permissions", Collections.singletonList("cluster:admin/settings/*")); + wildcardSettingsRole.put("allowed_cluster_settings", Collections.singletonList("cluster.routing.*")); + updateSecurityConfig("roles", "wildcard_settings_role", wildcardSettingsRole); + + // Create users and assign roles + createUser("cluster_settings_user", STRONG_PASSWORD, Collections.singletonList("cluster_settings_role")); + createUser("index_settings_user", STRONG_PASSWORD, Collections.singletonList("index_settings_role")); + createUser("wildcard_settings_user", STRONG_PASSWORD, Collections.singletonList("wildcard_settings_role")); + createUser("no_settings_user", STRONG_PASSWORD, Collections.singletonList("kibana_user")); + } + + private void createTestIndex() throws Exception { + createIndex("test-index", Settings.EMPTY); + } + + private void createIndex(String name, Settings settings) { + CreateIndexRequest createIndexRequest = new CreateIndexRequest(name) + .settings(settings); + getClient().admin().indices().create(createIndexRequest).actionGet(); + } + + private void updateSecurityConfig(String configType, String name, Map config) { + String endpoint = String.format("_plugins/_security/api/%s/%s", configType, name); + try { + String jsonBody = DefaultObjectMapper.objectMapper.writeValueAsString(config); + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + endpoint, + jsonBody, + encodeBasicHeader("admin", ADMIN_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + } catch (Exception e) { + Assert.fail("Failed to update security config: " + e.getMessage()); + } + } + + private void createUser(String username, String password, List roles) { + Map userConfig = new HashMap<>(); + userConfig.put("password", password); + userConfig.put("backend_roles", roles); + + String endpoint = String.format("_plugins/_security/api/internalusers/%s", username); + try { + String jsonBody = DefaultObjectMapper.objectMapper.writeValueAsString(userConfig); + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + endpoint, + jsonBody, + encodeBasicHeader("admin", ADMIN_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + } catch (Exception e) { + Assert.fail("Failed to create user: " + e.getMessage()); + } + } + + @Override + protected String getResourceFolder() { + return "settings_valve"; + } +} \ No newline at end of file diff --git a/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java index 4f90a0865a..afb88ae7e9 100644 --- a/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java +++ b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java @@ -28,6 +28,7 @@ import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.CompatConfig; import org.opensearch.security.configuration.DlsFlsRequestValve; +import org.opensearch.security.configuration.SettingsPermissionValve; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.resolver.IndexResolverReplacer; @@ -86,7 +87,8 @@ public void testImmutableIndicesWildcardMatcher() { mock(ClusterService.class), mock(CompatConfig.class), mock(IndexResolverReplacer.class), - mock(XFFResolver.class) + mock(XFFResolver.class), + mock(SettingsPermissionValve.class) ); assertThat(expected, equalTo(filter.getImmutableIndicesMatcher())); } @@ -109,7 +111,8 @@ public void testUnexepectedCausesAreNotSendToCallers() { mock(ClusterService.class), mock(CompatConfig.class), mock(IndexResolverReplacer.class), - mock(XFFResolver.class) + mock(XFFResolver.class), + mock(SettingsPermissionValve.class) ); // Act diff --git a/src/test/resources/log4j2-test.properties b/src/test/resources/log4j2-test.properties index 78871d8395..ecb40b6e9e 100644 --- a/src/test/resources/log4j2-test.properties +++ b/src/test/resources/log4j2-test.properties @@ -35,6 +35,9 @@ logger.sslConfig.level = info logger.cas.name = org.opensearch.cluster.service.ClusterApplierService logger.cas.level = error +logger.tests.name = org.opensearch.security.configuration.SettingsPermissionValveTests +logger.tests.level = info + #logger.ncs.name = org.opensearch.cluster.NodeConnectionsService #logger.ncs.level = off #logger.ssl.name = org.opensearch.security.ssl.transport.SecuritySSLNettyTransport diff --git a/src/test/resources/settings_valve/action_groups.yml b/src/test/resources/settings_valve/action_groups.yml new file mode 100644 index 0000000000..7e5f87d0db --- /dev/null +++ b/src/test/resources/settings_valve/action_groups.yml @@ -0,0 +1,160 @@ +--- +_meta: + type: "actiongroups" + config_version: 2 +OPENDISTRO_SECURITY_CLUSTER_ALL: + reserved: false + hidden: false + allowed_actions: + - "cluster:*" + type: "cluster" + description: "Migrated from v6" +ALL: + reserved: false + hidden: false + allowed_actions: + - "indices:*" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_CRUD: + reserved: false + hidden: false + allowed_actions: + - "OPENDISTRO_SECURITY_READ" + - "OPENDISTRO_SECURITY_WRITE" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_SEARCH: + reserved: false + hidden: false + allowed_actions: + - "indices:data/read/search*" + - "indices:data/read/msearch*" + - "OPENDISTRO_SECURITY_SUGGEST" + type: "index" + description: "Migrated from v6" +MONITOR: + reserved: false + hidden: false + allowed_actions: + - "indices:monitor/*" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_DATA_ACCESS: + reserved: false + hidden: false + allowed_actions: + - "indices:data/*" + - "indices:admin/mapping/put" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_CREATE_INDEX: + reserved: false + hidden: false + allowed_actions: + - "indices:admin/create" + - "indices:admin/mapping/put" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_WRITE: + reserved: false + hidden: false + allowed_actions: + - "indices:data/write*" + - "indices:admin/mapping/put" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_MANAGE_ALIASES: + reserved: false + hidden: false + allowed_actions: + - "indices:admin/aliases*" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_READ: + reserved: false + hidden: false + allowed_actions: + - "indices:data/read*" + - "indices:admin/resolve/index" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_DELETE: + reserved: false + hidden: false + allowed_actions: + - "indices:data/write/delete*" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_CLUSTER_COMPOSITE_OPS: + reserved: false + hidden: false + allowed_actions: + - "indices:data/write/bulk" + - "indices:admin/aliases*" + - "indices:data/write/reindex" + - "OPENDISTRO_SECURITY_CLUSTER_COMPOSITE_OPS_RO" + type: "cluster" + description: "Migrated from v6" +OPENDISTRO_SECURITY_CLUSTER_COMPOSITE_OPS_RO: + reserved: false + hidden: false + allowed_actions: + - "indices:data/read/mget" + - "indices:data/read/msearch" + - "indices:data/read/mtv" + - "indices:data/read/coordinate-msearch*" + - "indices:admin/aliases/exists*" + - "indices:admin/aliases/get*" + type: "cluster" + description: "Migrated from v6" +OPENDISTRO_SECURITY_GET: + reserved: false + hidden: false + allowed_actions: + - "indices:data/read/get*" + - "indices:data/read/mget*" + - "indices:admin/resolve/index" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_MANAGE: + reserved: false + hidden: false + allowed_actions: + - "indices:monitor/*" + - "indices:admin/*" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_CLUSTER_MONITOR: + reserved: false + hidden: false + allowed_actions: + - "cluster:monitor/*" + type: "cluster" + description: "Migrated from v6" +OPENDISTRO_SECURITY_INDEX: + reserved: false + hidden: false + allowed_actions: + - "indices:data/write/index*" + - "indices:data/write/update*" + - "indices:admin/mapping/put" + type: "index" + description: "Migrated from v6" +OPENDISTRO_SECURITY_SUGGEST: + reserved: false + hidden: false + allowed_actions: + - "indices:data/read/suggest*" + type: "index" + description: "Migrated from v6" +DATASTREAM_ALL: + reserved: false + hidden: false + allowed_actions: + - "indices:admin/data_stream/get" + - "indices:admin/data_stream/create" + - "indices:admin/data_stream/delete" + - "indices:monitor/data_stream/stats" + type: "index" + description: "Migrated from v6" diff --git a/src/test/resources/settings_valve/allowlist.yml b/src/test/resources/settings_valve/allowlist.yml new file mode 100644 index 0000000000..d90ab38da6 --- /dev/null +++ b/src/test/resources/settings_valve/allowlist.yml @@ -0,0 +1,17 @@ +--- +_meta: + type: "allowlist" + config_version: 2 + +#this name must be config +config: + enabled: false + requests: + /_cat/nodes: + - GET + /_cat/plugins: + - GET + /_cluster/health: + - GET + /_cluster/settings: + - GET diff --git a/src/test/resources/settings_valve/audit.yml b/src/test/resources/settings_valve/audit.yml new file mode 100644 index 0000000000..796c92f828 --- /dev/null +++ b/src/test/resources/settings_valve/audit.yml @@ -0,0 +1,46 @@ +_meta: + type: "audit" + config_version: 2 + +config: + # enable/disable auditlog + enabled: true + + audit: + # rest + enable_rest: false + disabled_rest_categories: [] + + # transport + enable_transport: false + disabled_transport_categories: [] + + # ignore + ignore_users: + - kibanaserver + ignore_requests: [] + + # verbose attributes + resolve_bulk_requests: false + log_request_body: false + resolve_indices: false + exclude_sensitive_headers: false + + compliance: + # enable/disable compliance + enabled: true + + # configs + internal_config: true + external_config: false + + # compliance read + read_metadata_only: false + read_watched_fields: {} + read_ignore_users: [] + + # compliance write + write_metadata_only: false + write_log_diffs: false + write_watched_indices: [] + write_ignore_users: [] diff --git a/src/test/resources/settings_valve/config.yml b/src/test/resources/settings_valve/config.yml new file mode 100644 index 0000000000..3663b3c706 --- /dev/null +++ b/src/test/resources/settings_valve/config.yml @@ -0,0 +1,98 @@ +--- +_meta: + type: "config" + config_version: 2 +config: + dynamic: + filtered_alias_mode: "disallow" + disable_rest_auth: false + disable_intertransport_auth: false + respect_request_indices_options: false + kibana: + multitenancy_enabled: true + private_tenant_enabled: true + default_tenant: "" + server_username: "kibanaserver" + index: ".kibana" + http: + anonymous_auth_enabled: false + xff: + enabled: false + internalProxies: "192\\.168\\.0\\.10|192\\.168\\.0\\.11" + remoteIpHeader: "x-forwarded-for" + + + authc: + authentication_domain_kerb: + http_enabled: false + transport_enabled: false + order: 3 + http_authenticator: + challenge: true + type: "kerberos" + config: {} + authentication_backend: + type: "noop" + config: {} + description: "Migrated from v6" + authentication_domain_proxy: + http_enabled: false + transport_enabled: false + order: 2 + http_authenticator: + challenge: true + type: "proxy" + config: + user_header: "x-proxy-user" + roles_header: "x-proxy-roles" + authentication_backend: + type: "noop" + config: {} + description: "Migrated from v6" + authentication_domain_clientcert: + http_enabled: false + transport_enabled: false + order: 1 + http_authenticator: + challenge: true + type: "clientcert" + config: {} + authentication_backend: + type: "noop" + config: {} + description: "Migrated from v6" + authentication_domain_basic_internal: + http_enabled: true + transport_enabled: true + order: 0 + http_authenticator: + challenge: true + type: "basic" + config: {} + authentication_backend: + type: "intern" + config: {} + description: "Migrated from v6" + authz: + roles_from_xxx: + http_enabled: false + transport_enabled: false + authorization_backend: + type: "xxx" + config: {} + description: "Migrated from v6" + roles_from_myldap: + http_enabled: false + transport_enabled: false + authorization_backend: + type: "ldap" + config: + rolesearch: "(uniqueMember={0})" + resolve_nested_roles: true + rolebase: "ou=groups,o=TEST" + rolename: "cn" + description: "Migrated from v6" + do_not_fail_on_forbidden: false + multi_rolespan_enabled: false + hosts_resolver_mode: "ip-only" + transport_userrname_attribute: null diff --git a/src/test/resources/settings_valve/internal_users.yml b/src/test/resources/settings_valve/internal_users.yml new file mode 100644 index 0000000000..282631ab3a --- /dev/null +++ b/src/test/resources/settings_valve/internal_users.yml @@ -0,0 +1,29 @@ +--- +_meta: + type: "internalusers" + config_version: 2 +admin: + hash: "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m" + reserved: true + backend_roles: + - "admin" + +cluster_settings_user: + hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" + backend_roles: + - "cluster_settings_role" + +index_settings_user: + hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" + backend_roles: + - "index_settings_role" + +wildcard_settings_user: + hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" + backend_roles: + - "wildcard_settings_role" + +no_settings_user: + hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" + backend_roles: + - "kibana_user" \ No newline at end of file diff --git a/src/test/resources/settings_valve/kirk-keystore.jks b/src/test/resources/settings_valve/kirk-keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..dd7562ef81822291c9e48be60b6daef2883288a9 GIT binary patch literal 4525 zcmdVccT^MWwgB){LPGDoiG|JtLlcmi&;&(_6h#7rP(uq{DS;pYA_Afmm4HYQP!tF# zO+it?LY1P3NK;Ux3W)H6$9wKM=ic}3T5sJy?)_ud%--LgH8bDr`F*qXdFwL>1cER& z@XrnA;}zt?2;6|iLvA1t91KukKLBYi?ob|vHEVPXTvFy98Cf88WeS!f7A?SFPvZg)V z7v6j;sEtmwpQf&7q@7zop%6{r2s~o4`h$O*s-O*b>U-1znW*c#zSZBch%8xV*PK!@M#{2`WZYarXxm&{`QatOX?4u9M`&!g z&8@j*V|x00YnIuVYvX+t0oL3*%ZApt%D}TBgx;82*eFi6zP*KyC^rTTB}T2_jyE(_ z;f-sVA5zg)Q)lMvlIJLf*G91S{6-yTXOSiB%Pl6=)ezz52b@>D2PMdN`A#<=9WZ-| z7KW!1+>q&M9FI4m)Uo2)<*krrUc5S@Z)L7G7dgI!>|D_H@?HQ2<&8$8eAM5V`nS_j zNpG-+#Ccy*OAKTS(OGuQwm4>u0HI3?WwUI`J?nS!O@}YqX!P0n!lx^wZ-xy>9X%}X z%zmko=qLu(IqcUGH-zYG9NL}p(8Yv5_N2}HLil{#_GY+#+?8~+txTk4E?9{>Y6;8Q zEZMQQG+Zc7(ONjc_Z{R_?;uOgps(gW9B#f(6USN@W|Q~IWxj{osPJ=+k~BeQ@cQL| z$FtVYsZN!ibBkZ5nzYpH;ZHJ%>jHUIYX_z7yg_w(Pt?L{^X(1y1V;C7nhA@9^p>1jxzk4S}xL;ZtiB54PN2WN{SYvv3Rh9rlt7aOkInvb_wo?IeDCPpYvkB~u~s|dnp zzlO4kJt=!g)8MF%;L*>C7`w|&Q5;Zw^-N`BMf^2O*~k34r_QaBMiXr@B`1*OXV;sA z9gZ{qm~Tz#GUS?VMvHuD!B$x1U&)(!$F*Sp&b2~cEiXb}fOVHxav+Cc&lruM_R&V( z{43J!IB|M3ZPcwnQbKw_ebQNXOK%`r0lA@di3f&@c4}>O5mAX}`oa7CAntB*PQV6H z^L0WBm#47xTa;TPT3rIKe{dQg49MoR+S7kZUoEgm8$1*sT7hyWeFN zn%<%O^kC4vVk0^76VKU1bP6oo&2Wj(7bV7r$yWzRuT)$$b)}BoAG@t&D^9#0)4o5t z3!Xb!WQChtbPDCZqnv#{@Cy@D^Gw)@;l6aTSvhyQ&XWfqXs~EWy=uctyrPgS<;cB3 ziFkO@!mSh-w6rR5LZ+*OP##Ou?k-vUl=T*#5wv)XW2;UyNTZj8}9GudhAJ(K5qN?vK^qvH6t3LGn(B3(e+O(u*EW1 zgo5DqN~(Y+2m~`?c&GqQ{_o?!Vs@#_RA&(9SEpn!M%ab_5c z29yHk05S3?2n7rV(d*;#xkoZwk^xiERpN!)4V9H=l4Tn>l)s)UWPUlhW+IW-e}Nd?fsk@0y>16?UrrsO!Hfv0>DAFyj~ z4;Oe+Dtg6e1A$uu6@_fEHn~Tu=r`1LiAGjPWDh&kDWabnW};Hzp#zbs1*P^&lDQn3 zhr`|MiT6!+C&i|>I%eo`0eSDaQI)&vJ`Nd~4)V@wtQn-jj?ItB_B_)Xwk^s@lernM zO6lqvz0uD;>}Ldz03>%+rJfw%-gl1kJbl2iG7Yh2s)~NTOz*9{732Q!JpEGs6V6Or z?v@;35->T4fIz_@a8cZQAa0bgx9TTIG8{VR`$aOlLZ$GqH0yMO z@Y6xHgC39jse&KLbn`{{Q=Dt?sE>A+&5;&KL6?ewqvkimo8(4^)y-6@f|g&srG3>K zOR=S%Q-k-M?40&}c8i-mnBH1sjS;pxgNV#%ZPwef`YO#$*D?j$PZ$l zv{pW;Ex{36V{U;rt7KE%$r;XTZ-@9eYiWqK{GxeG!gsw=N<4qP;JB$*gcAmX#ZTkm zq-$D~{KvLfP0R=BdS#-bj(0KEcF*w_t(~hvN!;6+PE{lAeAu>?ljx1Nff@jeHBiRi z;8P7Z7*;-^Yp6&xT-dBKQxS9QF4n?^yDy1p2?1 z{23BJ<8P2)b|7Jd|FHo6E0OvuRKyY-<<1OEd3a0BM&Rq!?9JVYy#B1+Qd!I5#H&na zk30jH2V789$$yAG~m0RXuJD{3u#WSeCxe~2|f62_g>oeC8H2- ze_UH=t}fpaj}759iKB>E5tqFUk`YL%VC@Mru%ad`VPXugO{!w0WjwP{qm4bia|eAw z%*&+wPLE(jG-lJ5O7spY(>%Nwf zJoPod6ROYp-O|9-fNjCN;-j5sdwH&MMsRxO-8N?0U+SP@S1s;CxMp2EWy){1+$1-p z-DsI!@Z4bXQA?=y)QQ#3-$e2N-2dzmme zbbuEP3)|)lIMxspuEbuJf&jNE&eccAgGAH(;O4$ri-*b+2mD#*+5{X{T+%#m+hwnw zkf-(~ntl3qGJvy}XGYaA_O+L)P&_p4@%st9PcpPWv(828(5o1EE|+dIyIAoDXB5Rx z2VB*Q-H5fTW<9l){gQ8BNW*?6U`6S{ffb#a&3)ZV>)Im)?txTeS&1&gRUu8nNMOgv z+(FN+c$pG?jSUeB*?EX{HtNS#Gz{^Sc$kU#;o^jQwe_IHf@kKow7!c`4=eK)3_^Qx ze7?GX&f}#QwMYDVJliV?LGc3@|E>ar@q@9&P+{tq3gEv?*MFl5|8b7}D-UjPy+2V- zR(n5asG`m*?|e`x$VJQF^3>hqpOW4P>L+*@OdT!pG`Rd)sD7&MgFN|kU=M2@8(Y6w ztRiYPqHa?A_K*Cd0VS>?dqH_B%{`uuhc64=N%<(+p1=57=C#>Iv?CgYyQ>hI6)OpE zamrO&M9IHI$%MboyIg}=uimEGEuTd;oGskcyAn}lHDB0@@N+%$RGxMe6Cr4funqQ$ z(@D`B1+Ln_7dr$zQ;nCclkF8IVs{r0xqV`xp_b29-!1B1zY$=RRk4Wz8NU~@x|U=) z^7_KegkHLFMM4Z#0{ z2OPh900*E95B~5Q@=w3@{|^RoP6f5c3mqhA=})^K1scsL<<#R9%F6QEx{2Rb`ky~z zrY&|YhQ9HW!AbMWFK9)w`=or^w-wr%%ogmWARu$h2!fZ<*ZkVuCOohaSsN5nyOO&>*~TNv9yR<|${+cysDRR3}7H>+p?a9c8v%bU%E#C4CGWvaeW$ oIl4xa=rqgDw#QA?#n3);-E>Npf^^i;kSQ<9A)UYFuMDdGAEFEd8UO$Q literal 0 HcmV?d00001 diff --git a/src/test/resources/settings_valve/node-0-keystore.jks b/src/test/resources/settings_valve/node-0-keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..5693b7bf88ae5acc096a179fe2d4c9832dabd71a GIT binary patch literal 4593 zcmdUxXH-+$w#QR80YYd72pp=SNDVvm-m4<0^j-s@B?zI3(xeFpB8n76suWR(QY;ju zhzckmMZiMuh=L*@M=3Ha7xqfr*EbS~oAP^X}L4Py^ zk>rM#0IAXytj7&NAT&@g8rTMz5J)iu6o87tSfNlF2oMd-_0m;uVszPZHlFdlsJy{9 zL%fbt;HkbjHMh4O+w0k#4->j*hiOlN`0`b%=KNS4rP+7uBBcFz3W2Q z)g-ni){9hfC5eWepNd1R3(qAC=(QVVWt3)_3af6&Fa&;};0uilDHF-&HVaB=Qh}>2 z=ggVgam<{J18>(}FBv;rvfPljMDtfm8iQSw;w7y9!#n7-)of@?>67#{mgz?;N{cJZ z-#@NyrmDIhmyuJyR7a;b!K@c(_kC6*N`BT`=8-{w5@Pi-MrpT#=h*bcY=5<#T>X`5 z+{ruA7H$F&WnLADB4m*}G!NcIQ@oBXPd!8yc_lQR>?+AZgg0Gi^J7ft)4JO++vP<6 z0u|aC;TZp5sQGHpM0nyynL)#wTTG8`&zpR9@{N(_Reu(@&}cARaK8X7OiZ=#*U?KK z7m@5ER|@xcms=1T1V05IPPVgNtmUHD$&J<#OHl2CD+Fmhz zYiU8(-2k9`%;=`6OfhD@A9*+8jFpjF+R|7RN23`qF#v{5WtFAqh1??yh)x83bmV67 zY+=Y04Rmf+ZzWraRrJd7b;SOr;&L=7eO1t6%cqzg4Kbsuk$e`mp#5<7>tdLlWpay0 zcU%v;AE&>QQ$m}nHmkia=9&ZE?)spU^-00vSsc#6kMd&dfUpdE$%FWWF}HJV_XGKs zxEKzt2`i?IB6(QEcn|xhO`f!>+j`Ksf|)I!xBwvz9xpO(QSsA5^UkMCC=~jYva~mZmauEu<0rapM*G&ItkI| zX4n=+s;WMpTB76dmA|1aR0pw$*fLnZU}7VanWN6E?!tA=Do=Sat0(^DOTe)!e7iF? zPs31DZ!n^0l0Q*lci^LOYTM*%xlW)lTVtXxTh58QaR-ld95SFMuD>3N1G)QOR))r9 z6)QSp@2m{Z_1dFTqFDOyUGmmt%KdXV6U17RMC?PD$ZH0yP!1;Gf?2Y|x6ZzlPEJZg zjnM5j*Ph+UTD$R@$Spe2iYFd88vk{_V+RS+27u)Q!H!FXMxJG1k!!8^V zc06%=(12ZSHAwDwdbLIO;^KP<*9|FM@8bICZmE?=Vfb9R&NsAA0_Vx3@QTPFO$Az-vS>F|l_&usPd9Eye zCd&}`>q3I3W*+5mD*BK(?ABhXU}sGVE#x`w3oZ$@ZxuT*8@_#=bjeD2jKj`WlK6H* zNkoll!x^j>XqG0h`DpFw3L;9*+dDK()mpB-NW&@EnbhughHpT zwe<1SFaEZA%Z6Y_;H(>GR<#6YnrLB~iWPh}C}y1Ba@P;IbR@0-%E$0h!PR#sYTtJq zjXQyW?R*SrUf#DMCAmOhhLTWgyIpl%wY?p(_({T*ywKE-4DPB57trNhLQP5@2fgi91twGB?^E}hI69h8ZFF;9B(%(54 z>)}Zz1iAzU;Qd`mL^7U8-n&~#cOR0!vqvD2xF-UMo@6pLRwSGQ0`TMjNnhsxGTvX( zA0I^W#=Bu%{e3{$y*XF`EGQoR7oieO??ZBR_92i0$nih~wFXI`5rqRZG=B^q>IYD;ht>KCAMp1EfiS2nmB`#FmqUx=Qh}7oD)2;r%m{8jF zHrDRLDL4DR!hOKjgiQz_#H~$t8w$NS@p)G)Vq7`!{xE=`lLb@WVLn7iC!unV9F4r0 zGwr#iG9sjn3t?;No?b7`W>T*Te%Kp!VlIgv!&Maduyx;l)_b2Oj(-_7G)1X$Rnotz zjhnHWZ$J8i(!W0V)FJO$fTP+0FH>`-i;W?STTRy2r$!6nD%dr+@3{#y+J_T5xaYFX z{{bWhDv)TwXxQ>EAi@50Twqk{{~eI_|A-?hkU0MZNGC}o@*a(TxxoLu%b!rB@H>iR zexOLf=D!x9{|RON6;ix0Hlp!E)9zmT7ebxy%UKiS5TOcfqY-z`+8B$JMd`Pe&4er zGTaIaDXnw44o6F_cGS%vo6J&&p5>}%FK9{d`INnHx;S}G1n7%mrW|;rj1`L=ikW;0PkS?{BvY_yJ<8PQ2PaAR^`O+Xdtx|7%F|SK& zs;wzVW!iqN<0s18LAQU#7!(2S+k5!vJ*?4z24!+P7|L+-}bwgz&&+SyKz6gJ(_8ONWe&kg9$f6oy=bUhf zrotw7G}&PYwGg(w6AsWm_b@@wHLpy%QB2?Pe=0kkImT{&PClh8|DXblTB7VwsWAO-%as3rRQRu3@qgmMP1Z5{a2wItgT=%DIC&b8SPxYc|85#SJL`opnLTjf~s28 z$*h*ypzOw4d^u15;C#WB#`VxUCzcBy)B3u^KNrhCDHF=6Pir3F8>O10_#VtPqgiRk z5R&!3m?m0@PevXr9&(*$$VYxTPq|mvvvJc;=W@js5~4rGb>c>X@yKA>?4-s;{fd%4 zf$@hn##nFXlKQtEgwM3{l{L~1w$igX$7*8ot8?p1`LF!sfh*|zFFauWI}d0;IF$!~ zs9yf^t^aReAZnXeV^?6!pMUXrPunS-S!~vQXYsPKoR%JsZ(sYnUeM*Qbgl%w^%d3= zD%F*ppHbw08ep@u%!;nSy)r?Q%}op1DmIdIcz?-4JFRp{j`;C;6hYc zGIruPmTV=^uKPVoCU({AsiwPqm}$&BDs~Vk(_?q2vA?YyyVmbM(XB;6E+BP4?V|&Y zz;NO*@#xO==LHFyf@yWbvCF9g5qAs4H?8#piY^}Wx&1XC+1b^lL8g1W{vm1GC&r>y zW!W5PmKkbL%?ql2&XP6Wm^>NQ0h3TRxtXXc$#|g7*SXxdZWJ1LxLArVyqd?uc7ciU fkgKedwpBVyQD_3{J;vvymZy^>PpdUX45j%uEA}fN literal 0 HcmV?d00001 diff --git a/src/test/resources/settings_valve/node-1-keystore.jks b/src/test/resources/settings_valve/node-1-keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..5df582180efd6363448ec4b2906ec170317b3784 GIT binary patch literal 4595 zcmdUxcT`hb_Q%r*7((wNMWsFJ4MloyN>k}Y5QG4s1xV;f2?P)nMLH-D5fB9_Qk0@n zq)1T^0*E3ay@&$RRG6T?nfGSi{AR7+`u#D#weDH>e$U=_owoO9pS_j66%Ysnp)~N1 z2KT|bpe0Zg=?pX}gn>X*P=Em22k7AlF*q0o7KJc@!Bij^0k$;3UbWPnzVR$5+(}*X z%Cub{x>dnI$4N-MT!W*SjJM?~cC*ahE&KGbIXQi6c1j9`yluOY_O9v6e;&+KEPF-Jf2#w=d4ogc|6)VUcsUTtH4kf@}RAQV~W%EW1t15aL%)m=jzbcE@2M@ll~dyvr`FKeABxftZ zvO??~>HH!Ma&OPded&;72?-OUy#PCNIzjQ_=%fjGx=Bj+-9=_(SFX@>Tz}eZxLHAo zo)hfL9Bp#J#SO!!{ic!qYDXdu$DVTEiO(e1m+6;+1>MF%Gs4M7WCn_iE7=U9=JvGK zMi_`^vnP2zi9`TNUtjf4J)hF=7KHF3sgSJB;B)MP+cM+(mX}t|&l#mT3HGR&+)88g zw~|dc8Ndx+-od;@Y*>izAW0&O5tc^Vk=x2@MwMqmtoGqOP}S8~gX@Szh8*{ccV4_Y z*0jNu_O3TpI`m4`U1!m?0vBZrZYE|UCpW07>y$4(J7>)UDtVgCCv=2yb$**G^6Rc! zQIK^S|BKefK>CHxNMwUT^$7Ox$wCw{`qA#5s|v_YqUOlWQ!JU2&N&-lhU&RHI1SY& z!?UJ(Cc4Q8zAPEvdnU$+svQ2z7K{Ck->QwIIIVA}Hzz*)T7I8vvZxtyP9_}3dOqJf zec__}C-%uPNV)imYx7M-1Pk_hO@FWThfd|S26fBNK8zomWRo;wyDdB!W~0e3f*DbS zXP(JiPr%Jpj|Im4uHWd+eCnjB??f+57!~YQsvh62+RpMAyq+bEbD2lG^b|umsj2!_Jlw|D>*{; zY4KCuIN|m&ZW3`lcdbs%sOR>k?S8odQ{z`=&8JBI_YaM3pMG-Hs-oBICwZosu&#HJEIPu)~blvMpWUVX1xl$YMshs{9mZr@o zMfT?tpRssaJ-1FblUHJ|;8?@#2OmOCX7%|>mwqD=!sqB$7desQFYC|anu~+qh8)hg zp=r505n)W!)GJP7Bud zX-jF&Z;cPVu4OjgCC9&|x)y`7WBxquo{|Kz5wUqv?SW8oLBsP?+I37119L-rY@+iFXbjWH=%moe z(q!#$oqYF+@-JnhrkQRl?WYZU8m%1L(GtS`n@y32Q>YF1#IH2s+#6Jl0>8c<>P&&@ zTVQirS9D;k)^wVJ%DX044&y1YN*ock7iWK!2n}eFanb(hZLYU88I?9wJ@y^*EtI;* zuYBiBEkhi!eg|RU@2nS z<>G=1vj7E+P!N^1q#U3K0>R=aL?i$Ns7C=50)|i?fDV30H11^F79AK&O9cX=(@`t{ z6C82qy3nQ6HE|a(7dzA$>y6?7*#6W>jShD5_Vq%exBw0~)B=V;GV1>9;n&7`^Plo@ zmP82v#|})iNbY|&@ta}Xd@w$4{MwqodqMw4FMyYW4y6Fd0dgo=8F>`SmV-|EKuZ6T zfF+6+ICL-?oCTtzi=qQ)4+J#}%p8rw|1J@~BoaXUm>3Jx%*+VI4R9SOC=2t?0wVcq zX_Az60|fA~KW-7Fen1Ie1`wqm0U?0Fpyl4quZ{82T;yVSaEaWqb>OGB-nR(Uu)|zO zh8DjmKI_2f5nc_WKBCo4EcVn6w$x*Ur;Cw0U3Z9@*IYK6`+P8M+2S zQxYR$roA(I4>RTsuDr5ZTqb4-31BYDj<8WmE!2XC0Sw-U!JrVTf*&Ws z!vcc%Sm893a!f@<4F&)G3-Z%U4`YY1>Y}d>ey0)>S>Nhik$9}b@o1s8 z0N^|L`#6jfV2@%=3z|wr(W=+Y^~@xD=-h7q0Be~9E*v=U!R!DVz^cHaz%1^pJ09VIfH>hVLWOY13+wFU<&MSS<6)7MTSx+xC=^CT z^~dm`WEdrUz@YXkktG4^pX%cu9co4iGKmVMi}0py6*hy}tJ{^N<;MgE_*Ngw4GZ9S zANJ-cSX`A>l4rHJfhlKY&C_0TMMpfUN#oAYJ{x1JcnSaYO+U=f42S6pO_l(CC*7^xwPu8AX7? z?)#K<=h||Lp)`l)_tXSm0(X7je%(Of(I?T2}K6Ff2Xl zeQ{itTVW}=eId`muq3mmeh%@({OVX&o_fxbmISX?>BN(3(=ornhN2m&jx{PviV;WH z?GQs|@7BqAzWbcHMW#KMUa{mchA_J4R_eoL?$}6MR`GhG^UaE8Pq7guvG?`p0 z=+l~c_9Rec)_$|+C(2v_mp@|+3SS|2J6!0?Gzw;*f&?zM(8JtninduqF_P$ zjJ{Tn{DURwOOvRf6;FcSw}}VY!D@Z*bJ$g zQK|fX!_>Fr0=Ez-SNidwyY1;LuIi+PBkecVMnpyob|Y+22)zg5foW0vR8Q=3POKrs zULZt*M{=`jWOu6eiIy9e4n4Yb>zl^Sko#v>Z#|{C8)JKXei=EzNBkaNGJa-1nS@wAQ;8 z_{LjA?>M{IXXQ|O&!jEky}+JCIvhrvL&Qo4;v}M}_-UZ+FnKq$h~4&`%UI%E^`~#C zColM@te7~m8(+5O@6`PoElXTC@6dF$4>OBhL=s0}rFv`*wf5!O#LZ#X4==T<5K9Oh zK--Y~81_2RSe($i-F+)z_jpSEYvSrva%6R(_^ypU?)EifuktS>L~mc82A<~W_PeC- zUa^*SDytT-R@t#;)q=p9ZpQ4X$CszWdLRMIs@xW-H?x)$_j3^SsaZy~pt#-yiSy$9>G4*LBWu&wbtJ^*b-xJZ&BX0zv2( z_{YWQLvY2(D$qk0p#Io55QqT`P+;2tJ0l#)2!?^BA$!4K1`vz_oAIj}Ky`}fZ{+g1 z#+ym<$Jl{q-g!D~g~i$dI_UA2l7c497TNEad~hb{(b)biRrB{*^;sV@mwLs3Lu0N9 z!*0$cAzUwK)Nh|J+u@2@=reoyZg99tkZsu4BFfF!PkUKR{o*QiDIwr~CEmNx)dK*} z?}ta592Zo4G%%@T;5zzf-g^1;Lpaz&?$&%l_lbZTO+qzZH(Po_T{-=ei4~lld;~y*8t2U#lN{ox&Z} zjHEJ2@iIFhzhAd;TyQ>Q>P~$^<+Da2Gb0u;@L+t6KIuyPCDTgOP`)wg2z`=*^n8v1~Jn~(;5eSlK3 z40bsYi}1@o3!9B^(<9@2P>4zCBIn28sCBr4TSFevhvQm$@(6;x5&Ld7&zec-TQ-)DP4aY<3 zFF1y<#9~H|Ze^)%XtCY{x&UQa2zc}Ej9&F%m%`gDw&n90XF4M#FdIdyUODbD_k6#a zu4AT@vD4Z@VIN%x6zclkBblyuRHe^z)rL(u69QYM?K;D7cpSSS{S7S_z4m4g zk-ptl_{_|9-D!l7m^U2mKM)o4C_P~pX2R0f9=95JX z7gLIy-4ZXHDhMh*&e7p{i=WC7IJPi9)1)~s<5+l$GmmG#+?JkhJ&;IgHU46MY;Gn4 zE^4HQoh*4-FPn4E|7i(tp!E4=^5>35r&7D=2xSWS@o&{V9P41z_5r=R%YpcAt6H`_ zZv$iy&@Z_hoplYVZEm!@#l}OQ7o&0@)N0PQgs3}*Bg|ALQnIq5yUoIQnWd&r2e>`! z8us_?xUZ#j@NR3yk;UMh6sflYb@L~(3aRD1eQ@IQve4|vqznX97P~XjI(p@bgzHl- znTW7S&KFwt(7H#-dhofC&Ez}W;vyo3PTF~GCYAc+GUw~YaV*R?!^{B{=7|g~(Zyg7 zCbLg11c9U)#3sX}LkG*2LBX~wwLN4I>(%^BIx7#mNWk8_wDI)SOecucC?Nlx(s}me zKG;J{&gu}e=gX+;--Q!!IX4^plE)gWLVPx8vKxo%n$aQ_&};;14sC9ERXiK|qWq3Q zg$c(DHyuVOh{0M84XA=Zuo$|B6o3Lfmk))2ArLSWU}I#FjyWE)&IShWVE_RU87MAb zFC(1gvP68^vWzPj!3%XEc%%3Lo<9vT;ewpJeZ6of1i;4#wS>W0IrV>zi0Bf$MNas* z$f3jm(cP&%tO9?X5;6C1^YQR;6VcWFJq!9jW&uKcY$z1~4WLm7&?yJ-Y!D7t8`jB>f)3e}$|7{Kp^Tf|{G3LJ0tf-3ZFH?`Qjv z`}JsY^ydaBV9`G=5xsjrDPRr|eH;#+n|GH;QRI#euqool zOK(UlHIu1!P=UlZ&u?2i!J}S;jo79mAy6;~TtopQ=qtiQZ?sTLK@Ko@2L^*e81jF- z5kW2(1ONeoyHkf?yZ|>NGrbuzFfc*E|M&*^In55^g>h3B3VVB4`sO%smv8kp`uACM ziwXk5yT2BL@dLc!-06WI(olOet3P#oOgg7`QNN^zJ?O!sGxM%HNam6W&;lk24=1U(`b2!aO)f+efEcr4Gm#Ej{3W(bDcPC* zB7PFq0z>0v#jgz+MxQ7N+!WZqB--QdxY`*Jl$+sDnq1_=tfAP#pk`I}s70%{<;x4G ze8$}5$t=Oz1{xw%FXE3MxwF*N<#hW$?R+U1YxIFXXsS~^>SD#2v zsn?sNTaaQ!o5U`Aly?#AcZWsMnNq<=ubzdZW^*57*cN3iT# zA5|E9*;&(>vY&jlZv%7c9KiFJGXJ3h{|LSy1`j4jOKHMjFZG35#Rtw|pHx1)XqY5z zQFN3W1qC30eV+IJUbIp7X4!)ePxu(&!;e#G%L?kq+Wqi`fGXdJ4R_%E3RCgG1uVc?it8k&FSXrS^ z-(0Qmt#GSK)(~3eE1rQN73*o=uX0UCziL!{KispjsXmhL>KAP&CETgIhETT>;$b+-9ac}vxqwqRYW~J^rS%Zk$sp<(NDb%l*wVI8s|75r< zd@Z>8qk6@5j+t+%vw#HXnnFXbyRB&!;y!ie!1J8tLCL|BTNiCnaDxXj0qNl)42^c# zN0#BpS8&Oo!E0GnN}H8C(N?SRERW&~ziV9&zI$e&@F|nGb8H*3z)UHa-;l|Y=pCU! zRUHL#Ef|(t`P>r?SIrZwWhTSLi-%lh_Y}Za<0~H&^=y{->ZRZQ4hI>IBhKW-nT!mk zPETs37~U@J6ZueQYa-|AR9gSm&3%bU`QAfCdppJHYsL?w3aWDJZwhDp^uPsh`WFv4 z{^0=w0Hu5Ihttd7?)Co-2GVx<56>3b2p6QZ^}O)Yo0hv;>m*ZFcCER`ZT)M1S37gT za_4fuTW?7NF<#`7+IeiS_?Kt}=_(L;t$5~BxklGIjR>}lRVuywwe*LKWxDB;un z#qVjyoqg2jO&ocR6RkyB^}j_ZMXy-2XydUL%%f&mqX%JS20Zo;v3GQ%*ZT1j-8z-< z&u~3J*Jz*tHk@EAL+RXTD~#I`ORX7>UPvAYyI&-;Wn)OZkz(w1=W7AHv#U#s#Qbz) zjJoX=WmT=dU{q7A_tESBQv$bAGOZXI`?Np`w92NQ#?B0$oZW2S<-O|Fpv(2Ga@$jz4S(kZ6)aDZR%4iLwC_?zl>ZFLP2DeLj?DRVUE{jf zuevBhF1O7(Wm6gySocTIGW*1(eV$9B;`;0vTTFg#{NEb(rIT}K=b}Fjb1T;czF%~J zJvXWSr1ruflS&D9_R!+oM%!-7dj|7DSU(*X&**Xg65JJ`^Q1v#>CNO<+zYwBbZ*?d z_~OrH1uj$8{NZA7e<&F^zr*kL^@%T@I81O~v;2y}qg_#cdfAD~_uNU(_{eU)eyed@ zwDF7CzFS)sZke}dh5S?|W=00a#mNSV27JKuA$^-_(6PO5TDh68Aut( zf&}z_U`{Tr)P0b&I|nr_sKF41*b1rWflnou?Fl4P*XQB8W&mqSg(_*WGTVCEAIq&SU!A5i zKTa;{_@z8?`>odZ{nzR@E!O!J>0Z2Kf^W{+uM4@(oIUGM!g1io{qDaxEn(Yi-iNXr zGP$|ec0t*elL9jz?dy3`eVR$v+J9M>tv;{P_T0o(e%l`~mZ~o`;;7pqlOFw=k5@I> eG{HGy@~6JmhcBkqNFSY0=&xpc>Cc;)T}A+=vX}z^ literal 0 HcmV?d00001 diff --git a/src/test/resources/settings_valve/whitelist.yml b/src/test/resources/settings_valve/whitelist.yml new file mode 100644 index 0000000000..173bdbd201 --- /dev/null +++ b/src/test/resources/settings_valve/whitelist.yml @@ -0,0 +1,17 @@ +--- +_meta: + type: "whitelist" + config_version: 2 + +#this name must be config +config: + enabled: false + requests: + /_cat/nodes: + - GET + /_cat/plugins: + - GET + /_cluster/health: + - GET + /_cluster/settings: + - GET From 5b771efc328f7820aba86fa28e2acc55c77296f2 Mon Sep 17 00:00:00 2001 From: Samuel Herman Date: Thu, 27 Mar 2025 12:17:41 -0700 Subject: [PATCH 2/4] fix wildcard test Signed-off-by: Samuel Herman --- .../configuration/SettingsPermissionValve.java | 1 - .../configuration/SettingsPermissionValveImpl.java | 12 ++++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java index 08efb17eab..f25c722246 100644 --- a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java @@ -1,6 +1,5 @@ package org.opensearch.security.configuration; -import org.opensearch.action.ActionRequest; import org.opensearch.core.action.ActionListener; import org.opensearch.security.privileges.PrivilegesEvaluationContext; diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java index e3e39cee54..4f7423cb99 100644 --- a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java @@ -2,12 +2,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchSecurityException; import org.opensearch.action.ActionRequest; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.securityconf.DynamicConfigModel; @@ -78,11 +80,17 @@ private boolean validateClusterSettings( // Get allowed settings patterns from user's roles Set allowedSettings = getAllowedSettingsFromRoles(context); + // For backwards compatibility we will allow all settings if no allowed settings are defined + if (allowedSettings.isEmpty()) { + return true; + } + log.debug("Allowed settings: {} for user: {}", allowedSettings, context.getUser().getName()); + // Validate persistent settings if (!validateSettingsMap(request.persistentSettings(), allowedSettings)) { auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); listener.onFailure( - new SecurityException("User not authorized to modify these cluster settings: " + request.persistentSettings().keySet()) + new OpenSearchSecurityException("User not authorized to modify these cluster settings: " + request.persistentSettings().keySet(), RestStatus.FORBIDDEN) ); return false; } @@ -91,7 +99,7 @@ private boolean validateClusterSettings( if (!validateSettingsMap(request.transientSettings(), allowedSettings)) { auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); listener.onFailure( - new SecurityException("User not authorized to modify these cluster settings: " + request.transientSettings().keySet()) + new OpenSearchSecurityException("User not authorized to modify these cluster settings: " + request.transientSettings().keySet(), RestStatus.FORBIDDEN) ); return false; } From 7c9ea0d6f53e8fabf4f63a967bdb0dc23df1db99 Mon Sep 17 00:00:00 2001 From: Samuel Herman Date: Thu, 27 Mar 2025 12:31:14 -0700 Subject: [PATCH 3/4] fix license headers Signed-off-by: Samuel Herman --- .../configuration/SettingsPermissionValve.java | 11 +++++++++++ .../configuration/SettingsPermissionValveImpl.java | 11 +++++++++++ .../configuration/SettingsPermissionValveTests.java | 11 +++++++++++ 3 files changed, 33 insertions(+) diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java index f25c722246..a49772b09b 100644 --- a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValve.java @@ -1,3 +1,14 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.configuration; import org.opensearch.core.action.ActionListener; diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java index 4f7423cb99..6401c8f3ca 100644 --- a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java @@ -1,3 +1,14 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.configuration; import org.apache.logging.log4j.LogManager; diff --git a/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java index 06b6967e7f..8b2cc7e320 100644 --- a/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java +++ b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java @@ -1,3 +1,14 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.configuration; import org.junit.Assert; From 308b026977bcd6920f1f6a5c37477b1d09bed05b Mon Sep 17 00:00:00 2001 From: Samuel Herman Date: Mon, 31 Mar 2025 14:25:23 -0700 Subject: [PATCH 4/4] add backwards compatibility and creation scenario Signed-off-by: Samuel Herman --- .../security/OpenSearchSecurityPlugin.java | 2 +- .../SettingsPermissionValveImpl.java | 25 ++- .../SettingsPermissionValveTests.java | 173 +++++++++++++++++- .../settings_valve/internal_users.yml | 7 +- src/test/resources/settings_valve/roles.yml | 13 ++ .../settings_valve/roles_mapping.yml | 5 + 6 files changed, 209 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index cf2ed1967c..4f6d03bd60 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1132,7 +1132,7 @@ public Collection createComponents( threadPool, dlsFlsBaseContext ); - settingsPermissionValve = new SettingsPermissionValveImpl(clusterService, adminDns, auditLog); + settingsPermissionValve = new SettingsPermissionValveImpl(clusterService, threadPool, adminDns, auditLog); cr.subscribeOnChange(configMap -> { ((SettingsPermissionValveImpl) settingsPermissionValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); } diff --git a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java index 6401c8f3ca..781e8a4197 100644 --- a/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/SettingsPermissionValveImpl.java @@ -17,13 +17,13 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.privileges.PrivilegesEvaluationContext; -import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.WildcardMatcher; @@ -44,6 +44,7 @@ public class SettingsPermissionValveImpl implements SettingsPermissionValve { public SettingsPermissionValveImpl( ClusterService clusterService, + ThreadPool threadPool, AdminDNs adminDNs, AuditLog auditLog ) { @@ -56,6 +57,7 @@ public SettingsPermissionValveImpl( SecurityDynamicConfiguration config = rolesConfiguration.get(); if (config != null) { // Handle any cluster state related updates if needed + // TODO: is this needed here? It's used in DlsFlsValveImpl but seems to be working well without it here on cluster updates } }); } @@ -72,13 +74,15 @@ public boolean invoke(PrivilegesEvaluationContext context, ActionListener lis try { if (request instanceof ClusterUpdateSettingsRequest) { return validateClusterSettings(context, (ClusterUpdateSettingsRequest) request, listener); - } else if (request instanceof UpdateSettingsRequest) { - return validateIndexSettings(context, (UpdateSettingsRequest) request, listener); + } else if (request instanceof UpdateSettingsRequest updateSettingsRequest) { + return validateIndexSettings(context, updateSettingsRequest.settings(), updateSettingsRequest, listener); + } else if (request instanceof CreateIndexRequest createIndexRequest) { + return validateIndexSettings(context, createIndexRequest.settings(), createIndexRequest, listener); } return true; } catch (Exception e) { log.error("Error while evaluating settings permissions", e); - listener.onFailure(new SecurityException("Error while evaluating settings permissions: " + e.getMessage())); + listener.onFailure(new OpenSearchSecurityException("Error while evaluating settings permissions: " + e.getMessage(), RestStatus.FORBIDDEN)); return false; } } @@ -120,16 +124,21 @@ private boolean validateClusterSettings( private boolean validateIndexSettings( PrivilegesEvaluationContext context, - UpdateSettingsRequest request, + Settings requestSettings, + ActionRequest request, ActionListener listener ) { // Get allowed settings patterns from user's roles Set allowedSettings = getAllowedSettingsFromRoles(context); - - if (!validateSettingsMap(request.settings(), allowedSettings)) { + // For backwards compatibility we will allow all settings if no allowed settings are defined + if (allowedSettings.isEmpty()) { + return true; + } + log.debug("Allowed settings: {} for user: {}", allowedSettings, context.getUser().getName()); + if (!validateSettingsMap(requestSettings, allowedSettings)) { auditLog.logMissingPrivileges(context.getAction(), request, context.getTask()); listener.onFailure( - new SecurityException("User not authorized to modify these index settings: " + request.settings().keySet()) + new OpenSearchSecurityException("User not authorized to use these settings during index creation/update: " + requestSettings.keySet(), RestStatus.FORBIDDEN) ); return false; } diff --git a/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java index 8b2cc7e320..168ba5a419 100644 --- a/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java +++ b/src/test/java/org/opensearch/security/configuration/SettingsPermissionValveTests.java @@ -13,6 +13,7 @@ import org.junit.Assert; import org.junit.Test; +import org.opensaml.xmlsec.signature.P; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.common.settings.Settings; @@ -116,6 +117,146 @@ public void testWildcardSettingsPermissions() throws Exception { Assert.assertEquals(403, response.getStatusCode()); } + @Test + public void testIndexCreationSettingsPermissions() throws Exception { + setupTestUsers(); + + // Test admin user can create index with any settings + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "test-index-1", + "{\"settings\":{\"index\":{\"number_of_replicas\":2,\"refresh_interval\":\"1s\"}}}", + encodeBasicHeader("admin", ADMIN_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user with specific index settings permission - allowed setting + response = nonSslRestHelper().executePutRequest( + "test-index-2", + "{\"settings\":{\"index\":{\"number_of_replicas\":1}}}", + encodeBasicHeader("index_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user with specific index settings permission - unauthorized setting + response = nonSslRestHelper().executePutRequest( + "test-index-3", + "{\"settings\":{\"index\":{\"number_of_replicas\":1,\"refresh_interval\":\"2s\"}}}", + encodeBasicHeader("index_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + + // Test user without index settings permission + response = nonSslRestHelper().executePutRequest( + "test-index-4", + "{\"settings\":{\"index\":{\"number_of_replicas\":3}}}", + encodeBasicHeader("no_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + } + + // Adding this test which is somewhat redundant with testIndexCreationSettingsPermissions just to make sure other elements of the payload are ignored + // In the privileges evaluation decision + @Test + public void testIndexCreationWithMappingsAndSettings() throws Exception { + setupTestUsers(); + + // Test user with specific index settings permission - allowed setting with mappings + String payload = "{" + + "\"settings\":{\"index\":{\"number_of_replicas\":1}}," + + "\"mappings\":{" + + " \"properties\":{" + + " \"field1\":{\"type\":\"text\"}," + + " \"field2\":{\"type\":\"keyword\"}" + + " }" + + "}" + + "}"; + + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "test-index-mappings", + payload, + encodeBasicHeader("index_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test user with specific index settings permission - unauthorized setting with mappings + payload = "{" + + "\"settings\":{\"index\":{\"number_of_replicas\":1,\"refresh_interval\":\"1s\"}}," + + "\"mappings\":{" + + " \"properties\":{" + + " \"field1\":{\"type\":\"text\"}" + + " }" + + "}" + + "}"; + + response = nonSslRestHelper().executePutRequest( + "test-index-mappings-2", + payload, + encodeBasicHeader("index_settings_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(403, response.getStatusCode()); + } + + /** + * Test backwards compatibility with old roles.yml where allowed_cluster_settings and allowed_settings are not present + * The expected behavior is to allow all settings if the action is allowed + */ + @Test + public void testBackwardsCompatibilitySettingsPermissions() throws Exception { + setupTestUsers(); + + // Test cluster settings update with backwards compatible role + RestHelper.HttpResponse response = nonSslRestHelper().executePutRequest( + "_cluster/settings", + "{\"persistent\":{\"cluster.routing.rebalance.enable\":\"none\"}}", + encodeBasicHeader("backwards_compatible_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test index creation with any settings with backwards compatible role + response = nonSslRestHelper().executePutRequest( + "test-index", + "{\"settings\":{\"index\":{\"number_of_replicas\":3,\"refresh_interval\":\"2s\"}}}", + encodeBasicHeader("backwards_compatible_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test index settings update with backwards compatible role + response = nonSslRestHelper().executePutRequest( + "test-index/_settings", + "{\"index\":{\"number_of_replicas\":2,\"refresh_interval\":\"1s\"}}", + encodeBasicHeader("backwards_compatible_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + + // Test index creation with settings and mappings with backwards compatible role + String payload = "{" + + "\"settings\":{\"index\":{\"number_of_replicas\":2,\"refresh_interval\":\"3s\"}}," + + "\"mappings\":{" + + " \"properties\":{" + + " \"field1\":{\"type\":\"text\"}," + + " \"field2\":{\"type\":\"keyword\"}" + + " }" + + "}" + + "}"; + + response = nonSslRestHelper().executePutRequest( + "test-index-backwards-compat-mappings", + payload, + encodeBasicHeader("backwards_compatible_user", STRONG_PASSWORD) + ); + log.info("Response: {}", response.getBody()); + Assert.assertEquals(200, response.getStatusCode()); + } + private void setupTestUsers() throws Exception { Settings settings = Settings.builder() .put("plugins.security.restapi.roles_enabled", "admin") @@ -131,21 +272,31 @@ private void setupTestUsers() throws Exception { Map indexSettingsRole = new HashMap<>(); Map indexPermission = new HashMap<>(); indexPermission.put("index_patterns", Collections.singletonList("*")); - indexPermission.put("allowed_actions", Collections.singletonList("indices:admin/settings/*")); + indexPermission.put("allowed_actions", List.of("indices:admin/settings/*", "indices:admin/create")); indexPermission.put("allowed_settings", Collections.singletonList("index.number_of_replicas")); indexSettingsRole.put("index_permissions", Collections.singletonList(indexPermission)); updateSecurityConfig("roles", "index_settings_role", indexSettingsRole); Map wildcardSettingsRole = new HashMap<>(); - wildcardSettingsRole.put("cluster_permissions", Collections.singletonList("cluster:admin/settings/*")); + wildcardSettingsRole.put("cluster_permissions", List.of("cluster:admin/settings/*", "indices:admin/create")); wildcardSettingsRole.put("allowed_cluster_settings", Collections.singletonList("cluster.routing.*")); updateSecurityConfig("roles", "wildcard_settings_role", wildcardSettingsRole); + Map backwardCompatibleRole = new HashMap<>(); + backwardCompatibleRole.put("cluster_permissions", List.of("cluster:admin/settings/*", "indices:admin/create")); + // Do not include the allowed_cluster_settings and index allowed_settings to check for backward compatibility + indexPermission = new HashMap<>(); + indexPermission.put("index_patterns", Collections.singletonList("*")); + indexPermission.put("allowed_actions", List.of("indices:admin/settings/*", "indices:admin/create")); + backwardCompatibleRole.put("index_permissions", Collections.singletonList(indexPermission)); + updateSecurityConfig("roles", "backward_compatible_role", backwardCompatibleRole); + // Create users and assign roles createUser("cluster_settings_user", STRONG_PASSWORD, Collections.singletonList("cluster_settings_role")); createUser("index_settings_user", STRONG_PASSWORD, Collections.singletonList("index_settings_role")); createUser("wildcard_settings_user", STRONG_PASSWORD, Collections.singletonList("wildcard_settings_role")); createUser("no_settings_user", STRONG_PASSWORD, Collections.singletonList("kibana_user")); + createUser("backwards_compatible_user", STRONG_PASSWORD, Collections.singletonList("backward_compatible_role")); } private void createTestIndex() throws Exception { @@ -167,8 +318,13 @@ private void updateSecurityConfig(String configType, String name, Map roles) { jsonBody, encodeBasicHeader("admin", ADMIN_PASSWORD) ); - log.info("Response: {}", response.getBody()); - Assert.assertEquals(200, response.getStatusCode()); + log.info("Response: {}", response); + if (response.toString().contains("201 null")) { + // For backwards compatibility + Assert.assertEquals(201, response.getStatusCode()); + } else { + Assert.assertEquals(200, response.getStatusCode()); + } } catch (Exception e) { Assert.fail("Failed to create user: " + e.getMessage()); } diff --git a/src/test/resources/settings_valve/internal_users.yml b/src/test/resources/settings_valve/internal_users.yml index 282631ab3a..ed01d17fa6 100644 --- a/src/test/resources/settings_valve/internal_users.yml +++ b/src/test/resources/settings_valve/internal_users.yml @@ -26,4 +26,9 @@ wildcard_settings_user: no_settings_user: hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" backend_roles: - - "kibana_user" \ No newline at end of file + - "kibana_user" + +backwards_compatible_user: + hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHXXnZUr.X5fsC.CxIi" + backend_roles: + - "backwards_compatible_role" diff --git a/src/test/resources/settings_valve/roles.yml b/src/test/resources/settings_valve/roles.yml index b4f491e989..42b7fdc84a 100644 --- a/src/test/resources/settings_valve/roles.yml +++ b/src/test/resources/settings_valve/roles.yml @@ -14,15 +14,28 @@ index_settings_role: - "*" allowed_actions: - "indices:admin/settings/*" + - "indices:admin/create" allowed_settings: - "index.number_of_replicas" wildcard_settings_role: cluster_permissions: - "cluster:admin/settings/*" + - "indices:admin/create" allowed_cluster_settings: - "cluster.routing.*" +backwards_compatible_role: + cluster_permissions: + - "cluster:admin/settings/*" + - "indices:admin/create" + index_permissions: + - index_patterns: + - "*" + allowed_actions: + - "indices:admin/settings/*" + - "indices:admin/create" + admin: reserved: false hidden: false diff --git a/src/test/resources/settings_valve/roles_mapping.yml b/src/test/resources/settings_valve/roles_mapping.yml index 966c80b5cd..bdffca873c 100644 --- a/src/test/resources/settings_valve/roles_mapping.yml +++ b/src/test/resources/settings_valve/roles_mapping.yml @@ -523,3 +523,8 @@ wildcard_settings_role: hidden: false users: - "wildcard_settings_user" +backwards_compatible_role: + reserved: false + hidden: false + users: + - "backwards_compatible_user"