Skip to content

Commit

Permalink
Validate read priv of enrich source indices (#43595)
Browse files Browse the repository at this point in the history
This commit adds permissions validation on the indices provided in the
enrich policy. These indices should be validated at store time so as not
to have cryptic error messages in the event the user does not have
permissions to access said indices.
  • Loading branch information
hub-cap committed Jul 10, 2019
1 parent adc06ff commit d2c3f4b
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,10 @@ public Builder indices(String... indices) {
return this;
}

public Builder indices(Collection<String> indices) {
return indices(indices.toArray(new String[indices.size()]));
}

public Builder privileges(String... privileges) {
indicesPrivileges.privileges = privileges;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void testFailure() {
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"my-source-index\"], \"enrich_key\": \"host\", " +
"\"enrich_values\": [\"globalRank\", \"tldRank\", \"tld\"]}");
ResponseException exc = expectThrows(ResponseException.class, () -> assertOK(client().performRequest(putPolicyRequest)));
ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest));
assertTrue(exc.getMessage().contains("action [cluster:admin/xpack/enrich/put] is unauthorized for user [test_enrich_no_privs]"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
*/
package org.elasticsearch.xpack.enrich;

import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.enrich.CommonEnrichRestTestCase;

import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
import static org.hamcrest.CoreMatchers.containsString;

public class EnrichSecurityIT extends CommonEnrichRestTestCase {

Expand All @@ -29,4 +32,15 @@ protected Settings restAdminSettings() {
.put(ThreadContext.PREFIX + ".Authorization", token)
.build();
}

public void testInsufficientPermissionsOnNonExistentIndex() {
// This test is here because it requires a valid user that has permission to execute policy PUTs but should fail if the user
// does not have access to read the backing indices used to enrich the data.
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"some-other-index\"], \"enrich_key\": \"host\", " +
"\"enrich_values\": [\"globalRank\", \"tldRank\", \"tld\"]}");
ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest));
assertThat(exc.getMessage(),
containsString("unable to store policy because no indices match with the specified index patterns [some-other-index]"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.IngestPlugin;
import org.elasticsearch.plugins.Plugin;
Expand All @@ -32,6 +33,7 @@
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.enrich.action.DeleteEnrichPolicyAction;
import org.elasticsearch.xpack.core.enrich.action.ExecuteEnrichPolicyAction;
import org.elasticsearch.xpack.core.enrich.action.GetEnrichPolicyAction;
Expand Down Expand Up @@ -94,6 +96,8 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
return Collections.singletonMap(EnrichProcessorFactory.TYPE, factory);
}

protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); }

