Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport] Add more context to index access denied errors #66878

Merged
merged 5 commits into from
Dec 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

Expand Down Expand Up @@ -103,6 +104,17 @@ public static <T> T get(Iterable<T> iterable, int position) {
}
}

public static <T> int indexOf(Iterable<T> iterable, Predicate<T> predicate) {
int i = 0;
for (T element : iterable) {
if (predicate.test(element)) {
return i;
}
i++;
}
return -1;
}

public static long size(Iterable<?> iterable) {
return StreamSupport.stream(iterable.spliterator(), true).count();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.hamcrest.Matchers.is;
import static org.hamcrest.object.HasToString.hasToString;

public class IterablesTests extends ESTestCase {
Expand Down Expand Up @@ -86,6 +89,19 @@ public void testFlatten() {
assertEquals(1, count);
}

public void testIndexOf() {
final List<String> list = Stream.generate(() -> randomAlphaOfLengthBetween(3, 9))
.limit(randomIntBetween(10, 30))
.distinct()
.collect(Collectors.toList());
for (int i = 0; i < list.size(); i++) {
final String val = list.get(i);
assertThat(Iterables.indexOf(list, val::equals), is(i));
}
assertThat(Iterables.indexOf(list, s -> false), is(-1));
assertThat(Iterables.indexOf(list, s -> true), is(0));
}

private void test(Iterable<String> iterable) {
try {
Iterables.get(iterable, -1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRequest;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse;
Expand Down Expand Up @@ -292,6 +294,14 @@ public boolean isAuditable() {
return auditable;
}

/**
* Returns additional context about an authorization failure, if {@link #isGranted()} is false.
*/
@Nullable
public String getFailureContext() {
return null;
}

/**
* Returns a new authorization result that is granted and auditable
*/
Expand Down Expand Up @@ -321,6 +331,19 @@ public IndexAuthorizationResult(boolean auditable, IndicesAccessControl indicesA
this.indicesAccessControl = indicesAccessControl;
}

@Override
public String getFailureContext() {
if (isGranted()) {
return null;
} else {
return getFailureDescription(indicesAccessControl.getDeniedIndices());
}
}

public static String getFailureDescription(Collection<?> deniedIndices) {
return "on indices [" + Strings.collectionToCommaDelimitedString(deniedIndices) + "]";
}

public IndicesAccessControl getIndicesAccessControl() {
return indicesAccessControl;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import org.elasticsearch.xpack.core.security.authz.permission.DocumentPermissions;
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Encapsulates the field and document permissions per concrete index based on the current request.
Expand Down Expand Up @@ -51,6 +53,13 @@ public boolean isGranted() {
return granted;
}

public Collection<?> getDeniedIndices() {
return Collections.unmodifiableSet(this.indexPermissions.entrySet().stream()
.filter(e -> e.getValue().granted == false)
.map(Map.Entry::getKey)
.collect(Collectors.toSet()));
}

/**
* Encapsulates the field and document permissions for an index.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,36 +14,39 @@
import org.elasticsearch.action.admin.indices.close.CloseIndexAction;
import org.elasticsearch.action.admin.indices.create.AutoCreateAction;
import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction;
import org.elasticsearch.xpack.core.action.CreateDataStreamAction;
import org.elasticsearch.xpack.core.action.DeleteDataStreamAction;
import org.elasticsearch.xpack.core.action.GetDataStreamAction;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction;
import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsAction;
import org.elasticsearch.action.admin.indices.exists.types.TypesExistsAction;
import org.elasticsearch.action.admin.indices.get.GetIndexAction;
import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsAction;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsAction;
import org.elasticsearch.action.admin.indices.mapping.put.AutoPutMappingAction;
import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction;
import org.elasticsearch.action.admin.indices.settings.get.GetSettingsAction;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryAction;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.xpack.core.action.CreateDataStreamAction;
import org.elasticsearch.xpack.core.action.DeleteDataStreamAction;
import org.elasticsearch.xpack.core.action.GetDataStreamAction;
import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction;
import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;
import org.elasticsearch.xpack.core.ccr.action.UnfollowAction;
import org.elasticsearch.xpack.core.ilm.action.ExplainLifecycleAction;
import org.elasticsearch.xpack.core.security.support.Automatons;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.elasticsearch.common.collect.Map.ofEntries;
import static org.elasticsearch.common.collect.Map.entry;
import static org.elasticsearch.xpack.core.security.support.Automatons.patterns;
import static org.elasticsearch.xpack.core.security.support.Automatons.unionAndMinimize;

Expand Down Expand Up @@ -99,27 +102,26 @@ public final class IndexPrivilege extends Privilege {
public static final IndexPrivilege MAINTENANCE = new IndexPrivilege("maintenance", MAINTENANCE_AUTOMATON);
public static final IndexPrivilege AUTO_CONFIGURE = new IndexPrivilege("auto_configure", AUTO_CONFIGURE_AUTOMATON);

private static final Map<String, IndexPrivilege> VALUES = MapBuilder.<String, IndexPrivilege>newMapBuilder()
.put("none", NONE)
.put("all", ALL)
.put("manage", MANAGE)
.put("create_index", CREATE_INDEX)
.put("monitor", MONITOR)
.put("read", READ)
.put("index", INDEX)
.put("delete", DELETE)
.put("write", WRITE)
.put("create", CREATE)
.put("create_doc", CREATE_DOC)
.put("delete_index", DELETE_INDEX)
.put("view_index_metadata", VIEW_METADATA)
.put("read_cross_cluster", READ_CROSS_CLUSTER)
.put("manage_follow_index", MANAGE_FOLLOW_INDEX)
.put("manage_leader_index", MANAGE_LEADER_INDEX)
.put("manage_ilm", MANAGE_ILM)
.put("maintenance", MAINTENANCE)
.put("auto_configure", AUTO_CONFIGURE)
.immutableMap();
private static final Map<String, IndexPrivilege> VALUES = sortByAccessLevel(ofEntries(
entry("none", NONE),
entry("all", ALL),
entry("manage", MANAGE),
entry("create_index", CREATE_INDEX),
entry("monitor", MONITOR),
entry("read", READ),
entry("index", INDEX),
entry("delete", DELETE),
entry("write", WRITE),
entry("create", CREATE),
entry("create_doc", CREATE_DOC),
entry("delete_index", DELETE_INDEX),
entry("view_index_metadata", VIEW_METADATA),
entry("read_cross_cluster", READ_CROSS_CLUSTER),
entry("manage_follow_index", MANAGE_FOLLOW_INDEX),
entry("manage_leader_index", MANAGE_LEADER_INDEX),
entry("manage_ilm", MANAGE_ILM),
entry("maintenance", MAINTENANCE),
entry("auto_configure", AUTO_CONFIGURE)));

public static final Predicate<String> ACTION_MATCHER = ALL.predicate();
public static final Predicate<String> CREATE_INDEX_MATCHER = CREATE_INDEX.predicate();
Expand Down Expand Up @@ -157,7 +159,7 @@ private static IndexPrivilege resolve(Set<String> name) {
if (ACTION_MATCHER.test(part)) {
actions.add(actionToPattern(part));
} else {
IndexPrivilege indexPrivilege = VALUES.get(part);
IndexPrivilege indexPrivilege = part == null ? null : VALUES.get(part);
if (indexPrivilege != null && size == 1) {
return indexPrivilege;
} else if (indexPrivilege != null) {
Expand Down Expand Up @@ -187,4 +189,16 @@ public static Set<String> names() {
return Collections.unmodifiableSet(VALUES.keySet());
}

/**
* Returns the names of privileges that grant the specified action.
* @return A collection of names, ordered (to the extent possible) from least privileged (e.g. {@link #CREATE_DOC})
* to most privileged (e.g. {@link #ALL})
* @see Privilege#sortByAccessLevel
*/
public static Collection<String> findPrivilegesThatGrant(String action) {
return Collections.unmodifiableList(VALUES.entrySet().stream()
.filter(e -> e.getValue().predicate.test(action))
.map(e -> e.getKey())
.collect(Collectors.toList()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@
package org.elasticsearch.xpack.core.security.authz.privilege;

import org.apache.lucene.util.automaton.Automaton;
import org.apache.lucene.util.automaton.Operations;
import org.elasticsearch.xpack.core.security.support.Automatons;

import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.function.Predicate;

import static org.elasticsearch.xpack.core.security.support.Automatons.patterns;
Expand Down Expand Up @@ -74,4 +80,21 @@ public String toString() {
public Automaton getAutomaton() {
return automaton;
}

/**
* Sorts the map of privileges from least-privilege to most-privilege
*/
static <T extends Privilege> SortedMap<String, T> sortByAccessLevel(Map<String, T> privileges) {
// How many other privileges is this privilege a subset of. Those with a higher count are considered to be a lower privilege
final Map<String, Long> subsetCount = new HashMap<>(privileges.size());
privileges.forEach((name, priv) -> subsetCount.put(name,
privileges.values().stream().filter(p2 -> p2 != priv && Operations.subsetOf(priv.automaton, p2.automaton)).count())
);

final Comparator<String> compare = Comparator.<String>comparingLong(key -> subsetCount.getOrDefault(key, 0L)).reversed()
.thenComparing(Comparator.naturalOrder());
final TreeMap<String, T> tree = new TreeMap<>(compare);
tree.putAll(privileges);
return Collections.unmodifiableSortedMap(tree);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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.core.security.authz.privilege;

import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
import org.elasticsearch.action.admin.indices.shrink.ShrinkAction;
import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
import org.elasticsearch.action.delete.DeleteAction;
import org.elasticsearch.action.index.IndexAction;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.action.update.UpdateAction;
import org.elasticsearch.common.util.iterable.Iterables;
import org.elasticsearch.test.ESTestCase;

import org.elasticsearch.common.collect.List;
import java.util.Set;

import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.findPrivilegesThatGrant;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThan;

public class IndexPrivilegeTests extends ESTestCase {

/**
* The {@link IndexPrivilege#values()} map is sorted so that privilege names that offer the _least_ access come before those that
* offer _more_ access. There is no guarantee of ordering between privileges that offer non-overlapping privileges.
*/
public void testOrderingOfPrivilegeNames() throws Exception {
final Set<String> names = IndexPrivilege.values().keySet();
final int all = Iterables.indexOf(names, "all"::equals);
final int manage = Iterables.indexOf(names, "manage"::equals);
final int monitor = Iterables.indexOf(names, "monitor"::equals);
final int read = Iterables.indexOf(names, "read"::equals);
final int write = Iterables.indexOf(names, "write"::equals);
final int index = Iterables.indexOf(names, "index"::equals);
final int create_doc = Iterables.indexOf(names, "create_doc"::equals);
final int delete = Iterables.indexOf(names, "delete"::equals);

assertThat(read, lessThan(all));
assertThat(manage, lessThan(all));
assertThat(monitor, lessThan(manage));
assertThat(write, lessThan(all));
assertThat(index, lessThan(write));
assertThat(create_doc, lessThan(index));
assertThat(delete, lessThan(write));
}

public void testFindPrivilegesThatGrant() {
assertThat(findPrivilegesThatGrant(SearchAction.NAME), equalTo(List.of("read", "all")));
assertThat(findPrivilegesThatGrant(IndexAction.NAME), equalTo(List.of("create_doc", "create", "index", "write", "all")));
assertThat(findPrivilegesThatGrant(UpdateAction.NAME), equalTo(List.of("index", "write", "all")));
assertThat(findPrivilegesThatGrant(DeleteAction.NAME), equalTo(List.of("delete", "write", "all")));
assertThat(findPrivilegesThatGrant(IndicesStatsAction.NAME), equalTo(List.of("monitor", "manage", "all")));
assertThat(findPrivilegesThatGrant(RefreshAction.NAME), equalTo(List.of("maintenance", "manage", "all")));
assertThat(findPrivilegesThatGrant(ShrinkAction.NAME), equalTo(List.of("manage", "all")));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ public void testCanManageIndexWithNoPermissions() throws Exception {
assertThat(indexExplain.get("failed_step"), equalTo("wait-for-shard-history-leases"));
Map<String, String> stepInfo = (Map<String, String>) indexExplain.get("step_info");
assertThat(stepInfo.get("type"), equalTo("security_exception"));
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized for user [test_ilm]"));
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized for user [test_ilm]" +
" on indices [not-ilm], this action is granted by the privileges [monitor,manage,all]"));
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ public void testLookbackWithoutPermissions() throws Exception {
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data]\""));
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data] on indices [network-data],"));
}

public void testLookbackWithPipelineBucketAgg() throws Exception {
Expand Down Expand Up @@ -966,7 +966,8 @@ public void testLookbackWithoutPermissionsAndRollup() throws Exception {
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
"action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data]\""));
"action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data] " +
"on indices [airline-data-aggs-rollup], "));
}

public void testLookbackWithSingleBucketAgg() throws Exception {
Expand Down
Loading