Skip to content

Commit

Permalink
Mirror privileges over data streams to their backing indices (#58381)
Browse files Browse the repository at this point in the history
  • Loading branch information
danhermann authored Jul 3, 2020
1 parent 07fda01 commit 50ed781
Show file tree
Hide file tree
Showing 15 changed files with 631 additions and 51 deletions.
4 changes: 2 additions & 2 deletions .idea/runConfigurations/Debug_Elasticsearch.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,6 @@
indices.delete:
index: logs-foobar

- do:
indices.create:
index: logs-foobarbaz

- do:
catch: bad_request
indices.close:
index: logs-*

- do:
indices.delete_data_stream:
name: logs-foobar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ public interface IndicesRequest {
*/
IndicesOptions indicesOptions();

/**
* Determines whether the request should be applied to data streams. When {@code false}, none of the names or
* wildcard expressions in {@link #indices} should be applied to or expanded to any data streams. All layers
* involved in the request's fulfillment including security, name resolution, etc., should respect this flag.
*/
default boolean includeDataStreams() {
return false;
}

interface Replaceable extends IndicesRequest {
/**
* Sets the indices that the action relates to.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ public ClusterSearchShardsRequest indicesOptions(IndicesOptions indicesOptions)
return this;
}

@Override
public boolean includeDataStreams() {
return true;
}

/**
* A comma separated list of routing values to control the shards the search will be executed on.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,9 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
flags.writeTo(out);
}

@Override
public boolean includeDataStreams() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ public IndicesOptions indicesOptions() {
return indicesOptions;
}

@Override
public boolean includeDataStreams() {
return true;
}

public boolean includeUnmapped() {
return includeUnmapped;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,11 @@ public SearchRequest indicesOptions(IndicesOptions indicesOptions) {
return this;
}

@Override
public boolean includeDataStreams() {
return true;
}

/**
* Returns whether network round-trips should be minimized when executing cross-cluster search requests.
* Defaults to <code>true</code>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public Automaton allowedActionsMatcher(String index) {
* Authorizes the provided action against the provided indices, given the current cluster metadata
*/
public Map<String, IndicesAccessControl.IndexAccessControl> authorize(String action, Set<String> requestedIndicesOrAliases,
Map<String, IndexAbstraction> allAliasesAndIndices,
Map<String, IndexAbstraction> lookup,
FieldPermissionsCache fieldPermissionsCache) {
// now... every index that is associated with the request, must be granted
// by at least one indices permission group
Expand All @@ -205,15 +205,20 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(String act
for (String indexOrAlias : requestedIndicesOrAliases) {
boolean granted = false;
Set<String> concreteIndices = new HashSet<>();
IndexAbstraction indexAbstraction = allAliasesAndIndices.get(indexOrAlias);
IndexAbstraction indexAbstraction = lookup.get(indexOrAlias);
if (indexAbstraction != null) {
for (IndexMetadata indexMetadata : indexAbstraction.getIndices()) {
concreteIndices.add(indexMetadata.getIndex().getName());
}
}

for (Group group : groups) {
if (group.check(action, indexOrAlias)) {
// check for privilege granted directly on the requested index/alias
if (group.check(action, indexOrAlias) ||
// check for privilege granted on parent data stream if a backing index
(indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX &&
indexAbstraction.getParentDataStream() != null &&
group.check(action, indexAbstraction.getParentDataStream().getName()))) {
granted = true;
for (String index : concreteIndices) {
Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, Metadata
if (IndexNameExpressionResolver.isAllIndices(indicesList(indicesRequest.indices()))) {
if (replaceWildcards) {
for (String authorizedIndex : authorizedIndices) {
if (isIndexVisible("*", authorizedIndex, indicesOptions, metadata)) {
if (isIndexVisible("*", authorizedIndex, indicesOptions, metadata, indicesRequest.includeDataStreams())) {
resolvedIndicesBuilder.addLocal(authorizedIndex);
}
}
Expand All @@ -145,7 +145,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, Metadata
split = new ResolvedIndices(Arrays.asList(indicesRequest.indices()), Collections.emptyList());
}
List<String> replaced = replaceWildcardsWithAuthorizedIndices(split.getLocal(), indicesOptions, metadata,
authorizedIndices, replaceWildcards);
authorizedIndices, replaceWildcards, indicesRequest.includeDataStreams());
if (indicesOptions.ignoreUnavailable()) {
//out of all the explicit names (expanded from wildcards and original ones that were left untouched)
//remove all the ones that the current user is not authorized for and ignore them
Expand Down Expand Up @@ -337,7 +337,8 @@ private boolean containsWildcards(IndicesRequest indicesRequest) {

//TODO Investigate reusing code from vanilla es to resolve index names and wildcards
private List<String> replaceWildcardsWithAuthorizedIndices(Iterable<String> indices, IndicesOptions indicesOptions, Metadata metadata,
List<String> authorizedIndices, boolean replaceWildcards) {
List<String> authorizedIndices, boolean replaceWildcards,
boolean includeDataStreams) {
//the order matters when it comes to exclusions
List<String> finalIndices = new ArrayList<>();
boolean wildcardSeen = false;
Expand All @@ -359,7 +360,7 @@ private List<String> replaceWildcardsWithAuthorizedIndices(Iterable<String> indi
// continue
aliasOrIndex = dateMathName;
} else if (authorizedIndices.contains(dateMathName) &&
isIndexVisible(aliasOrIndex, dateMathName, indicesOptions, metadata, true)) {
isIndexVisible(aliasOrIndex, dateMathName, indicesOptions, metadata, includeDataStreams, true)) {
if (minus) {
finalIndices.remove(dateMathName);
} else {
Expand All @@ -377,7 +378,7 @@ private List<String> replaceWildcardsWithAuthorizedIndices(Iterable<String> indi
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : authorizedIndices) {
if (Regex.simpleMatch(aliasOrIndex, authorizedIndex) &&
isIndexVisible(aliasOrIndex, authorizedIndex, indicesOptions, metadata)) {
isIndexVisible(aliasOrIndex, authorizedIndex, indicesOptions, metadata, includeDataStreams)) {
resolvedIndices.add(authorizedIndex);
}
}
Expand Down Expand Up @@ -412,13 +413,17 @@ private List<String> replaceWildcardsWithAuthorizedIndices(Iterable<String> indi
return finalIndices;
}

private static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata) {
return isIndexVisible(expression, index, indicesOptions, metadata, false);
private static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata,
boolean includeDataStreams) {
return isIndexVisible(expression, index, indicesOptions, metadata, includeDataStreams, false);
}

private static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata,
boolean dateMathExpression) {
boolean includeDataStreams, boolean dateMathExpression) {
IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(index);
if (indexAbstraction == null) {
throw new IllegalStateException("could not resolve index abstraction [" + index + "]");
}
final boolean isHidden = indexAbstraction.isHidden();
if (indexAbstraction.getType() == IndexAbstraction.Type.ALIAS) {
//it's an alias, ignore expandWildcardsOpen and expandWildcardsClosed.
Expand All @@ -433,12 +438,7 @@ private static boolean isIndexVisible(String expression, String index, IndicesOp
}
}
if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
// If indicesOptions.includeDataStreams() returns false then we fail later in IndexNameExpressionResolver.
if (isHidden == false || indicesOptions.expandWildcardsHidden()) {
return true;
} else {
return false;
}
return includeDataStreams;
}
assert indexAbstraction.getIndices().size() == 1 : "concrete index must point to a single index";
IndexMetadata indexMetadata = indexAbstraction.getIndices().get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.elasticsearch.common.Strings.arrayToCommaDelimitedString;
import static org.elasticsearch.xpack.security.action.user.TransportHasPrivilegesAction.getApplicationNames;
Expand Down Expand Up @@ -343,7 +344,7 @@ public void loadAuthorizedIndices(RequestInfo requestInfo, AuthorizationInfo aut
Map<String, IndexAbstraction> indicesLookup, ActionListener<List<String>> listener) {
if (authorizationInfo instanceof RBACAuthorizationInfo) {
final Role role = ((RBACAuthorizationInfo) authorizationInfo).getRole();
listener.onResponse(resolveAuthorizedIndicesFromRole(role, requestInfo.getAction(), indicesLookup));
listener.onResponse(resolveAuthorizedIndicesFromRole(role, requestInfo, indicesLookup));
} else {
listener.onFailure(
new IllegalArgumentException("unsupported authorization info:" + authorizationInfo.getClass().getSimpleName()));
Expand Down Expand Up @@ -500,18 +501,29 @@ GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole) {
return new GetUserPrivilegesResponse(cluster, conditionalCluster, indices, application, runAs);
}

static List<String> resolveAuthorizedIndicesFromRole(Role role, String action, Map<String, IndexAbstraction> aliasAndIndexLookup) {
Predicate<String> predicate = role.allowedIndicesMatcher(action);
static List<String> resolveAuthorizedIndicesFromRole(Role role, RequestInfo requestInfo, Map<String, IndexAbstraction> lookup) {
Predicate<String> predicate = role.allowedIndicesMatcher(requestInfo.getAction());

List<String> indicesAndAliases = new ArrayList<>();
// do not include data streams for actions that do not operate on data streams
TransportRequest request = requestInfo.getRequest();
boolean includeDataStreams = (request instanceof IndicesRequest) && ((IndicesRequest) request).includeDataStreams();

Set<String> indicesAndAliases = new HashSet<>();
// TODO: can this be done smarter? I think there are usually more indices/aliases in the cluster then indices defined a roles?
for (Map.Entry<String, IndexAbstraction> entry : aliasAndIndexLookup.entrySet()) {
String aliasOrIndex = entry.getKey();
if (predicate.test(aliasOrIndex)) {
indicesAndAliases.add(aliasOrIndex);
for (Map.Entry<String, IndexAbstraction> entry : lookup.entrySet()) {
String indexAbstraction = entry.getKey();
if (predicate.test(indexAbstraction)) {
if (entry.getValue().getType() != IndexAbstraction.Type.DATA_STREAM) {
indicesAndAliases.add(indexAbstraction);
} else if (includeDataStreams) {
// add data stream and its backing indices for any authorized data streams
indicesAndAliases.addAll(entry.getValue().getIndices().stream()
.map(i -> i.getIndex().getName()).collect(Collectors.toList()));
indicesAndAliases.add(indexAbstraction);
}
}
}
return Collections.unmodifiableList(indicesAndAliases);
return Collections.unmodifiableList(new ArrayList<>(indicesAndAliases));
}

private void buildIndicesAccessControl(Authentication authentication, String action,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
Expand All @@ -35,7 +37,7 @@ public class AuthorizedIndicesTests extends ESTestCase {

public void testAuthorizedIndicesUserWithoutRoles() {
List<String> authorizedIndices =
RBACEngine.resolveAuthorizedIndicesFromRole(Role.EMPTY, "", Metadata.EMPTY_METADATA.getIndicesLookup());
RBACEngine.resolveAuthorizedIndicesFromRole(Role.EMPTY, getRequestInfo(""), Metadata.EMPTY_METADATA.getIndicesLookup());
assertTrue(authorizedIndices.isEmpty());
}

Expand Down Expand Up @@ -71,7 +73,7 @@ public void testAuthorizedIndicesUserWithSomeRoles() {
CompositeRolesStore.buildRoleFromDescriptors(descriptors, new FieldPermissionsCache(Settings.EMPTY), null, future);
Role roles = future.actionGet();
List<String> list =
RBACEngine.resolveAuthorizedIndicesFromRole(roles, SearchAction.NAME, metadata.getIndicesLookup());
RBACEngine.resolveAuthorizedIndicesFromRole(roles, getRequestInfo(SearchAction.NAME), metadata.getIndicesLookup());
assertThat(list, containsInAnyOrder("a1", "a2", "aaaaaa", "b", "ab"));
assertFalse(list.contains("bbbbb"));
assertFalse(list.contains("ba"));
Expand All @@ -81,15 +83,15 @@ public void testAuthorizedIndicesUserWithSomeRoles() {

public void testAuthorizedIndicesUserWithSomeRolesEmptyMetadata() {
Role role = Role.builder("role").add(IndexPrivilege.ALL, "*").build();
List<String> authorizedIndices =
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, Metadata.EMPTY_METADATA.getIndicesLookup());
List<String> authorizedIndices = RBACEngine.resolveAuthorizedIndicesFromRole(role, getRequestInfo(SearchAction.NAME),
Metadata.EMPTY_METADATA.getIndicesLookup());
assertTrue(authorizedIndices.isEmpty());
}

public void testSecurityIndicesAreRemovedFromRegularUser() {
Role role = Role.builder("user_role").add(IndexPrivilege.ALL, "*").cluster(Set.of("all"), Set.of()).build();
List<String> authorizedIndices =
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, Metadata.EMPTY_METADATA.getIndicesLookup());
List<String> authorizedIndices = RBACEngine.resolveAuthorizedIndicesFromRole(role, getRequestInfo(SearchAction.NAME),
Metadata.EMPTY_METADATA.getIndicesLookup());
assertTrue(authorizedIndices.isEmpty());
}

Expand All @@ -114,7 +116,7 @@ public void testSecurityIndicesAreRestrictedForDefaultRole() {
.build();

List<String> authorizedIndices =
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metadata.getIndicesLookup());
RBACEngine.resolveAuthorizedIndicesFromRole(role, getRequestInfo(SearchAction.NAME), metadata.getIndicesLookup());
assertThat(authorizedIndices, containsInAnyOrder("an-index", "another-index"));
assertThat(authorizedIndices, not(contains(internalSecurityIndex)));
assertThat(authorizedIndices, not(contains(RestrictedIndicesNames.SECURITY_MAIN_ALIAS)));
Expand All @@ -140,13 +142,21 @@ public void testSecurityIndicesAreNotRemovedFromUnrestrictedRole() {
.build();

List<String> authorizedIndices =
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metadata.getIndicesLookup());
RBACEngine.resolveAuthorizedIndicesFromRole(role, getRequestInfo(SearchAction.NAME), metadata.getIndicesLookup());
assertThat(authorizedIndices, containsInAnyOrder(
"an-index", "another-index", RestrictedIndicesNames.SECURITY_MAIN_ALIAS, internalSecurityIndex));

List<String> authorizedIndicesSuperUser =
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metadata.getIndicesLookup());
RBACEngine.resolveAuthorizedIndicesFromRole(role, getRequestInfo(SearchAction.NAME), metadata.getIndicesLookup());
assertThat(authorizedIndicesSuperUser, containsInAnyOrder(
"an-index", "another-index", RestrictedIndicesNames.SECURITY_MAIN_ALIAS, internalSecurityIndex));
}

public static AuthorizationEngine.RequestInfo getRequestInfo(String action) {
return getRequestInfo(TransportRequest.Empty.INSTANCE, action);
}

public static AuthorizationEngine.RequestInfo getRequestInfo(TransportRequest request, String action) {
return new AuthorizationEngine.RequestInfo(null, request, action);
}
}
Loading

0 comments on commit 50ed781

Please sign in to comment.