public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
if (enabled == false) {
return Collections.emptyList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,48 @@
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.action.support.master.TransportMasterNodeAction;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.enrich.action.PutEnrichPolicyAction;
import org.elasticsearch.xpack.core.security.SecurityContext;
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction;
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest;
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.support.Exceptions;
import org.elasticsearch.xpack.enrich.EnrichStore;

import java.io.IOException;

public class TransportPutEnrichPolicyAction extends TransportMasterNodeAction<PutEnrichPolicyAction.Request, AcknowledgedResponse> {

private final XPackLicenseState licenseState;
private final SecurityContext securityContext;
private final Client client;

@Inject
public TransportPutEnrichPolicyAction(TransportService transportService,
ClusterService clusterService,
ThreadPool threadPool,
ActionFilters actionFilters,
public TransportPutEnrichPolicyAction(Settings settings, TransportService transportService,
ClusterService clusterService, ThreadPool threadPool, Client client,
XPackLicenseState licenseState, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver) {
super(PutEnrichPolicyAction.NAME, transportService, clusterService, threadPool, actionFilters,
PutEnrichPolicyAction.Request::new, indexNameExpressionResolver);
this.licenseState = licenseState;
this.securityContext = XPackSettings.SECURITY_ENABLED.get(settings) ?
new SecurityContext(settings, threadPool.getThreadContext()) : null;
this.client = client;
}

@Override
Expand All @@ -52,6 +70,38 @@ protected AcknowledgedResponse read(StreamInput in) throws IOException {
@Override
protected void masterOperation(PutEnrichPolicyAction.Request request, ClusterState state,
ActionListener<AcknowledgedResponse> listener) {

if (licenseState.isAuthAllowed()) {
RoleDescriptor.IndicesPrivileges privileges = RoleDescriptor.IndicesPrivileges.builder()
.indices(request.getPolicy().getIndices())
.privileges("read")
.build();

String username = securityContext.getUser().principal();

HasPrivilegesRequest privRequest = new HasPrivilegesRequest();
privRequest.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]);
privRequest.username(username);
privRequest.clusterPrivileges(Strings.EMPTY_ARRAY);
privRequest.indexPrivileges(privileges);

ActionListener<HasPrivilegesResponse> wrappedListener = ActionListener.wrap(
r -> {
if (r.isCompleteMatch()) {
putPolicy(request, listener);
} else {
listener.onFailure(Exceptions.authorizationError("unable to store policy because no indices match with the " +
"specified index patterns {}", request.getPolicy().getIndices(), username));
}
},
listener::onFailure);
client.execute(HasPrivilegesAction.INSTANCE, privRequest, wrappedListener);
} else {
putPolicy(request, listener);
}
}

private void putPolicy(PutEnrichPolicyAction.Request request, ActionListener<AcknowledgedResponse> listener ) {
EnrichStore.putPolicy(request.getName(), request.getPolicy(), clusterService, e -> {
if (e == null) {
listener.onResponse(new AcknowledgedResponse(true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class BasicEnrichTests extends ESSingleNodeTestCase {

@Override
protected Collection<Class<? extends Plugin>> getPlugins() {
return Arrays.asList(EnrichPlugin.class, ReindexPlugin.class);
return Arrays.asList(LocalStateEnrich.class, ReindexPlugin.class);
}

public void testIngestDataWithEnrichProcessor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.node.Node;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
import org.elasticsearch.xpack.core.enrich.action.DeleteEnrichPolicyAction;
import org.elasticsearch.xpack.core.enrich.action.ExecuteEnrichPolicyAction;
Expand Down Expand Up @@ -54,14 +55,19 @@ public class EnrichMultiNodeIT extends ESIntegTestCase {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(EnrichPlugin.class, ReindexPlugin.class);
return Arrays.asList(LocalStateEnrich.class, ReindexPlugin.class);
}

@Override
protected Collection<Class<? extends Plugin>> transportClientPlugins() {
return nodePlugins();
}

@Override
protected Settings transportClientSettings() {
return Settings.builder().put(super.transportClientSettings()).put(XPackSettings.SECURITY_ENABLED.getKey(), false).build();
}

public void testEnrichAPIs() {
final int numPolicies = randomIntBetween(2, 4);
internalCluster().startNodes(randomIntBetween(2, 3));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class EnrichPolicyUpdateTests extends ESSingleNodeTestCase {

@Override
protected Collection<Class<? extends Plugin>> getPlugins() {
return Collections.singleton(EnrichPlugin.class);
return Collections.singleton(LocalStateEnrich.class);
}

public void testUpdatePolicyOnly() {
Expand Down Expand Up @@ -56,6 +56,4 @@ public void testUpdatePolicyOnly() {
Pipeline pipelineInstance2 = ingestService.getPipeline("1");
assertThat(pipelineInstance2, sameInstance(pipelineInstance1));
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
*/
package org.elasticsearch.xpack.enrich;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.reindex.ReindexPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
import org.elasticsearch.xpack.core.enrich.action.ListEnrichPolicyAction;
import org.elasticsearch.xpack.core.enrich.action.PutEnrichPolicyAction;
Expand All @@ -29,14 +31,19 @@ public class EnrichRestartIT extends ESIntegTestCase {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(EnrichPlugin.class, ReindexPlugin.class);
return Arrays.asList(LocalStateEnrich.class, ReindexPlugin.class);
}

@Override
protected Collection<Class<? extends Plugin>> transportClientPlugins() {
return nodePlugins();
}

@Override
protected Settings transportClientSettings() {
return Settings.builder().put(super.transportClientSettings()).put(XPackSettings.SECURITY_ENABLED.getKey(), false).build();
}

public void testRestart() throws Exception {
final int numPolicies = randomIntBetween(2, 4);
internalCluster().startNode();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* 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.enrich;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin;

import java.nio.file.Path;

public class LocalStateEnrich extends LocalStateCompositeXPackPlugin {

public LocalStateEnrich(final Settings settings, final Path configPath) throws Exception {
super(settings, configPath);

plugins.add(new EnrichPlugin(settings) {
@Override
protected XPackLicenseState getLicenseState() {
return LocalStateEnrich.this.getLicenseState();
}
});
}
}

0 comments on commit d2c3f4b

Please sign in to comment.