Improve QueryShardContext creation in SecurityIndexSearcherWrapper. (elastic/elasticsearch#3930)

Currently security always parses the permissions filters with a shard id equal
to `0` even if the query is executed on a different shard. Also it does not
protect against queries that may rely on the current timestamp even though we
don`t currently have ways to make sure that all shards use a consistent
timestamp.

Sibling of elastic/elasticsearchelastic/elasticsearch#21196.

Original commit: elastic/x-pack-elasticsearch@cab47f2ed2
This commit is contained in:
Adrien Grand 2016-11-02 09:49:06 +01:00 committed by GitHub
parent 043da7afe8
commit 3e92b905c7
3 changed files with 14 additions and 16 deletions

View File

@ -445,7 +445,14 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
assert licenseState != null;
if (XPackSettings.DLS_FLS_ENABLED.get(settings)) {
module.setSearcherWrapper(indexService ->
new SecurityIndexSearcherWrapper(indexService.getIndexSettings(), indexService.newQueryShardContext(),
new SecurityIndexSearcherWrapper(indexService.getIndexSettings(),
shardId -> indexService.newQueryShardContext(shardId.id(),
// we pass a null index reader, which is legal and will disable rewrite optimizations
// based on index statistics, which is probably safer...
null,
() -> {
throw new IllegalArgumentException("permission filters are not allowed to use the current timestamp");
}),
indexService.mapperService(), indexService.cache().bitsetFilterCache(),
indexService.getThreadPool().getThreadContext(), licenseState,
indexService.getScriptService()));

View File

@ -82,6 +82,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import static org.apache.lucene.search.BooleanClause.Occur.SHOULD;
@ -100,21 +101,21 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
private final MapperService mapperService;
private final Set<String> allowedMetaFields;
private final QueryShardContext queryShardContext;
private final Function<ShardId, QueryShardContext> queryShardContextProvider;
private final BitsetFilterCache bitsetFilterCache;
private final XPackLicenseState licenseState;
private final ThreadContext threadContext;
private final Logger logger;
private final ScriptService scriptService;
public SecurityIndexSearcherWrapper(IndexSettings indexSettings, QueryShardContext queryShardContext,
public SecurityIndexSearcherWrapper(IndexSettings indexSettings, Function<ShardId, QueryShardContext> queryShardContextProvider,
MapperService mapperService, BitsetFilterCache bitsetFilterCache,
ThreadContext threadContext, XPackLicenseState licenseState,
ScriptService scriptService) {
this.scriptService = scriptService;
this.logger = Loggers.getLogger(getClass(), indexSettings.getSettings());
this.mapperService = mapperService;
this.queryShardContext = queryShardContext;
this.queryShardContextProvider = queryShardContextProvider;
this.bitsetFilterCache = bitsetFilterCache;
this.threadContext = threadContext;
this.licenseState = licenseState;
@ -153,7 +154,7 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
if (permissions.getQueries() != null) {
BooleanQuery.Builder filter = new BooleanQuery.Builder();
for (BytesReference bytesReference : permissions.getQueries()) {
QueryShardContext queryShardContext = copyQueryShardContext(this.queryShardContext);
QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId);
bytesReference = evaluateTemplate(bytesReference);
try (XContentParser parser = XContentFactory.xContent(bytesReference).createParser(bytesReference)) {
Optional<QueryBuilder> queryBuilder = queryShardContext.newParseContext(parser).parseInnerQueryBuilder();
@ -263,11 +264,6 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
return allowedMetaFields;
}
// for testing:
protected QueryShardContext copyQueryShardContext(QueryShardContext context) {
return new QueryShardContext(context);
}
private void resolveParentChildJoinFields(Set<String> allowedFields) {
for (DocumentMapper mapper : mapperService.docMappers(false)) {
ParentFieldMapper parentFieldMapper = mapper.parentFieldMapper();

View File

@ -92,14 +92,9 @@ public class SecurityIndexSearcherWrapperIntegrationTests extends ESTestCase {
});
XPackLicenseState licenseState = mock(XPackLicenseState.class);
when(licenseState.isDocumentAndFieldLevelSecurityAllowed()).thenReturn(true);
SecurityIndexSearcherWrapper wrapper = new SecurityIndexSearcherWrapper(indexSettings, queryShardContext, mapperService,
SecurityIndexSearcherWrapper wrapper = new SecurityIndexSearcherWrapper(indexSettings, s -> queryShardContext, mapperService,
bitsetFilterCache, threadContext, licenseState, scriptService) {
@Override
protected QueryShardContext copyQueryShardContext(QueryShardContext context) {
return queryShardContext;
}
@Override
protected IndicesAccessControl getIndicesAccessControl() {
return new IndicesAccessControl(true, singletonMap("_index", indexAccessControl));