diff --git a/dev-tools/checkstyle_suppressions.xml b/dev-tools/checkstyle_suppressions.xml index 22dfb03276d..589d005ca4b 100644 --- a/dev-tools/checkstyle_suppressions.xml +++ b/dev-tools/checkstyle_suppressions.xml @@ -105,7 +105,7 @@ - + diff --git a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index 9a7ac6aed13..ec441a88a02 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -56,6 +56,7 @@ import org.elasticsearch.xpack.action.TransportXPackInfoAction; import org.elasticsearch.xpack.action.TransportXPackUsageAction; import org.elasticsearch.xpack.action.XPackInfoAction; import org.elasticsearch.xpack.action.XPackUsageAction; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.common.http.HttpClient; import org.elasticsearch.xpack.common.http.HttpRequestTemplate; import org.elasticsearch.xpack.common.http.HttpSettings; @@ -397,6 +398,7 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I List> actions = new ArrayList<>(); actions.add(new ActionHandler<>(XPackInfoAction.INSTANCE, TransportXPackInfoAction.class)); actions.add(new ActionHandler<>(XPackUsageAction.INSTANCE, TransportXPackUsageAction.class)); + actions.add(new ActionHandler<>(XPackDeleteByQueryAction.INSTANCE, XPackDeleteByQueryAction.TransportAction.class)); actions.addAll(licensing.getActions()); actions.addAll(monitoring.getActions()); actions.addAll(security.getActions()); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/MlDeleteByQueryAction.java b/plugin/src/main/java/org/elasticsearch/xpack/common/action/XPackDeleteByQueryAction.java similarity index 63% rename from plugin/src/main/java/org/elasticsearch/xpack/ml/action/MlDeleteByQueryAction.java rename to plugin/src/main/java/org/elasticsearch/xpack/common/action/XPackDeleteByQueryAction.java index 959e43d468b..fc1dbaeac65 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/MlDeleteByQueryAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/common/action/XPackDeleteByQueryAction.java @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -package org.elasticsearch.xpack.ml.action; +package org.elasticsearch.xpack.common.action; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; @@ -19,6 +19,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.client.ParentTaskAssigningClient; @@ -31,24 +32,24 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.ml.job.persistence.JobProvider; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction.XPackDeleteByQueryRequestBuilder; -public class MlDeleteByQueryAction extends Action { +public class XPackDeleteByQueryAction extends Action { - public static final MlDeleteByQueryAction INSTANCE = new MlDeleteByQueryAction(); - // TODO: Ideally we'd use an "internal" action here as we don't want transport client users running it, but unfortunately the internal - // _xpack user is forbidden to run "internal" actions. Putting "internal" at the top level of the action name at least restricts it to - // superusers, and makes clear to anyone who sees the name that it's not for general use. - public static final String NAME = "indices:internal/data/write/mldeletebyquery"; + public static final XPackDeleteByQueryAction INSTANCE = new XPackDeleteByQueryAction(); + // Ideally we'd use an "internal" action here as we don't want transport client users running it + // but unfortunately the _xpack user is forbidden to run "internal" actions as these are really + // intended to be run as the system user + public static final String NAME = "indices:internal/data/write/xpackdeletebyquery"; - private MlDeleteByQueryAction() { + private XPackDeleteByQueryAction() { super(NAME); } @Override - public MlDeleteByQueryRequestBuilder newRequestBuilder(ElasticsearchClient client) { - return new MlDeleteByQueryRequestBuilder(client, this); + public XPackDeleteByQueryRequestBuilder newRequestBuilder(ElasticsearchClient client) { + return new XPackDeleteByQueryRequestBuilder(client, this); } @Override @@ -56,29 +57,29 @@ public class MlDeleteByQueryAction extends Action { + public static class XPackDeleteByQueryRequestBuilder extends + AbstractBulkByScrollRequestBuilder { - private MlDeleteByQueryRequestBuilder(ElasticsearchClient client, - Action action) { + private XPackDeleteByQueryRequestBuilder(ElasticsearchClient client, + Action action) { this(client, action, new SearchRequestBuilder(client, SearchAction.INSTANCE)); } - private MlDeleteByQueryRequestBuilder(ElasticsearchClient client, - Action action, - SearchRequestBuilder search) { + private XPackDeleteByQueryRequestBuilder(ElasticsearchClient client, + Action action, + SearchRequestBuilder search) { super(client, action, search, - new DeleteByQueryRequest(search.setIndicesOptions( - JobProvider.addIgnoreUnavailable(SearchRequest.DEFAULT_INDICES_OPTIONS)).request())); + new DeleteByQueryRequest( + search.setIndicesOptions(addIgnoreUnavailable(SearchRequest.DEFAULT_INDICES_OPTIONS)).request())); } @Override - protected MlDeleteByQueryRequestBuilder self() { + protected XPackDeleteByQueryRequestBuilder self() { return this; } @Override - public MlDeleteByQueryRequestBuilder abortOnVersionConflict(boolean abortOnVersionConflict) { + public XPackDeleteByQueryRequestBuilder abortOnVersionConflict(boolean abortOnVersionConflict) { request.setAbortOnVersionConflict(abortOnVersionConflict); return this; } @@ -93,7 +94,7 @@ public class MlDeleteByQueryAction extends Action listener) { if (request.getSlices() > 1) { - BulkByScrollParallelizationHelper.startSlices(client, taskManager, MlDeleteByQueryAction.INSTANCE, + BulkByScrollParallelizationHelper.startSlices(client, taskManager, XPackDeleteByQueryAction.INSTANCE, clusterService.localNode().getId(), (ParentBulkByScrollTask) task, request, listener); } else { ClusterState state = clusterService.state(); @@ -117,4 +118,11 @@ public class MlDeleteByQueryAction extends Action(UpdatePersistentTaskStatusAction.INSTANCE, UpdatePersistentTaskStatusAction.TransportAction.class), new ActionHandler<>(CompletionPersistentTaskAction.INSTANCE, CompletionPersistentTaskAction.TransportAction.class), new ActionHandler<>(RemovePersistentTaskAction.INSTANCE, RemovePersistentTaskAction.TransportAction.class), - new ActionHandler<>(MlDeleteByQueryAction.INSTANCE, MlDeleteByQueryAction.TransportAction.class), new ActionHandler<>(UpdateProcessAction.INSTANCE, UpdateProcessAction.TransportAction.class), new ActionHandler<>(DeleteExpiredDataAction.INSTANCE, DeleteExpiredDataAction.TransportAction.class) ); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteExpiredDataAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteExpiredDataAction.java index 09eedbded71..b8dd8dbbeb1 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteExpiredDataAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteExpiredDataAction.java @@ -13,7 +13,6 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.client.Client; import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; @@ -29,6 +28,7 @@ import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.ml.job.retention.ExpiredModelSnapshotsRemover; import org.elasticsearch.xpack.ml.job.retention.ExpiredResultsRemover; import org.elasticsearch.xpack.ml.notifications.Auditor; +import org.elasticsearch.xpack.security.InternalClient; import java.io.IOException; import java.util.Objects; @@ -118,13 +118,13 @@ public class DeleteExpiredDataAction extends Action { - private final Client client; + private final InternalClient client; private final ClusterService clusterService; @Inject public TransportAction(Settings settings, ThreadPool threadPool, TransportService transportService, - ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, Client client, - ClusterService clusterService) { + ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, + InternalClient client, ClusterService clusterService) { super(settings, NAME, threadPool, transportService, actionFilters, indexNameExpressionResolver, Request::new); this.client = client; this.clusterService = clusterService; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java index a7174cf0f6e..ae574300c81 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java @@ -29,7 +29,7 @@ import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskId; -import org.elasticsearch.xpack.ml.action.MlDeleteByQueryAction; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.ml.job.config.Job; import org.elasticsearch.xpack.ml.job.process.autodetect.state.CategorizerState; import org.elasticsearch.xpack.ml.job.process.autodetect.state.ModelSnapshot; @@ -98,7 +98,7 @@ public class JobStorageDeletionTask extends Task { searchRequest.indicesOptions(JobProvider.addIgnoreUnavailable(IndicesOptions.lenientExpandOpen())); request.setSlices(5); - client.execute(MlDeleteByQueryAction.INSTANCE, request, dbqHandler); + client.execute(XPackDeleteByQueryAction.INSTANCE, request, dbqHandler); } private void deleteQuantiles(String jobId, Client client, ActionListener finishedHandler) { @@ -132,7 +132,7 @@ public class JobStorageDeletionTask extends Task { searchRequest.indicesOptions(IndicesOptions.lenientExpandOpen()); WildcardQueryBuilder query = new WildcardQueryBuilder(UidFieldMapper.NAME, Uid.createUid(CategorizerState.TYPE, jobId + "#*")); searchRequest.source(new SearchSourceBuilder().query(query)); - client.execute(MlDeleteByQueryAction.INSTANCE, request, new ActionListener() { + client.execute(XPackDeleteByQueryAction.INSTANCE, request, new ActionListener() { @Override public void onResponse(BulkByScrollResponse bulkByScrollResponse) { finishedHandler.onResponse(true); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java index dcf77e69f95..9071980c4a1 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java @@ -16,7 +16,7 @@ import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.xpack.ml.action.MlDeleteByQueryAction; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.ml.job.config.Job; import org.elasticsearch.xpack.ml.job.messages.Messages; import org.elasticsearch.xpack.ml.job.persistence.AnomalyDetectorsIndex; @@ -29,7 +29,6 @@ import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.util.Objects; -import java.util.function.Function; /** * Removes all results that have expired the configured retention time @@ -60,7 +59,7 @@ public class ExpiredResultsRemover extends AbstractExpiredJobDataRemover { LOGGER.info("Removing results of job [{}] that have a timestamp before [{}]", job.getId(), cutoffEpochMs); DeleteByQueryRequest request = createDBQRequest(job, cutoffEpochMs); - client.execute(MlDeleteByQueryAction.INSTANCE, request, new ActionListener() { + client.execute(XPackDeleteByQueryAction.INSTANCE, request, new ActionListener() { @Override public void onResponse(BulkByScrollResponse bulkByScrollResponse) { try { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/InternalClient.java b/plugin/src/main/java/org/elasticsearch/xpack/security/InternalClient.java index f1eb365fa73..db53af79f94 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/InternalClient.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/InternalClient.java @@ -51,7 +51,7 @@ public class InternalClient extends FilterClient { /** * Constructs an InternalClient. - * If {@code cryptoService} is non-null, the client is secure. Otherwise this client is a passthrough. + * If security is enabled the client is secure. Otherwise this client is a passthrough. */ public InternalClient(Settings settings, ThreadPool threadPool, Client in) { super(settings, threadPool, in); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index cb08ec32fc6..4cdbd9da6f9 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportActionProxy; import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.security.SecurityLifecycleService; import org.elasticsearch.xpack.security.action.user.AuthenticateAction; import org.elasticsearch.xpack.security.action.user.ChangePasswordAction; @@ -200,6 +201,12 @@ public class AuthorizationService extends AbstractComponent { throw denial(authentication, action, request); } + // we only want the xpack user to use the xpack delete by query action + if (XPackDeleteByQueryAction.NAME.equals(action) + && XPackUser.is(authentication.getRunAsUser()) == false) { + throw denial(authentication, action, request); + } + // some APIs are indices requests that are not actually associated with indices. For example, // search scroll request, is categorized under the indices context, but doesn't hold indices names // (in this case, the security check on the indices was done on the search request that initialized diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemoverTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemoverTests.java index 805e07d628c..f6682fdceaf 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemoverTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemoverTests.java @@ -15,7 +15,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.mock.orig.Mockito; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.ml.MlMetadata; -import org.elasticsearch.xpack.ml.action.MlDeleteByQueryAction; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.ml.job.config.Job; import org.elasticsearch.xpack.ml.job.config.JobTests; import org.elasticsearch.xpack.ml.job.persistence.AnomalyDetectorsIndex; @@ -64,7 +64,7 @@ public class ExpiredResultsRemoverTests extends ESTestCase { listener.onResponse(null); return null; } - }).when(client).execute(same(MlDeleteByQueryAction.INSTANCE), any(), any()); + }).when(client).execute(same(XPackDeleteByQueryAction.INSTANCE), any(), any()); onFinish = mock(Runnable.class); } @@ -149,7 +149,7 @@ public class ExpiredResultsRemoverTests extends ESTestCase { } return null; } - }).when(client).execute(same(MlDeleteByQueryAction.INSTANCE), any(), any()); + }).when(client).execute(same(XPackDeleteByQueryAction.INSTANCE), any(), any()); } private void givenJobs(List jobs) { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index c42a64e6a75..e25a26e15bb 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -40,6 +40,7 @@ import org.elasticsearch.action.admin.indices.upgrade.get.UpgradeStatusAction; import org.elasticsearch.action.admin.indices.upgrade.get.UpgradeStatusRequest; import org.elasticsearch.action.bulk.BulkAction; import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.bulk.byscroll.DeleteByQueryRequest; import org.elasticsearch.action.delete.DeleteAction; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.get.GetAction; @@ -79,6 +80,7 @@ import org.elasticsearch.license.GetLicenseAction; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.xpack.common.action.XPackDeleteByQueryAction; import org.elasticsearch.xpack.security.SecurityLifecycleService; import org.elasticsearch.xpack.security.action.user.AuthenticateAction; import org.elasticsearch.xpack.security.action.user.AuthenticateRequest; @@ -692,6 +694,30 @@ public class AuthorizationServiceTests extends ESTestCase { assertThat(request.indices(), arrayContaining(".security")); } + public void testOnlyXPackUserCanExecuteXPackDBQAction() { + final User superuser = new User("custom_admin", ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()); + roleMap.put(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR); + ClusterState state = mock(ClusterState.class); + when(clusterService.state()).thenReturn(state); + when(state.metaData()).thenReturn(MetaData.builder() + .put(new IndexMetaData.Builder(SecurityLifecycleService.SECURITY_INDEX_NAME) + .settings(Settings.builder().put("index.version.created", Version.CURRENT).build()) + .numberOfShards(1).numberOfReplicas(0).build(), true) + .build()); + + String action = XPackDeleteByQueryAction.NAME; + DeleteByQueryRequest request = new DeleteByQueryRequest(new SearchRequest("_all")); + authorize(createAuthentication(XPackUser.INSTANCE), action, request); + verify(auditTrail).accessGranted(XPackUser.INSTANCE, action, request); + assertThat(request.indices(), arrayContaining(".security")); + + DeleteByQueryRequest request1 = new DeleteByQueryRequest(new SearchRequest("_all")); + assertThrowsAuthorizationException( + () -> authorize(createAuthentication(superuser), action, request1), + action, superuser.principal()); + verify(auditTrail).accessDenied(superuser, action, request1); + } + public void testAnonymousRolesAreAppliedToOtherUsers() { TransportRequest request = new ClusterHealthRequest(); Settings settings = Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "anonymous_user_role").build(); diff --git a/plugin/src/test/resources/org/elasticsearch/transport/actions b/plugin/src/test/resources/org/elasticsearch/transport/actions index 5cae99a91f7..71f4330796e 100644 --- a/plugin/src/test/resources/org/elasticsearch/transport/actions +++ b/plugin/src/test/resources/org/elasticsearch/transport/actions @@ -135,7 +135,7 @@ cluster:admin/xpack/ml/datafeeds/stop cluster:admin/xpack/ml/datafeeds/start cluster:admin/xpack/ml/job/open cluster:admin/xpack/ml/job/update -indices:internal/data/write/mldeletebyquery +indices:internal/data/write/xpackdeletebyquery cluster:internal/xpack/ml/job/update/process cluster:admin/xpack/ml/delete_expired_data cluster:admin/persistent/create