Indices resolution: don't go over the indices for requests that don't contain wildcards, just return them as they are

We have two ways of resolving wildcards in shield:
1) expanding them to matching authorized indices for the current user, which is used for every request that implements `IndicesRequest.Replaceable`, giving to wildcards a different meaning in the context of shield, abnd replacing the resolved names to the request on the coordinating node.
2) resolving them as es core would by default: we do this only for `IndicesAliasesRequest` since it's the only request that supports wildcards but doesn't allow to replace its indices. This is done on every node that processes the request, no replacement to the request takes place.

Shard/node level requests are a bit of a special case though since they could potentially contain wildcards. They hold the original indices and indices options, thus they effectively support wildcards, but given that wildcards always get replaced on the coordinating node even before shard/node level requests get created, we are sure they will never contain wildcards. Hence we should never even try to explode their wildcards, since they can't contain any.

We should make the above distinctions clearer in code by:
1) having an assert that verifies the IndicesAliasesRequest special case
2) making sure that we explode wildcards as core would only for IndicesAliasesRequest, not touching shard/node level requests
3) adding an assert that verifies that shard/node level requests never contain wildcards
i

Also, the process of going over the indices by using MetaData#convertFromWildcards (what option 2 does) has one side effect other than wildcards resolution: it causes unnecessary exceptions in shield, exceptions that would be thrown by core anyway when needed after the authorization process. This happens because we try and reuse code taken from es core that does wildcards resolution plus indices validation at once (even if there were no wildcards among indices).

In general all of the user requests that support wildcards (based on their indices options) should have their indices replaced on the coordinating node (the only exception being IndicesAliasesRequest, see elastic/elasticsearch#112), using shield specific code. Their subsequent (internal) shard level requests will never contain wildcards. That's why there is no need to go over all of the indices when there's no wildcards, which would cause some needless validation to happen as well.

Side note: the additional validation step caused tribe node failures with requests against indices belonging to multiple tribes (the exact purpose of the tribe node). Each tribe complained because it didn't have all of the indices in its own cluster state, which is perfectly fine (think of `tribe1` that holds `index1` and `tribe2` that holds `index2`, when searching against both indices from a tribe node). Although this commit makes sure that we don't throw any index missing exception for indices that are not available, all of the tribes will still need to authorize the action on all of the indices (`tribe1` requires privileges for `index2` so does `tribe2` for `index1`, otherwise the shard level requests will get rejected.

Closes elastic/elasticsearch#541

Original commit: elastic/x-pack-elasticsearch@dd81ec0177
This commit is contained in:
javanna 2014-12-23 15:33:14 +01:00 committed by Luca Cavanna
parent 95f125deda
commit 6df767790f
1 changed files with 29 additions and 2 deletions

View File

@ -7,6 +7,7 @@ package org.elasticsearch.shield.authz.indicesresolver;
import org.elasticsearch.action.CompositeIndicesRequest;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
@ -69,7 +70,7 @@ public class DefaultIndicesResolver implements IndicesResolver<TransportRequest>
//indices resulted in no indices. This is important as we always need to replace wildcards for security reason,
//to make sure that the operation is executed on the indices that we authorized it to execute on.
//If we can't replace because we got an empty set, we can only throw exception.
//Downside of this is that a single item exception is going to its composite requests fail as a whole.
//Downside of this is that a single item exception is going to make fail the composite request that holds it as a whole.
if (indices == null || indices.isEmpty()) {
if (MetaData.isAllIndices(indicesRequest.indices())) {
throw new IndexMissingException(new Index(MetaData.ALL));
@ -79,11 +80,25 @@ public class DefaultIndicesResolver implements IndicesResolver<TransportRequest>
((IndicesRequest.Replaceable) indicesRequest).indices(indices.toArray(new String[indices.size()]));
return Sets.newHashSet(indices);
}
return Sets.newHashSet(explodeWildcards(indicesRequest, metaData));
if (containsWildcards(indicesRequest)) {
//used for requests that support wildcards but don't allow to replace their indices (e.g. IndicesAliasesRequest)
//potentially insecure as cluster state may change hence we may end up resolving to different indices on different nodes
assert indicesRequest instanceof IndicesAliasesRequest
: "IndicesAliasesRequest is the only request known to support wildcards that doesn't support replacing its indices";
return Sets.newHashSet(explodeWildcards(indicesRequest, metaData));
}
//NOTE: shard level requests do support wildcards (as they hold the original indices options) but don't support replacing their indices.
//That is fine though because they never contain wildcards, as they get replaced as part of the authorization of their
//corresponding parent request on the coordinating node. Hence wildcards don't get replaced nor exploded for shard level requests.
}
return Sets.newHashSet(indicesRequest.indices());
}
/*
* Explodes wildcards based on default core behaviour. Used for IndicesAliasesRequest only as it doesn't support
* replacing its indices. It will go away once that gets fixed.
*/
private String[] explodeWildcards(IndicesRequest indicesRequest, MetaData metaData) {
//note that "_all" will map to concrete indices only, as the same happens in core
//which is different from "*" as the latter expands to all indices and aliases
@ -100,6 +115,18 @@ public class DefaultIndicesResolver implements IndicesResolver<TransportRequest>
return metaData.convertFromWildcards(indicesRequest.indices(), indicesRequest.indicesOptions());
}
private boolean containsWildcards(IndicesRequest indicesRequest) {
if (MetaData.isAllIndices(indicesRequest.indices())) {
return true;
}
for (String index : indicesRequest.indices()) {
if (index.startsWith("+") || index.startsWith("-") || Regex.isSimpleMatchPattern(index)) {
return true;
}
}
return false;
}
private List<String> replaceWildcardsWithAuthorizedIndices(IndicesRequest indicesRequest, MetaData metaData, List<String> authorizedIndices) {
if (MetaData.isAllIndices(indicesRequest.indices())) {