honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve error if Indices Permission is too complex (elastic/x-pack-elasticsearch#4239) 

If a user has roles that grant access to a large number of disparate
index patterns, then the resulting Automaton can become large and
too costly to determinise. This happens rarely, and is usually a sign
of a poorly implemented security model, so we have no immediate plans
to change the implementation. However the resulting error message is
not clear and does not provide sufficient information for users to
resolve the underlying problem.

This commit catches the underlying exception and provides a more
specific error message, with DEBUG logging of the offending index
patterns.

Original commit: elastic/x-pack-elasticsearch@532be70efc
This commit is contained in:
Tim Vernum 2018-03-29 10:55:48 +10:00 committed by GitHub
parent 53436450c4
commit 94b6f637a6
2 changed files with 39 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java
View File

@ -6,11 +6,15 @@
package org.elasticsearch.xpack.core.security.authz.permission;
import org.apache.lucene.util.automaton.Automaton;
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
import org.elasticsearch.xpack.core.security.support.Automatons;
@ -55,10 +59,23 @@ public final class IndicesPermission implements Iterable<IndicesPermission.Group
indices.addAll(Arrays.asList(group.indices));
}
}
return Automatons.predicate(indices);
return indexMatcher(indices);
};
}
static Predicate<String> indexMatcher(List<String> indices) {
try {
return Automatons.predicate(indices);
} catch (TooComplexToDeterminizeException e) {
Loggers.getLogger(IndicesPermission.class).debug("Index pattern automaton [{}] is too complex", indices);
String description = Strings.collectionToCommaDelimitedString(indices);
if (description.length() > 80) {
description = Strings.cleanTruncate(description, 80) + "...";
}
throw new ElasticsearchSecurityException("The set of permitted index patterns [{}] is too complex to evaluate", e, description);
}
}
@Override
public Iterator<Group> iterator() {
return Arrays.asList(groups).iterator();
@ -198,7 +215,7 @@ public final class IndicesPermission implements Iterable<IndicesPermission.Group
this.privilege = privilege;
this.actionMatcher = privilege.predicate();
this.indices = indices;
this.indexNameMatcher = Automatons.predicate(indices);
this.indexNameMatcher = indexMatcher(Arrays.asList(indices));
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
this.query = query;
}

20
plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java
View File

@ -5,7 +5,9 @@
*/
package org.elasticsearch.xpack.security.authz.accesscontrol;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.Version;
import org.elasticsearch.action.get.GetAction;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.cluster.metadata.IndexMetaData;
@ -31,6 +33,7 @@ import java.util.Collections;
import java.util.Map;
import java.util.Set;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@ -249,6 +252,23 @@ public class IndicesPermissionTests extends ESTestCase {
assertFalse(core.check("unknown"));
}
public void testErrorMessageIfIndexPatternIsTooComplex() {
final IndicesPermission.Group[] groups = new IndicesPermission.Group[26];
for (int i = 0; i < 26; i++) {
final char ch = (char) ('a' + i);
String index = Character.toString(ch);
for (int j = 0; j < 250; j++) {
index += "*" + ch;
}
groups[i] = new IndicesPermission.Group(IndexPrivilege.ALL, new FieldPermissions(), null, index);
}
final IndicesPermission permission = new IndicesPermission(groups);
final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class,
() -> permission.allowedIndicesMatcher(GetAction.NAME));
assertThat(e.getMessage(), containsString("a*a*a*a*a"));
assertThat(e.getMessage(), containsString("too complex to evaluate"));
}
private static FieldPermissionsDefinition fieldPermissionDef(String[] granted, String[] denied) {
return new FieldPermissionsDefinition(granted, denied);
}