Only negate index expression on all indices with preceding wildcard

Adapt security plugin to https://github.com/elastic/elasticsearch/pull/20898 .

Closes elastic/elasticsearch#3749

Original commit: elastic/x-pack-elasticsearch@2f3b0b17e1
This commit is contained in:
javanna 2016-10-17 23:43:47 +02:00 committed by Luca Cavanna
parent 74334b3713
commit 7191bb76ee
6 changed files with 123 additions and 45 deletions

View File

@ -58,7 +58,6 @@ import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
import static org.bouncycastle.asn1.x500.style.RFC4519Style.name;
import static org.elasticsearch.xpack.security.Security.setting;
import static org.elasticsearch.xpack.security.support.Exceptions.authorizationError;
@ -214,7 +213,7 @@ public class AuthorizationService extends AbstractComponent {
//all wildcard expressions have been resolved and only the security plugin could have set '-*' here.
//'-*' matches no indices so we allow the request to go through, which will yield an empty response
if (indexNames.size() == 1 && indexNames.contains(IndicesAndAliasesResolver.NO_INDEX)) {
if (indexNames.size() == 1 && indexNames.contains(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER)) {
setIndicesAccessControl(IndicesAccessControl.ALLOW_NO_INDICES);
grant(authentication, action, request);
return;

View File

@ -17,7 +17,6 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.transport.TransportRequest;
@ -32,8 +31,12 @@ import java.util.stream.Collectors;
public class IndicesAndAliasesResolver {
public static final String NO_INDEX = "-*";
private static final List<String> NO_INDICES = Collections.singletonList(NO_INDEX);
//placeholder used in the security plugin to indicate that the request is authorized knowing that it will yield an empty response
public static final String NO_INDEX_PLACEHOLDER = "-*";
private static final Set<String> NO_INDEX_PLACEHOLDER_SET = Collections.singleton(NO_INDEX_PLACEHOLDER);
//`*,-*` what we replace indices with if we need Elasticsearch to return empty responses without throwing exception
private static final String[] NO_INDICES_ARRAY = new String[] {"*", "-*"};
static final List<String> NO_INDICES_LIST = Arrays.asList(NO_INDICES_ARRAY);
private final IndexNameExpressionResolver nameExpressionResolver;
@ -58,20 +61,18 @@ public class IndicesAndAliasesResolver {
return resolveIndicesAndAliases((IndicesRequest) request, metaData, authorizedIndices);
}
private Set<String> resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData,
AuthorizedIndices authorizedIndices) {
private Set<String> resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, AuthorizedIndices authorizedIndices) {
boolean indicesReplacedWithNoIndices = false;
final Set<String> indices;
if (indicesRequest instanceof PutMappingRequest
&& ((PutMappingRequest) indicesRequest).getConcreteIndex() != null) {
if (indicesRequest instanceof PutMappingRequest && ((PutMappingRequest) indicesRequest).getConcreteIndex() != null) {
/*
* This is a special case since PutMappingRequests from dynamic mapping updates have a concrete index
* if this index is set and it's in the list of authorized indices we are good and don't need to put
* the list of indices in there, if we do so it will result in an invalid request and the update will fail.
*/
indices = Collections.singleton(((PutMappingRequest) indicesRequest).getConcreteIndex().getName());
assert indicesRequest.indices() == null || indicesRequest.indices().length == 0
: "indices are: " + Arrays.toString(indicesRequest.indices()); // Arrays.toString() can handle null values - all good
return Collections.singleton(((PutMappingRequest) indicesRequest).getConcreteIndex().getName());
} else if (indicesRequest instanceof IndicesRequest.Replaceable) {
IndicesRequest.Replaceable replaceable = (IndicesRequest.Replaceable) indicesRequest;
final boolean replaceWildcards = indicesRequest.indicesOptions().expandWildcardsOpen()
@ -110,16 +111,20 @@ public class IndicesAndAliasesResolver {
//this is how we tell es core to return an empty response, we can let the request through being sure
//that the '-*' wildcard expression will be resolved to no indices. We can't let empty indices through
//as that would be resolved to _all by es core.
replacedIndices = NO_INDICES;
replaceable.indices(NO_INDICES_ARRAY);
indicesReplacedWithNoIndices = true;
indices = NO_INDEX_PLACEHOLDER_SET;
} else {
throw new IndexNotFoundException(Arrays.toString(indicesRequest.indices()));
}
} else {
replaceable.indices(replacedIndices.toArray(new String[replacedIndices.size()]));
indices = new HashSet<>(replacedIndices);
}
replaceable.indices(replacedIndices.toArray(new String[replacedIndices.size()]));
indices = Sets.newHashSet(replacedIndices);
} else {
if (containsWildcards(indicesRequest)) {
//an alias can still contain '*' in its name as of 5.0. Such aliases cannot be referred to when using
//the security plugin, otherwise the following exception gets thrown
throw new IllegalStateException("There are no external requests known to support wildcards that don't support replacing " +
"their indices");
}
@ -132,7 +137,7 @@ public class IndicesAndAliasesResolver {
for (String name : indicesRequest.indices()) {
resolvedNames.add(nameExpressionResolver.resolveDateMathExpression(name));
}
indices = Sets.newHashSet(resolvedNames);
indices = new HashSet<>(resolvedNames);
}
if (indicesRequest instanceof AliasesRequest) {
@ -156,7 +161,7 @@ public class IndicesAndAliasesResolver {
Collections.addAll(indices, aliasesRequest.aliases());
}
}
return indices;
return Collections.unmodifiableSet(indices);
}
private List<String> loadAuthorizedAliases(List<String> authorizedIndices, MetaData metaData) {
@ -222,31 +227,27 @@ public class IndicesAndAliasesResolver {
//TODO Investigate reusing code from vanilla es to resolve index names and wildcards
private List<String> replaceWildcardsWithAuthorizedIndices(String[] indices, IndicesOptions indicesOptions, MetaData metaData,
List<String> authorizedIndices, boolean replaceWildcards) {
//the order matters when it comes to + and - (see MetaData#convertFromWildcards)
//the order matters when it comes to + and -
List<String> finalIndices = new ArrayList<>();
for (int i = 0; i < indices.length; i++) {
String index = indices[i];
boolean wildcardSeen = false;
for (String index : indices) {
String aliasOrIndex;
boolean minus = false;
if (index.charAt(0) == '+') {
aliasOrIndex = index.substring(1);
} else if (index.charAt(0) == '-') {
if (i == 0) {
//mimic the MetaData#convertFromWilcards behaviour with "-index" syntax
//but instead of adding all the indices, add only the ones that the user is authorized for
for (String authorizedIndex : authorizedIndices) {
if (isIndexVisible(authorizedIndex, indicesOptions, metaData)) {
finalIndices.add(authorizedIndex);
}
}
if (wildcardSeen) {
aliasOrIndex = index.substring(1);
minus = true;
} else {
aliasOrIndex = index;
}
aliasOrIndex = index.substring(1);
minus = true;
} else {
aliasOrIndex = index;
}
if (replaceWildcards && Regex.isSimpleMatchPattern(aliasOrIndex)) {
wildcardSeen = true;
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : authorizedIndices) {
if (Regex.simpleMatch(aliasOrIndex, authorizedIndex) && isIndexVisible(authorizedIndex, indicesOptions, metaData)) {

View File

@ -24,7 +24,7 @@ public class IndicesAccessControl {
public static final IndicesAccessControl ALLOW_ALL = new IndicesAccessControl(true, Collections.emptyMap());
public static final IndicesAccessControl ALLOW_NO_INDICES = new IndicesAccessControl(true,
Collections.singletonMap(IndicesAndAliasesResolver.NO_INDEX,
Collections.singletonMap(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER,
new IndicesAccessControl.IndexAccessControl(true, new FieldPermissions(), null)));
private final boolean granted;

View File

@ -242,7 +242,7 @@ public class AuthorizationServiceTests extends ESTestCase {
verify(auditTrail).accessGranted(user, SearchAction.NAME, searchRequest);
IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY);
IndicesAccessControl.IndexAccessControl indexAccessControl =
indicesAccessControl.getIndexPermissions(IndicesAndAliasesResolver.NO_INDEX);
indicesAccessControl.getIndexPermissions(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER);
assertFalse(indexAccessControl.getFieldPermissions().hasFieldLevelSecurity());
assertNull(indexAccessControl.getQueries());
}
@ -508,8 +508,8 @@ public class AuthorizationServiceTests extends ESTestCase {
SearchRequest searchRequest = new SearchRequest("_all");
authorizationService.authorize(createAuthentication(user), SearchAction.NAME, searchRequest);
assertEquals(1, searchRequest.indices().length);
assertEquals(IndicesAndAliasesResolver.NO_INDEX, searchRequest.indices()[0]);
assertEquals(2, searchRequest.indices().length);
assertEquals(IndicesAndAliasesResolver.NO_INDICES_LIST, Arrays.asList(searchRequest.indices()));
}
public void testGrantedNonXPackUserCanExecuteMonitoringOperationsAgainstSecurityIndex() {

View File

@ -49,6 +49,7 @@ import org.elasticsearch.xpack.security.user.User;
import org.elasticsearch.xpack.security.user.XPackUser;
import org.junit.Before;
import java.util.Arrays;
import java.util.Collection;
import java.util.Set;
@ -96,7 +97,6 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
.put(indexBuilder("-index11").settings(settings))
.put(indexBuilder("-index20").settings(settings))
.put(indexBuilder("-index21").settings(settings))
.put(indexBuilder("+index30").settings(settings))
.put(indexBuilder(SecurityTemplateService.SECURITY_INDEX_NAME).settings(settings)).build();
user = new User("user", "role");
@ -153,6 +153,74 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
illegalStateException.getMessage());
}
public void testExplicitDashIndices() {
SearchRequest request = new SearchRequest("-index10", "-index20");
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-index10", "-index20"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testWildcardDashIndices() {
SearchRequest request;
if (randomBoolean()) {
request = new SearchRequest("-index*", "--index20");
} else {
request = new SearchRequest("*", "--index20");
}
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-index10", "-index11", "-index21"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testExplicitMixedWildcardDashIndices() {
SearchRequest request = new SearchRequest("-index21", "-does_not_exist", "-index1*", "--index11", "+-index20");
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-index10", "-index21", "-index20", "-does_not_exist"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testDashIndicesNoExpandWildcard() {
SearchRequest request = new SearchRequest("-index1*", "--index11");
request.indicesOptions(IndicesOptions.fromOptions(false, randomBoolean(), false, false));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-index1*", "--index11"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testDashIndicesPlusAndMinus() {
SearchRequest request = new SearchRequest("+-index10", "+-index11", "--index11", "-index20");
request.indicesOptions(IndicesOptions.fromOptions(false, randomBoolean(), randomBoolean(), randomBoolean()));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-index10", "-index11", "--index11", "-index20"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testDashNotExistingIndex() {
SearchRequest request = new SearchRequest("-does_not_exist");
request.indicesOptions(IndicesOptions.fromOptions(false, randomBoolean(), randomBoolean(), randomBoolean()));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(userDashIndices, SearchAction.NAME));
String[] expectedIndices = new String[]{"-does_not_exist"};
assertThat(indices.size(), equalTo(expectedIndices.length));
assertThat(request.indices().length, equalTo(expectedIndices.length));
assertThat(indices, hasItems(expectedIndices));
assertThat(request.indices(), arrayContainingInAnyOrder(expectedIndices));
}
public void testResolveEmptyIndicesExpandWilcardsOpenAndClosed() {
SearchRequest request = new SearchRequest();
request.indicesOptions(IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), true, true));
@ -242,7 +310,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsMinusExpandWilcardsOpen() {
SearchRequest request = new SearchRequest("-foofoo*");
SearchRequest request = new SearchRequest("*", "-foofoo*");
request.indicesOptions(IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), true, false));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar"};
@ -253,7 +321,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsMinusExpandWilcardsOpenAndClosed() {
SearchRequest request = new SearchRequest("-foofoo*");
SearchRequest request = new SearchRequest("*", "-foofoo*");
request.indicesOptions(IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), true, true));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar", "bar-closed"};
@ -264,7 +332,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsPlusAndMinusExpandWilcardsOpenStrict() {
SearchRequest request = new SearchRequest("-foofoo*", "+barbaz", "+foob*");
SearchRequest request = new SearchRequest("*", "-foofoo*", "+barbaz", "+foob*");
request.indicesOptions(IndicesOptions.fromOptions(false, true, true, false));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar", "barbaz"};
@ -275,7 +343,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsPlusAndMinusExpandWilcardsOpenIgnoreUnavailable() {
SearchRequest request = new SearchRequest("-foofoo*", "+barbaz", "+foob*");
SearchRequest request = new SearchRequest("*", "-foofoo*", "+barbaz", "+foob*");
request.indicesOptions(IndicesOptions.fromOptions(true, true, true, false));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar"};
@ -286,7 +354,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsPlusAndMinusExpandWilcardsOpenAndClosedStrict() {
SearchRequest request = new SearchRequest("-foofoo*", "+barbaz");
SearchRequest request = new SearchRequest("*", "-foofoo*", "+barbaz");
request.indicesOptions(IndicesOptions.fromOptions(false, randomBoolean(), true, true));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar", "bar-closed", "barbaz"};
@ -297,7 +365,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
}
public void testResolveWildcardsPlusAndMinusExpandWilcardsOpenAndClosedIgnoreUnavailable() {
SearchRequest request = new SearchRequest("-foofoo*", "+barbaz");
SearchRequest request = new SearchRequest("*", "-foofoo*", "+barbaz");
request.indicesOptions(IndicesOptions.fromOptions(true, randomBoolean(), true, true));
Set<String> indices = defaultIndicesResolver.resolve(request, metaData, buildAuthorizedIndices(user, SearchAction.NAME));
String[] replacedIndices = new String[]{"bar", "bar-closed"};
@ -1073,8 +1141,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase {
private static void assertNoIndices(IndicesRequest.Replaceable request, Set<String> resolvedIndices) {
assertEquals(1, resolvedIndices.size());
assertEquals(IndicesAndAliasesResolver.NO_INDEX, resolvedIndices.iterator().next());
assertEquals(1, request.indices().length);
assertEquals(IndicesAndAliasesResolver.NO_INDEX, request.indices()[0]);
assertEquals(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER, resolvedIndices.iterator().next());
assertEquals(IndicesAndAliasesResolver.NO_INDICES_LIST, Arrays.asList(request.indices()));
}
}

View File

@ -27,6 +27,7 @@ import java.util.List;
import static org.elasticsearch.test.SecurityTestsUtils.assertAuthorizationExceptionDefaultUsers;
import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionDefaultUsers;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits;
import static org.hamcrest.core.IsCollectionContaining.hasItems;
import static org.hamcrest.core.IsEqual.equalTo;
@ -42,7 +43,7 @@ public class ReadActionsTests extends SecurityIntegTestCase {
" indices:\n" +
" - names: '*'\n" +
" privileges: [ manage, write ]\n" +
" - names: '/test.*/'\n" +
" - names: ['/test.*/', '/-alias.*/']\n" +
" privileges: [ read ]\n";
}
@ -162,7 +163,7 @@ public class ReadActionsTests extends SecurityIntegTestCase {
//index1 is not authorized and referred to through wildcard, test2 is excluded
createIndicesWithRandomAliases("test1", "test2", "test3", "index1");
SearchResponse searchResponse = client().prepareSearch("-test2").get();
SearchResponse searchResponse = client().prepareSearch("*", "-test2").get();
assertReturnedIndices(searchResponse, "test1", "test3");
}
@ -170,7 +171,7 @@ public class ReadActionsTests extends SecurityIntegTestCase {
//index1 is not authorized and referred to through wildcard, test2 is excluded
createIndicesWithRandomAliases("test1", "test2", "test21", "test3", "index1");
SearchResponse searchResponse = client().prepareSearch("-test2*").get();
SearchResponse searchResponse = client().prepareSearch("*", "-test2*").get();
assertReturnedIndices(searchResponse, "test1", "test3");
}
@ -410,6 +411,16 @@ public class ReadActionsTests extends SecurityIntegTestCase {
assertThat(response.getResponses()[4].getFailure().getCause(), instanceOf(IndexNotFoundException.class));
}
public void testDashStarDoesntMatchDashAliases() {
createIndicesWithRandomAliases("test1", "test2", "test3");
//use legacy aliases that start with "-" so we test this edge case that clashes with exclusion patterns
//we have to make sure that the special index pattern that identifies no indices doesn't end up matching such aliases
assertAcked(client().admin().indices().prepareAliases().addAlias("test1", "-alias-test1").addAlias("test2", "-alias-test2"));
SearchResponse searchResponse = client().prepareSearch("does_not_exist_*")
.setIndicesOptions(IndicesOptions.fromOptions(randomBoolean(), true, true, randomBoolean())).get();
assertNoSearchHits(searchResponse);
}
private static void assertReturnedIndices(SearchResponse searchResponse, String... indices) {
List<String> foundIndices = new ArrayList<>();
for (SearchHit searchHit : searchResponse.getHits().getHits()) {