diff --git a/config/roles.yml b/config/roles.yml index 570168fe10..77906290a0 100644 --- a/config/roles.yml +++ b/config/roles.yml @@ -15,6 +15,7 @@ security_rest_api_full_access: cluster_permissions: - 'restapi:admin/actiongroups' - 'restapi:admin/allowlist' + - 'restapi:admin/config/update' - 'restapi:admin/internalusers' - 'restapi:admin/nodesdn' - 'restapi:admin/roles' diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 8b1e307172..9f2f63e782 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -27,33 +27,6 @@ package org.opensearch.security; // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.LinkOption; -import java.nio.file.Path; -import java.nio.file.attribute.PosixFilePermission; -import java.security.AccessController; -import java.security.MessageDigest; -import java.security.PrivilegedAction; -import java.security.Security; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiFunction; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.function.Supplier; -import java.util.function.UnaryOperator; -import java.util.stream.Collectors; -import java.util.stream.Stream; import com.google.common.collect.Lists; import org.apache.logging.log4j.LogManager; @@ -61,13 +34,11 @@ import org.apache.lucene.search.QueryCachingPolicy; import org.apache.lucene.search.Weight; import org.bouncycastle.jce.provider.BouncyCastleProvider; - import org.opensearch.OpenSearchException; import org.opensearch.OpenSearchSecurityException; import org.opensearch.SpecialPermission; import org.opensearch.Version; import org.opensearch.action.ActionRequest; -import org.opensearch.core.action.ActionResponse; import org.opensearch.action.search.PitService; import org.opensearch.action.search.SearchScrollAction; import org.opensearch.action.support.ActionFilter; @@ -80,7 +51,6 @@ import org.opensearch.common.lifecycle.Lifecycle; import org.opensearch.common.lifecycle.LifecycleComponent; import org.opensearch.common.lifecycle.LifecycleListener; -import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.network.NetworkModule; import org.opensearch.common.network.NetworkService; @@ -93,14 +63,18 @@ import org.opensearch.common.util.BigArrays; import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.core.index.Index; import org.opensearch.core.indices.breaker.CircuitBreakerService; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.transport.TransportResponse; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.http.HttpServerTransport; import org.opensearch.http.HttpServerTransport.Dispatcher; -import org.opensearch.core.index.Index; import org.opensearch.index.IndexModule; import org.opensearch.index.cache.query.QueryCache; import org.opensearch.indices.IndicesService; @@ -111,7 +85,6 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; -import org.opensearch.core.rest.RestStatus; import org.opensearch.script.ScriptService; import org.opensearch.search.internal.InternalScrollSearchRequest; import org.opensearch.search.internal.ReaderContext; @@ -140,6 +113,7 @@ import org.opensearch.security.configuration.PrivilegesInterceptorImpl; import org.opensearch.security.configuration.Salt; import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper; +import org.opensearch.security.dlic.rest.api.Endpoint; import org.opensearch.security.dlic.rest.api.SecurityRestApiActions; import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.filter.SecurityFilter; @@ -190,10 +164,42 @@ import org.opensearch.transport.TransportRequest; import org.opensearch.transport.TransportRequestHandler; import org.opensearch.transport.TransportRequestOptions; -import org.opensearch.core.transport.TransportResponse; import org.opensearch.transport.TransportResponseHandler; import org.opensearch.transport.TransportService; import org.opensearch.watcher.ResourceWatcherService; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.security.AccessController; +import java.security.MessageDigest; +import java.security.PrivilegedAction; +import java.security.Security; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.function.UnaryOperator; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; +import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting; +import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION; // CS-ENFORCE-SINGLE public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin @@ -332,14 +338,11 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) sm.checkPermission(new SpecialPermission()); } - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Object run() { - if (Security.getProvider("BC") == null) { - Security.addProvider(new BouncyCastleProvider()); - } - return null; + AccessController.doPrivileged((PrivilegedAction) () -> { + if (Security.getProvider("BC") == null) { + Security.addProvider(new BouncyCastleProvider()); } + return null; }); final String advancedModulesEnabledKey = ConfigConstants.SECURITY_ADVANCED_MODULES_ENABLED; @@ -347,6 +350,12 @@ public Object run() { deprecationLogger.deprecate("Setting {} is ignored.", advancedModulesEnabledKey); } + checkForDeprecatedSetting( + settings, + SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, + ENDPOINTS_WITH_PERMISSIONS.get(Endpoint.CONFIG).build(SECURITY_CONFIG_UPDATE) + " permission" + ); + log.info("Clustername: {}", settings.get("cluster.name", "opensearch")); if (!transportSSLEnabled && !SSLConfig.isSslOnlyMode()) { @@ -1788,7 +1797,7 @@ public List> getSettings() { ); settings.add( Setting.boolSetting( - ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, + SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, false, Property.NodeScope, Property.Filtered diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java index d05803be50..a63c496e38 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java @@ -35,9 +35,11 @@ public class RestApiAdminPrivilegesEvaluator { protected final Logger logger = LogManager.getLogger(RestApiAdminPrivilegesEvaluator.class); - public final static String CERTS_INFO_ACTION = "certs"; + public final static String CERTS_INFO_ACTION = "certs/info"; - public final static String RELOAD_CERTS_ACTION = "reloadcerts"; + public final static String RELOAD_CERTS_ACTION = "certs/reload"; + + public final static String SECURITY_CONFIG_UPDATE = "update"; private final static String REST_API_PERMISSION_PREFIX = "restapi:admin"; @@ -61,21 +63,13 @@ default String build() { public final static Map ENDPOINTS_WITH_PERMISSIONS = ImmutableMap.builder() .put(Endpoint.ACTIONGROUPS, action -> buildEndpointPermission(Endpoint.ACTIONGROUPS)) .put(Endpoint.ALLOWLIST, action -> buildEndpointPermission(Endpoint.ALLOWLIST)) + .put(Endpoint.CONFIG, action -> buildEndpointActionPermission(Endpoint.CONFIG, action)) .put(Endpoint.INTERNALUSERS, action -> buildEndpointPermission(Endpoint.INTERNALUSERS)) .put(Endpoint.NODESDN, action -> buildEndpointPermission(Endpoint.NODESDN)) .put(Endpoint.ROLES, action -> buildEndpointPermission(Endpoint.ROLES)) .put(Endpoint.ROLESMAPPING, action -> buildEndpointPermission(Endpoint.ROLESMAPPING)) .put(Endpoint.TENANTS, action -> buildEndpointPermission(Endpoint.TENANTS)) - .put(Endpoint.SSL, action -> { - switch (action) { - case CERTS_INFO_ACTION: - return buildEndpointActionPermission(Endpoint.SSL, "certs/info"); - case RELOAD_CERTS_ACTION: - return buildEndpointActionPermission(Endpoint.SSL, "certs/reload"); - default: - return null; - } - }) + .put(Endpoint.SSL, action -> buildEndpointActionPermission(Endpoint.SSL, action)) .build(); private final ThreadContext threadContext; diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java index 6c6ea0f827..f71135ce50 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java @@ -11,42 +11,41 @@ package org.opensearch.security.dlic.rest.api; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.Settings; -import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.security.dlic.rest.validation.EndpointValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType; -import org.opensearch.security.dlic.rest.validation.ValidationResult; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; -import java.util.Collections; import java.util.List; import java.util.Map; import static org.opensearch.security.dlic.rest.api.RequestHandler.methodNotImplementedHandler; -import static org.opensearch.security.dlic.rest.api.Responses.methodNotImplementedMessage; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; public class SecurityConfigApiAction extends AbstractApiAction { - private static final List getRoutes = addRoutesPrefix(Collections.singletonList(new Route(Method.GET, "/securityconfig/"))); - - private static final List allRoutes = new ImmutableList.Builder().addAll(getRoutes) - .addAll( - addRoutesPrefix(ImmutableList.of(new Route(Method.PUT, "/securityconfig/config"), new Route(Method.PATCH, "/securityconfig/"))) + private static final List routes = addRoutesPrefix( + List.of( + new Route(Method.GET, "/securityconfig"), + new Route(Method.PATCH, "/securityconfig"), + new Route(Method.PUT, "/securityconfig/config") ) - .build(); + ); private final boolean allowPutOrPatch; + private final boolean restApiAdminEnabled; + @Inject public SecurityConfigApiAction( final ClusterService clusterService, @@ -57,12 +56,13 @@ public SecurityConfigApiAction( super(Endpoint.CONFIG, clusterService, threadPool, securityApiDependencies); allowPutOrPatch = securityApiDependencies.settings() .getAsBoolean(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, false); + this.restApiAdminEnabled = securityApiDependencies.settings().getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false); this.requestHandlersBuilder.configureRequestHandlers(this::securityConfigApiActionRequestHandlers); } @Override public List routes() { - return allowPutOrPatch ? allRoutes : getRoutes; + return routes; } @Override @@ -74,20 +74,27 @@ protected CType getConfigType() { protected void consumeParameters(RestRequest request) {} private void securityConfigApiActionRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { - requestHandlersBuilder.onChangeRequest( - Method.PUT, - request -> withAllowedEndpoint(request).map(ignore -> processPutRequest("config", request)) - ) - .onChangeRequest(Method.PATCH, request -> withAllowedEndpoint(request).map(this::processPatchRequest)) + requestHandlersBuilder.withAccessHandler(this::accessHandler) + .verifyAccessForAllMethods() + .onChangeRequest(Method.PUT, request -> processPutRequest("config", request)) + .onChangeRequest(Method.PATCH, this::processPatchRequest) .override(Method.DELETE, methodNotImplementedHandler) .override(Method.POST, methodNotImplementedHandler); } - ValidationResult withAllowedEndpoint(final RestRequest request) { - if (!allowPutOrPatch) { - return ValidationResult.error(RestStatus.NOT_IMPLEMENTED, methodNotImplementedMessage(request.method())); + boolean accessHandler(final RestRequest request) { + switch (request.method()) { + case PATCH: + case PUT: + if (!restApiAdminEnabled) { + return allowPutOrPatch; + } else { + return securityApiDependencies.restApiAdminPrivilegesEvaluator() + .isCurrentUserAdminFor(endpoint, SECURITY_CONFIG_UPDATE); + } + default: + return true; } - return ValidationResult.success(request); } @Override diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index 2eda752a82..78f9ce91df 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -95,7 +95,7 @@ public static Collection getHandler( new AllowlistApiAction(Endpoint.ALLOWLIST, clusterService, threadPool, securityApiDependencies), new AuditApiAction(clusterService, threadPool, securityApiDependencies), new MultiTenancyConfigApiAction(clusterService, threadPool, securityApiDependencies), - new SecuritySSLCertsAction(clusterService, threadPool, securityKeyStore, certificatesReloadEnabled, securityApiDependencies) + new SecuritySSLCertsApiAction(clusterService, threadPool, securityKeyStore, certificatesReloadEnabled, securityApiDependencies) ); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java similarity index 90% rename from src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java rename to src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java index 639d93c6ab..1dee3d8c84 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java @@ -37,6 +37,8 @@ import static org.opensearch.security.dlic.rest.api.Responses.badRequestMessage; import static org.opensearch.security.dlic.rest.api.Responses.ok; import static org.opensearch.security.dlic.rest.api.Responses.response; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.CERTS_INFO_ACTION; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.RELOAD_CERTS_ACTION; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; /** @@ -45,7 +47,7 @@ * This action serves GET request for _plugins/_security/api/ssl/certs endpoint and * PUT _plugins/_security/api/ssl/{certType}/reloadcerts */ -public class SecuritySSLCertsAction extends AbstractApiAction { +public class SecuritySSLCertsApiAction extends AbstractApiAction { private static final List ROUTES = addRoutesPrefix( ImmutableList.of(new Route(Method.GET, "/ssl/certs"), new Route(Method.PUT, "/ssl/{certType}/reloadcerts/")) ); @@ -56,7 +58,7 @@ public class SecuritySSLCertsAction extends AbstractApiAction { private final boolean httpsEnabled; - public SecuritySSLCertsAction( + public SecuritySSLCertsApiAction( final ClusterService clusterService, final ThreadPool threadPool, final SecurityKeyStore securityKeyStore, @@ -116,18 +118,17 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild }).error((status, toXContent) -> response(channel, status, toXContent))); } - private boolean accessHandler(final RestRequest request) { - switch (request.method()) { - case GET: - return securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(endpoint, "certs"); - case PUT: - return securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(endpoint, "reloadcerts"); - default: - return false; + boolean accessHandler(final RestRequest request) { + if (request.method() == Method.GET) { + return securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(endpoint, CERTS_INFO_ACTION); + } else if (request.method() == Method.PUT) { + return securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(endpoint, RELOAD_CERTS_ACTION); + } else { + return false; } } - private ValidationResult withSecurityKeyStore() { + ValidationResult withSecurityKeyStore() { if (securityKeyStore == null) { return ValidationResult.error(RestStatus.OK, badRequestMessage("keystore is not initialized")); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java index e0bee59cec..7b98494e1b 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java @@ -36,31 +36,47 @@ public SecurityConfigApiActionTest() { } @Test - public void testSecurityConfigApiRead() throws Exception { + public void testSecurityConfigApiReadForSuperAdmin() throws Exception { setup(); rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; - HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); + verifyResponsesWithoutPermissionOrUnsupportedFlag(); + } + + @Test + public void testSecurityConfigApiReadRestApiUser() throws Exception { + + setupWithRestRoles(); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = false; + + final var restApiHeader = encodeBasicHeader("test", "test"); + verifyResponsesWithoutPermissionOrUnsupportedFlag(restApiHeader); + } + + private void verifyResponsesWithoutPermissionOrUnsupportedFlag(final Header... headers) { + HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", headers); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", new Header[0]); + response = rh.executePutRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", new Header[0]); + response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - response = rh.executePatchRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", new Header[0]); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); + response = rh.executePatchRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", new Header[0]); + response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", headers); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); } @Test - public void testSecurityConfigApiWrite() throws Exception { + public void testSecurityConfigApiWriteWithUnsupportedFlagForSuperAdmin() throws Exception { Settings settings = Settings.builder() .put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true) @@ -70,52 +86,66 @@ public void testSecurityConfigApiWrite() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; - HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); + verifyWriteOperations(); + } + + @Test + public void testSecurityConfigApiWriteWithFullListOfPermissions() throws Exception { + + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); + setupWithRestRoles(settings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = false; + + final var restAdminFullAccess = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); + verifyWriteOperations(restAdminFullAccess); + } + + @Test + public void testSecurityConfigApiWriteWithOnePermission() throws Exception { + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); + setupWithRestRoles(settings); + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = false; + final var updateOnlyRestApiHeader = encodeBasicHeader("rest_api_admin_config_update", "rest_api_admin_config_update"); + verifyWriteOperations(updateOnlyRestApiHeader); + } + + private void verifyWriteOperations(final Header... header) throws Exception { + HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePutRequest( - ENDPOINT + "/securityconfig/xxx", - FileHelper.loadFile("restapi/securityconfig.json"), - new Header[0] - ); + response = rh.executePutRequest(ENDPOINT + "/securityconfig/xxx", FileHelper.loadFile("restapi/securityconfig.json"), header); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest( - ENDPOINT + "/securityconfig/config", - FileHelper.loadFile("restapi/securityconfig.json"), - new Header[0] - ); + response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", FileHelper.loadFile("restapi/securityconfig.json"), header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePutRequest( - ENDPOINT + "/securityconfig/config", - FileHelper.loadFile("restapi/invalid_config.json"), - new Header[0] - ); + response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", FileHelper.loadFile("restapi/invalid_config.json"), header); Assert.assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, response.getStatusCode()); Assert.assertTrue(response.getContentType(), response.isJsonContentType()); Assert.assertTrue(response.getBody().contains("Unrecognized field")); - response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); + response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", new Header[0]); + response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", header); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); response = rh.executePatchRequest( ENDPOINT + "/securityconfig", "[{\"op\": \"replace\",\"path\": \"/config/dynamic/hosts_resolver_mode\",\"value\": \"other\"}]", - new Header[0] + header ); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", new Header[0]); + response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", header); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - } @Test - public void testSecurityConfigForHTTPPatch() throws Exception { + public void testSecurityConfigForPatchWithUnsupportedFlag() throws Exception { Settings settings = Settings.builder() .put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true) @@ -124,28 +154,58 @@ public void testSecurityConfigForHTTPPatch() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; + verifyPatch(); + } + + @Test + public void testSecurityConfigForPatchWithFullPermissions() throws Exception { + + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); + setupWithRestRoles(settings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = false; // non-default config + final var restAdminFullAccess = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); + verifyPatch(restAdminFullAccess); + } + + @Test + public void testSecurityConfigForPatchWithOnePermission() throws Exception { + + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); + setupWithRestRoles(settings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = false; + + // non-default config + final var updateOnlyRestApiHeader = encodeBasicHeader("rest_api_admin_config_update", "rest_api_admin_config_update"); + verifyPatch(updateOnlyRestApiHeader); + } + + private void verifyPatch(final Header... header) throws Exception { String updatedConfig = FileHelper.loadFile("restapi/securityconfig_nondefault.json"); // update config - HttpResponse response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", updatedConfig, new Header[0]); + HttpResponse response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", updatedConfig, header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); // make patch request response = rh.executePatchRequest( ENDPOINT + "/securityconfig", "[{\"op\": \"add\",\"path\": \"/config/dynamic/do_not_fail_on_forbidden\",\"value\": \"false\"}]", - new Header[0] + header ); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); // get config - response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); + response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); // verify configs are same Assert.assertEquals(DefaultObjectMapper.readTree(updatedConfig), DefaultObjectMapper.readTree(response.getBody()).get("config")); - } + } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java index e3cbfe4120..af80ad3a4d 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java @@ -13,29 +13,32 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; -import org.opensearch.core.rest.RestStatus; -import org.opensearch.security.support.ConfigConstants; +import org.opensearch.rest.RestRequest; import org.opensearch.security.util.FakeRestRequest; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; +import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION; public class SecurityConfigApiActionValidationTest extends AbstractApiActionValidationTest { @Test - public void withAllowedEndpoint() { - var securityConfigApiAction = new SecurityConfigApiAction( + public void accessHandlerForDefaultSettings() { + final var securityConfigApiAction = new SecurityConfigApiAction( clusterService, threadPool, new SecurityApiDependencies(null, configurationRepository, null, null, restApiAdminPrivilegesEvaluator, null, Settings.EMPTY) ); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); + assertFalse(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); + assertFalse(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PATCH).build())); + } - var result = securityConfigApiAction.withAllowedEndpoint(FakeRestRequest.builder().build()); - assertFalse(result.isValid()); - assertEquals(RestStatus.NOT_IMPLEMENTED, result.status()); - - securityConfigApiAction = new SecurityConfigApiAction( + @Test + public void accessHandlerForUnsupportedSetting() { + final var securityConfigApiAction = new SecurityConfigApiAction( clusterService, threadPool, new SecurityApiDependencies( @@ -45,11 +48,32 @@ public void withAllowedEndpoint() { null, restApiAdminPrivilegesEvaluator, null, - Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build() + Settings.builder().put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build() ) ); - result = securityConfigApiAction.withAllowedEndpoint(FakeRestRequest.builder().build()); - assertTrue(result.isValid()); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PATCH).build())); } + @Test + public void accessHandlerForRestAdmin() { + when(restApiAdminPrivilegesEvaluator.isCurrentUserAdminFor(Endpoint.CONFIG, RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE)).thenReturn(true); + final var securityConfigApiAction = new SecurityConfigApiAction( + clusterService, + threadPool, + new SecurityApiDependencies( + null, + configurationRepository, + null, + null, + restApiAdminPrivilegesEvaluator, + null, + Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build() + ) + ); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); + assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PATCH).build())); + } } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiActionValidationTest.java new file mode 100644 index 0000000000..59fa37274b --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiActionValidationTest.java @@ -0,0 +1,84 @@ +/* + * 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.dlic.rest.api; + +import org.junit.Test; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.rest.RestRequest; +import org.opensearch.security.util.FakeRestRequest; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.CERTS_INFO_ACTION; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.RELOAD_CERTS_ACTION; + +public class SecuritySSLCertsApiActionValidationTest extends AbstractApiActionValidationTest { + + @Test + public void withSecurityKeyStore() { + final var securitySSLCertsApiAction = new SecuritySSLCertsApiAction( + clusterService, + threadPool, + null, + true, + securityApiDependencies + ); + final var result = securitySSLCertsApiAction.withSecurityKeyStore(); + assertFalse(result.isValid()); + assertEquals(RestStatus.OK, result.status()); + } + + @Test + public void accessDenied() { + final var securitySSLCertsApiAction = new SecuritySSLCertsApiAction( + clusterService, + threadPool, + null, + true, + securityApiDependencies + ); + when(restApiAdminPrivilegesEvaluator.isCurrentUserAdminFor(Endpoint.SSL, CERTS_INFO_ACTION)).thenReturn(false); + when(restApiAdminPrivilegesEvaluator.isCurrentUserAdminFor(Endpoint.SSL, RELOAD_CERTS_ACTION)).thenReturn(false); + assertFalse(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); + assertFalse(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); + + for (final var m : RequestHandler.RequestHandlersBuilder.SUPPORTED_METHODS) { + if (m != RestRequest.Method.GET && m != RestRequest.Method.PUT) { + assertFalse(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(m).build())); + } + } + } + + @Test + public void hasAccess() { + final var securitySSLCertsApiAction = new SecuritySSLCertsApiAction( + clusterService, + threadPool, + null, + true, + securityApiDependencies + ); + when(restApiAdminPrivilegesEvaluator.isCurrentUserAdminFor(Endpoint.SSL, CERTS_INFO_ACTION)).thenReturn(true); + when(restApiAdminPrivilegesEvaluator.isCurrentUserAdminFor(Endpoint.SSL, RELOAD_CERTS_ACTION)).thenReturn(true); + assertTrue(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); + assertTrue(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); + + for (final var m : RequestHandler.RequestHandlersBuilder.SUPPORTED_METHODS) { + if (m != RestRequest.Method.GET && m != RestRequest.Method.PUT) { + assertFalse(securitySSLCertsApiAction.accessHandler(FakeRestRequest.builder().withMethod(m).build())); + } + } + } + +} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java index 3b959b4d26..35cfae3b3b 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java @@ -54,7 +54,7 @@ protected String getEndpointPrefix() { return PLUGINS_PREFIX; } - final int USER_SETTING_SIZE = 7 * 19; // Lines per account entry * number of accounts + final int USER_SETTING_SIZE = 7 * 20; // Lines per account entry * number of accounts private static final String ENABLED_SERVICE_ACCOUNT_BODY = "{" + " \"attributes\": { \"service\": \"true\", " @@ -98,7 +98,7 @@ public void testSecurityRoles() throws Exception { HttpResponse response = rh.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString()); Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(133, settings.size()); + Assert.assertEquals(USER_SETTING_SIZE, settings.size()); response = rh.executePatchRequest( ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", " @@ -153,7 +153,7 @@ public void testUserFilters() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; final int SERVICE_ACCOUNTS_IN_SETTINGS = 1; - final int INTERNAL_ACCOUNTS_IN_SETTINGS = 19; + final int INTERNAL_ACCOUNTS_IN_SETTINGS = 20; final String serviceAccountName = "JohnDoeService"; HttpResponse response; @@ -845,7 +845,7 @@ public void testScoreBasedPasswordRules() throws Exception { HttpResponse response = rh.executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(133, settings.size()); + Assert.assertEquals(USER_SETTING_SIZE, settings.size()); addUserWithPassword( "admin", diff --git a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java index 49a9be8a91..9d104381a6 100644 --- a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java +++ b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java @@ -53,6 +53,7 @@ import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.CERTS_INFO_ACTION; import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS; import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.RELOAD_CERTS_ACTION; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; public class SecurityRolesPermissionsTest { @@ -78,6 +79,8 @@ static String restAdminApiRoleName(final String endpoint) { new SimpleEntry<>(restAdminApiRoleName(CERTS_INFO_ACTION), role(pb.build(CERTS_INFO_ACTION))), new SimpleEntry<>(restAdminApiRoleName(RELOAD_CERTS_ACTION), role(pb.build(RELOAD_CERTS_ACTION))) ); + } else if (e.getKey() == Endpoint.CONFIG) { + return Stream.of(new SimpleEntry<>(restAdminApiRoleName(SECURITY_CONFIG_UPDATE), role(pb.build(SECURITY_CONFIG_UPDATE)))); } else { return Stream.of(new SimpleEntry<>(restAdminApiRoleName(endpoint), role(pb.build()))); } @@ -95,6 +98,8 @@ static String[] allRestApiPermissions() { return ENDPOINTS_WITH_PERMISSIONS.entrySet().stream().flatMap(entry -> { if (entry.getKey() == Endpoint.SSL) { return Stream.of(entry.getValue().build(CERTS_INFO_ACTION), entry.getValue().build(RELOAD_CERTS_ACTION)); + } else if (entry.getKey() == Endpoint.CONFIG) { + return Stream.of(entry.getValue().build(SECURITY_CONFIG_UPDATE)); } else { return Stream.of(entry.getValue().build()); } @@ -130,6 +135,11 @@ public void hasNoExplicitClusterPermissionPermissionForRestAdmin() { endpoint.name(), securityRolesForRole.hasExplicitClusterPermissionPermission(permissionBuilder.build(RELOAD_CERTS_ACTION)) ); + } else if (endpoint == Endpoint.CONFIG) { + Assert.assertFalse( + endpoint.name(), + securityRolesForRole.hasExplicitClusterPermissionPermission(permissionBuilder.build(SECURITY_CONFIG_UPDATE)) + ); } else { Assert.assertFalse( endpoint.name(), @@ -156,6 +166,11 @@ public void hasExplicitClusterPermissionPermissionForRestAdminWitFullAccess() { endpoint.name() + "/" + CERTS_INFO_ACTION, securityRolesForRole.hasExplicitClusterPermissionPermission(permissionBuilder.build(RELOAD_CERTS_ACTION)) ); + } else if (endpoint == Endpoint.CONFIG) { + Assert.assertTrue( + endpoint.name() + "/" + SECURITY_CONFIG_UPDATE, + securityRolesForRole.hasExplicitClusterPermissionPermission(permissionBuilder.build(SECURITY_CONFIG_UPDATE)) + ); } else { Assert.assertTrue( endpoint.name(), @@ -168,10 +183,10 @@ public void hasExplicitClusterPermissionPermissionForRestAdminWitFullAccess() { @Test public void hasExplicitClusterPermissionPermissionForRestAdmin() { - // verify all endpoint except SSL + // verify all endpoint except SSL and verify CONFIG endpoints final Collection noSslEndpoints = ENDPOINTS_WITH_PERMISSIONS.keySet() .stream() - .filter(e -> e != Endpoint.SSL) + .filter(e -> e != Endpoint.SSL && e != Endpoint.CONFIG) .collect(Collectors.toList()); for (final Endpoint endpoint : noSslEndpoints) { final String permission = ENDPOINTS_WITH_PERMISSIONS.get(endpoint).build(); @@ -190,6 +205,15 @@ public void hasExplicitClusterPermissionPermissionForRestAdmin() { ); assertHasNoPermissionsForRestApiAdminOnePermissionRole(Endpoint.SSL, sslAllowRole); } + // verify CONFIG endpoint with 1 action + final SecurityRoles securityConfigAllowRole = configModel.getSecurityRoles() + .filter(ImmutableSet.of(restAdminApiRoleName(SECURITY_CONFIG_UPDATE))); + final PermissionBuilder permissionBuilder = ENDPOINTS_WITH_PERMISSIONS.get(Endpoint.CONFIG); + Assert.assertTrue( + Endpoint.SSL + "/" + SECURITY_CONFIG_UPDATE, + securityConfigAllowRole.hasExplicitClusterPermissionPermission(permissionBuilder.build(SECURITY_CONFIG_UPDATE)) + ); + assertHasNoPermissionsForRestApiAdminOnePermissionRole(Endpoint.CONFIG, securityConfigAllowRole); } void assertHasNoPermissionsForRestApiAdminOnePermissionRole(final Endpoint allowEndpoint, final SecurityRoles allowOnlyRoleForRole) { diff --git a/src/test/resources/restapi/internal_users.yml b/src/test/resources/restapi/internal_users.yml index 658d3f3aa1..d5d26ef4b5 100644 --- a/src/test/resources/restapi/internal_users.yml +++ b/src/test/resources/restapi/internal_users.yml @@ -127,3 +127,9 @@ rest_api_admin_tenants: hidden: false backend_roles: [] description: "REST API Tenats admin user" +rest_api_admin_config_update: + hash: "$2y$12$capXg1HNP49Vxeb6ijzRnu5BLMUE0ZePq1l3MhF8tjnuxg614uaY6" + reserved: false + hidden: false + backend_roles: [] + description: "REST API Config update admin user" diff --git a/src/test/resources/restapi/roles.yml b/src/test/resources/restapi/roles.yml index f639a21a4e..1c3756cb4d 100644 --- a/src/test/resources/restapi/roles.yml +++ b/src/test/resources/restapi/roles.yml @@ -398,6 +398,7 @@ rest_api_admin_full_access: cluster_permissions: - 'restapi:admin/actiongroups' - 'restapi:admin/allowlist' + - 'restapi:admin/config/update' - 'restapi:admin/internalusers' - 'restapi:admin/nodesdn' - 'restapi:admin/roles' @@ -441,3 +442,7 @@ rest_api_admin_tenants_only: reserved: true cluster_permissions: - 'restapi:admin/tenants' +rest_api_admin_config_update_only: + reserved: true + cluster_permissions: + - 'restapi:admin/config/update' diff --git a/src/test/resources/restapi/roles_mapping.yml b/src/test/resources/restapi/roles_mapping.yml index a87287d5ff..8bfe826247 100644 --- a/src/test/resources/restapi/roles_mapping.yml +++ b/src/test/resources/restapi/roles_mapping.yml @@ -195,6 +195,7 @@ opendistro_security_test: - "rest_api_admin_tenants" - "rest_api_admin_ssl_info" - "rest_api_admin_ssl_reloadcerts" + - "rest_api_admin_config_update" and_backend_roles: [] description: "Migrated from v6" opendistro_security_role_starfleet_captains: @@ -260,3 +261,7 @@ rest_api_admin_tenants_only: reserved: false hidden: true users: [rest_api_admin_tenants] +rest_api_admin_config_update_only: + reserved: false + hidden: true + users: [rest_api_admin_config_update]