[Backport 2.8] Rest admin permissions (#2411) (#2807)

* role.yml changes for lron feature (#2789) (#2792) Signed-off-by: zhichao-aws <[email protected]> (cherry picked from commit a580dfc) Co-authored-by: zhichao-aws <[email protected]> * add ml model group system index (#2790) (#2797) Signed-off-by: Yaliang Wu <[email protected]> (cherry picked from commit 1bb2ef1) Co-authored-by: Yaliang Wu <[email protected]> * Rest admin permissions (#2411) Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit d676716) * Fixes CI errors Signed-off-by: Darshit Chanpura <[email protected]> * Fixes HTTP5 imports Signed-off-by: Darshit Chanpura <[email protected]> * Fixes password related changes in tests Signed-off-by: Darshit Chanpura <[email protected]> * Update ActionGroupsApiTest.java Remove unused import * Incorporates jar hell fix Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: zhichao-aws <[email protected]> Co-authored-by: Yaliang Wu <[email protected]> Co-authored-by: Andrey Pleskach <[email protected]> Co-authored-by: Stephen Crawford <[email protected]>
opensearch-project · May 30, 2023 · 2ebcfa7 · 2ebcfa7
1 parent ae19524
commit 2ebcfa7
Show file tree

Hide file tree

Showing 47 changed files with 2,421 additions and 866 deletions.
diff --git a/build.gradle b/build.gradle
@@ -289,7 +289,6 @@ configurations.all {
 }
 
 dependencies {
-    implementation 'jakarta.annotation:jakarta.annotation-api:1.3.5'
     implementation "org.opensearch.plugin:transport-netty4-client:${opensearch_version}"
     implementation "org.opensearch.client:opensearch-rest-high-level-client:${opensearch_version}"
     implementation 'com.google.guava:guava:30.0-jre'

diff --git a/config/roles.yml b/config/roles.yml
@@ -9,7 +9,20 @@ kibana_read_only:
 # The security REST API access role is used to assign specific users access to change the security settings through the REST API.
 security_rest_api_access:
   reserved: true
-
+
+security_rest_api_full_access:
+  reserved: true
+  cluster_permissions:
+    - 'restapi:admin/actiongroups'
+    - 'restapi:admin/allowlist'
+    - 'restapi:admin/internalusers'
+    - 'restapi:admin/nodesdn'
+    - 'restapi:admin/roles'
+    - 'restapi:admin/rolesmapping'
+    - 'restapi:admin/ssl/certs/info'
+    - 'restapi:admin/ssl/certs/reload'
+    - 'restapi:admin/tenants'
+
 # Allows users to view monitors, destinations and alerts
 alerting_read_access:
   reserved: true

diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -158,8 +158,6 @@
 import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin;
 import org.opensearch.security.ssl.SslExceptionHandler;
 import org.opensearch.security.ssl.http.netty.ValidatingDispatcher;
-import org.opensearch.security.ssl.rest.SecuritySSLCertsInfoAction;
-import org.opensearch.security.ssl.rest.SecuritySSLReloadCertsAction;
 import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor;
 import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport;
 import org.opensearch.security.ssl.util.SSLConfigConstants;
@@ -473,16 +471,11 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
             if(!SSLConfig.isSslOnlyMode()) {
                 handlers.add(new SecurityInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
                 handlers.add(new SecurityHealthAction(settings, restController, Objects.requireNonNull(backendRegistry)));
-                handlers.add(new SecuritySSLCertsInfoAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
                 handlers.add(new DashboardsInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
                 handlers.add(new TenantInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool),
                         Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
-                handlers.add(new SecurityConfigUpdateAction(settings, restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
-                handlers.add(new SecurityWhoAmIAction(settings ,restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
-                if (sslCertReloadEnabled) {
-                    handlers.add(new SecuritySSLReloadCertsAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
-                }
-
+                handlers.add(new SecurityConfigUpdateAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
+                handlers.add(new SecurityWhoAmIAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
                 handlers.addAll(
                         SecurityRestApiActions.getHandler(
                                 settings,
@@ -494,7 +487,9 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
                                 evaluator,
                                 threadPool,
                                 Objects.requireNonNull(auditLog),
-                                Objects.requireNonNull(userService))
+                                Objects.requireNonNull(userService),
+                                sks,
+                                sslCertReloadEnabled)
                 );
                 log.debug("Added {} rest handler(s)", handlers.size());
             }

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java
@@ -75,9 +75,9 @@ public abstract class AbstractApiAction extends BaseRestHandler {
 	final ThreadPool threadPool;
 	protected String securityIndexName;
 	private final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator;
+	protected final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator;
 	protected final AuditLog auditLog;
 	protected final Settings settings;
-	private AdminDNs adminDNs;
 
 	protected AbstractApiAction(final Settings settings, final Path configPath, final RestController controller,
                                 final Client client, final AdminDNs adminDNs, final ConfigurationRepository cl,
@@ -88,12 +88,13 @@ protected AbstractApiAction(final Settings settings, final Path configPath, fina
 		this.securityIndexName = settings.get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
 				ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX);
 
-		this.adminDNs = adminDNs;
 		this.cl = cl;
 		this.cs = cs;
 		this.threadPool = threadPool;
 		this.restApiPrivilegesEvaluator = new RestApiPrivilegesEvaluator(settings, adminDNs, evaluator,
 				principalExtractor, configPath, threadPool);
+		this.restApiAdminPrivilegesEvaluator =
+				new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), evaluator, adminDNs);
 		this.auditLog = auditLog;
 	}
 
@@ -195,7 +196,12 @@ protected void handlePut(final RestChannel channel, final RestRequest request, f
 		}
 
 		boolean existed = existingConfiguration.exists(name);
-		existingConfiguration.putCObject(name, DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass()));
+		final Object newContent = DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass());
+		if (!hasPermissionsToCreate(existingConfiguration, newContent, getResourceName())) {
+			forbidden(channel, "No permissions");
+			return;
+		}
+		existingConfiguration.putCObject(name, newContent);
 
 		AbstractApiAction.saveAndUpdateConfigs(this.securityIndexName, client, getConfigName(), existingConfiguration, new OnSucessActionListener<IndexResponse>(channel) {
 
@@ -216,6 +222,12 @@ protected void handlePost(final RestChannel channel, final RestRequest request,
 		notImplemented(channel, Method.POST);
 	}
 
+	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+											 final Object content,
+											 final String resourceName) throws IOException {
+		return false;
+	}
+
 	protected void handleGet(final RestChannel channel, RestRequest request, Client client, final JsonNode content)
 			throws IOException{
 
@@ -445,7 +457,6 @@ protected static XContentBuilder convertToJson(RestChannel channel, ToXContent t
 	}
 
 	protected void response(RestChannel channel, RestStatus status, String message) {
-
 		try {
 			final XContentBuilder builder = channel.newBuilder();
 			builder.startObject();
@@ -556,8 +567,7 @@ public String getName() {
 	protected abstract Endpoint getEndpoint();
 
 	protected boolean isSuperAdmin() {
-		User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
-		return adminDNs.isAdmin(user);
+		return restApiAdminPrivilegesEvaluator.isCurrentUserRestApiAdminFor(getEndpoint());
 	}
 
 	/**

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java
@@ -83,6 +83,13 @@ public AccountApiAction(Settings settings,
         this.threadContext = threadPool.getThreadContext();
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @Override
     public List<Route> routes() {
         return routes;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java
@@ -118,12 +118,35 @@ protected void handlePut(RestChannel channel, RestRequest request, Client client
 
 		// Prevent the case where action group references to itself in the allowed_actions.
 		final SecurityDynamicConfiguration<?> existingActionGroupsConfig = load(getConfigName(), false);
-		existingActionGroupsConfig.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass()));
+		final Object actionGroup = DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass());
+		existingActionGroupsConfig.putCObject(name, actionGroup);
 		if (hasActionGroupSelfReference(existingActionGroupsConfig, name)) {
 			badRequestResponse(channel, name + " cannot be an allowed_action of itself");
 			return;
 		}
-
+		// prevent creation of groups for REST admin api
+		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(actionGroup)) {
+			forbidden(channel, "Not allowed");
+			return;
+		}
 		super.handlePut(channel, request, client, content);
 	}
+
+	@Override
+	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfiguration,
+											 final Object content,
+											 final String resourceName) throws IOException {
+		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(content)) {
+			return false;
+		}
+		return true;
+	}
+
+	@Override
+	protected boolean isReadOnly(SecurityDynamicConfiguration<?> existingConfiguration, String name) {
+		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(existingConfiguration.getCEntry(name))) {
+			return true;
+		}
+		return super.isReadOnly(existingConfiguration, name);
+	}
 }
diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java
@@ -98,6 +98,13 @@ public AllowlistApiAction(final Settings settings, final Path configPath, final
         super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @Override
     protected void handleApiRequest(final RestChannel channel, final RestRequest request, final Client client) throws IOException {
         if (!isSuperAdmin()) {

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java
@@ -165,6 +165,13 @@ public AuditApiAction(final Settings settings,
         }
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @Override
     public List<Route> routes() {
         return routes;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java
@@ -34,6 +34,7 @@
 import org.opensearch.security.dlic.rest.validation.NoOpValidator;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.impl.CType;
+import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -53,6 +54,13 @@ public AuthTokenProcessorAction(final Settings settings, final Path configPath,
 				auditLog);
 	}
 
+	@Override
+	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+											 final Object content,
+											 final String resourceName) {
+		return true;
+	}
+
 	@Override
 	public List<Route> routes() {
 		return routes;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java b/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java
@@ -28,5 +28,6 @@ public enum Endpoint {
     VALIDATE,
     WHITELIST,
     ALLOWLIST,
-    NODESDN;
+    NODESDN,
+    SSL;
 }
diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java
@@ -38,6 +38,7 @@
 import org.opensearch.security.dlic.rest.validation.NoOpValidator;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.impl.CType;
+import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -58,6 +59,13 @@ public FlushCacheApiAction(final Settings settings, final Path configPath, final
 		super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
 	}
 
+	@Override
+	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+											 final Object content,
+											 final String resourceName) {
+		return true;
+	}
+
 	@Override
 	public List<Route> routes() {
 		return routes;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java
@@ -84,6 +84,13 @@ public InternalUsersApiAction(final Settings settings, final Path configPath, fi
         this.userService = userService;
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @Override
     public List<Route> routes() {
         return routes;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java
@@ -94,6 +94,13 @@ protected Endpoint getEndpoint() {
         return Endpoint.MIGRATE;
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @SuppressWarnings("unchecked")
     @Override
     protected void handlePost(RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java
@@ -77,6 +77,13 @@ public NodesDnApiAction(final Settings settings, final Path configPath, final Re
         this.staticNodesDnFromEsYml = settings.getAsList(ConfigConstants.SECURITY_NODES_DN, Collections.emptyList());
     }
 
+    @Override
+    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
+                                             final Object content,
+                                             final String resourceName) {
+        return true;
+    }
+
     @Override
     public List<Route> routes() {
         if (settings.getAsBoolean(ConfigConstants.SECURITY_NODES_DN_DYNAMIC_CONFIG_ENABLED, false)) {

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java
@@ -55,9 +55,9 @@ public abstract class PatchableResourceApiAction extends AbstractApiAction {
     protected final Logger log = LogManager.getLogger(this.getClass());
 
     public PatchableResourceApiAction(Settings settings, Path configPath, RestController controller, Client client,
-                                      AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
-                                      PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
-                                      AuditLog auditLog) {
+                                         AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
+                                         PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
+                                         AuditLog auditLog) {
         super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
                 auditLog);
     }
@@ -146,7 +146,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
 
         if (!validator.validate()) {
             request.params().clear();
-                badRequestResponse(channel, validator);
+            badRequestResponse(channel, validator);
             return;
         }
 
@@ -156,7 +156,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
                                    , existingConfiguration.getVersion(), existingConfiguration.getSeqNo(), existingConfiguration.getPrimaryTerm());
 
         if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) {
-            if(hasActionGroupSelfReference(mdc, name)) {
+            if (hasActionGroupSelfReference(mdc, name)) {
                 badRequestResponse(channel, name + " cannot be an allowed_action of itself");
                 return;
             }
@@ -188,7 +188,6 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
         for (String resourceName : existingConfiguration.getCEntries().keySet()) {
             JsonNode oldResource = existingAsObjectNode.get(resourceName);
             JsonNode patchedResource = patchedAsJsonNode.get(resourceName);
-
             if (oldResource != null && !oldResource.equals(patchedResource) && !isWriteable(channel, existingConfiguration, resourceName)) {
                 return;
             }
@@ -206,7 +205,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
             if(originalValidator != null) {
                 if (!originalValidator.validate()) {
                     request.params().clear();
-                        badRequestResponse(channel, originalValidator);
+                    badRequestResponse(channel, originalValidator);
                     return;
                 }
             }
@@ -222,7 +221,13 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
 
                 if (!validator.validate()) {
                     request.params().clear();
-                        badRequestResponse(channel, validator);
+                    badRequestResponse(channel, validator);
+                    return;
+                }
+                final Object newContent = DefaultObjectMapper.readTree(patchedResource, existingConfiguration.getImplementingClass());
+                if (!hasPermissionsToCreate(existingConfiguration, newContent, resourceName)) {
+                    request.params().clear();
+                    forbidden(channel, "No permissions");
                     return;
                 }
             }