From 3a0aa1312767c6380022fc1404d3610c64ebe985 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Thu, 7 Jul 2016 10:28:10 -0400 Subject: [PATCH] support lucene query cache when using FLS Original commit: elastic/x-pack-elasticsearch@d1e8b9605d99d38a343defa6695f76659baeab81 --- .../authz/accesscontrol/FieldExtractor.java | 91 ++++++++++++ .../authz/accesscontrol/OptOutQueryCache.java | 42 +++++- .../accesscontrol/FieldExtractorTests.java | 138 ++++++++++++++++++ 3 files changed, 266 insertions(+), 5 deletions(-) create mode 100644 elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractor.java create mode 100644 elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractorTests.java diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractor.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractor.java new file mode 100644 index 00000000000..bf5ab116a83 --- /dev/null +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractor.java @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.authz.accesscontrol; + +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.DisjunctionMaxQuery; +import org.apache.lucene.search.DocValuesNumbersQuery; +import org.apache.lucene.search.DocValuesRangeQuery; +import org.apache.lucene.search.FieldValueQuery; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.MultiPhraseQuery; +import org.apache.lucene.search.PhraseQuery; +import org.apache.lucene.search.PointInSetQuery; +import org.apache.lucene.search.PointRangeQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SynonymQuery; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.Weight; +import org.apache.lucene.search.spans.SpanTermQuery; + +import java.util.Set; + +/** + * Extracts fields from a query, or throws UnsupportedOperationException. + *

+ * Lucene queries have {@link Weight#extractTerms}, but this is really geared at things + * such as highlighting, not security. For example terms in a Boolean {@code MUST_NOT} clause + * are not included, TermsQuery doesn't implement the method as it could be terribly slow, etc. + */ +class FieldExtractor { + + /** + * Populates {@code fields} with the set of fields used by the query, or throws + * UnsupportedOperationException if it doesn't know how to do this. + */ + static void extractFields(Query query, Set fields) throws UnsupportedOperationException { + // NOTE: we expect a rewritten query, so we only need logic for "atomic" queries here: + if (query instanceof BooleanQuery) { + // extract from all clauses + BooleanQuery q = (BooleanQuery) query; + for (BooleanClause clause : q.clauses()) { + extractFields(clause.getQuery(), fields); + } + } else if (query instanceof DisjunctionMaxQuery) { + // extract from all clauses + DisjunctionMaxQuery q = (DisjunctionMaxQuery) query; + for (Query clause : q.getDisjuncts()) { + extractFields(clause, fields); + } + } else if (query instanceof SpanTermQuery) { + // we just do SpanTerm, other spans are trickier, they could contain + // the evil FieldMaskingSpanQuery: so SpanQuery.getField cannot be trusted. + fields.add(((SpanTermQuery)query).getField()); + } else if (query instanceof TermQuery) { + fields.add(((TermQuery)query).getTerm().field()); + } else if (query instanceof SynonymQuery) { + SynonymQuery q = (SynonymQuery) query; + // all terms must have the same field + fields.add(q.getTerms().get(0).field()); + } else if (query instanceof PhraseQuery) { + PhraseQuery q = (PhraseQuery) query; + // all terms must have the same field + fields.add(q.getTerms()[0].field()); + } else if (query instanceof MultiPhraseQuery) { + MultiPhraseQuery q = (MultiPhraseQuery) query; + // all terms must have the same field + fields.add(q.getTermArrays()[0][0].field()); + } else if (query instanceof PointRangeQuery) { + fields.add(((PointRangeQuery)query).getField()); + } else if (query instanceof PointInSetQuery) { + fields.add(((PointInSetQuery)query).getField()); + } else if (query instanceof FieldValueQuery) { + fields.add(((FieldValueQuery)query).getField()); + } else if (query instanceof DocValuesNumbersQuery) { + fields.add(((DocValuesNumbersQuery)query).getField()); + } else if (query instanceof DocValuesRangeQuery) { + fields.add(((DocValuesRangeQuery)query).getField()); + } else if (query instanceof MatchAllDocsQuery) { + // no field + } else if (query instanceof MatchNoDocsQuery) { + // no field + } else { + throw new UnsupportedOperationException(); // we don't know how to get the fields from it + } + } +} diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java index c95d44096c8..7cf8a0e0e83 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java @@ -16,8 +16,12 @@ import org.elasticsearch.indices.IndicesQueryCache; import org.elasticsearch.search.internal.ShardSearchRequest; import org.elasticsearch.xpack.security.authz.InternalAuthorizationService; +import java.util.HashSet; +import java.util.Set; + /** - * Opts out of the query cache if field level security is active for the current request. + * Opts out of the query cache if field level security is active for the current request, + * and its unsafe to cache. */ public final class OptOutQueryCache extends AbstractIndexComponent implements QueryCache { @@ -64,13 +68,41 @@ public final class OptOutQueryCache extends AbstractIndexComponent implements Qu IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(indexName); if (indexAccessControl != null && indexAccessControl.getFields() != null) { - logger.debug("opting out of the query cache. request for index [{}] has field level security enabled", indexName); - // If in the future there is a Query#extractFields() then we can be smart on when to skip the query cache. - // (only cache if all fields in the query also are defined in the role) - return weight; + if (cachingIsSafe(weight, indexAccessControl)) { + logger.trace("not opting out of the query cache. request for index [{}] is safe to cache", indexName); + return indicesQueryCache.doCache(weight, policy); + } else { + logger.trace("opting out of the query cache. request for index [{}] is unsafe to cache", indexName); + return weight; + } } else { logger.trace("not opting out of the query cache. request for index [{}] has field level security disabled", indexName); return indicesQueryCache.doCache(weight, policy); } } + + /** + * Returns true if its safe to use the query cache for this query. + */ + static boolean cachingIsSafe(Weight weight, IndicesAccessControl.IndexAccessControl permissions) { + // support caching for common queries, by inspecting the field + // TODO: If in the future there is a Query#extractFields() then we can do a better job + Set fields = new HashSet<>(); + try { + FieldExtractor.extractFields(weight.getQuery(), fields); + } catch (UnsupportedOperationException ok) { + // we don't know how to safely extract the fields of this query, don't cache. + return false; + } + + // we successfully extracted the set of fields: check each one + for (String field : fields) { + // don't cache any internal fields (e.g. _field_names), these are complicated. + if (field.startsWith("_") || permissions.getFields().contains(field) == false) { + return false; + } + } + // we can cache, all fields are ok + return true; + } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractorTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractorTests.java new file mode 100644 index 00000000000..b0e42a0d121 --- /dev/null +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/FieldExtractorTests.java @@ -0,0 +1,138 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.authz.accesscontrol; + +import org.apache.lucene.document.IntPoint; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.AssertingQuery; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.DisjunctionMaxQuery; +import org.apache.lucene.search.DocValuesNumbersQuery; +import org.apache.lucene.search.DocValuesRangeQuery; +import org.apache.lucene.search.FieldValueQuery; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.MultiPhraseQuery; +import org.apache.lucene.search.PhraseQuery; +import org.apache.lucene.search.SynonymQuery; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.spans.SpanTermQuery; +import org.elasticsearch.test.ESTestCase; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** Simple tests for query field extraction */ +public class FieldExtractorTests extends ESTestCase { + + public void testBoolean() { + Set fields = new HashSet<>(); + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + builder.add(new TermQuery(new Term("foo", "bar")), BooleanClause.Occur.MUST); + builder.add(new TermQuery(new Term("no", "baz")), BooleanClause.Occur.MUST_NOT); + FieldExtractor.extractFields(builder.build(), fields); + assertEquals(asSet("foo", "no"), fields); + } + + public void testDisjunctionMax() { + Set fields = new HashSet<>(); + DisjunctionMaxQuery query = new DisjunctionMaxQuery(Arrays.asList( + new TermQuery(new Term("one", "bar")), + new TermQuery(new Term("two", "baz")) + ), 1.0F); + FieldExtractor.extractFields(query, fields); + assertEquals(asSet("one", "two"), fields); + } + + public void testSpanTerm() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new SpanTermQuery(new Term("foo", "bar")), fields); + assertEquals(asSet("foo"), fields); + } + + public void testTerm() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new TermQuery(new Term("foo", "bar")), fields); + assertEquals(asSet("foo"), fields); + } + + public void testSynonym() { + Set fields = new HashSet<>(); + SynonymQuery query = new SynonymQuery(new Term("foo", "bar"), new Term("foo", "baz")); + FieldExtractor.extractFields(query, fields); + assertEquals(asSet("foo"), fields); + } + + public void testPhrase() { + Set fields = new HashSet<>(); + PhraseQuery.Builder builder = new PhraseQuery.Builder(); + builder.add(new Term("foo", "bar")); + builder.add(new Term("foo", "baz")); + FieldExtractor.extractFields(builder.build(), fields); + assertEquals(asSet("foo"), fields); + } + + public void testMultiPhrase() { + Set fields = new HashSet<>(); + MultiPhraseQuery.Builder builder = new MultiPhraseQuery.Builder(); + builder.add(new Term("foo", "bar")); + builder.add(new Term[] { new Term("foo", "baz"), new Term("foo", "baz2") }); + FieldExtractor.extractFields(builder.build(), fields); + assertEquals(asSet("foo"), fields); + } + + public void testPointRange() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(IntPoint.newRangeQuery("foo", 3, 4), fields); + assertEquals(asSet("foo"), fields); + } + + public void testPointSet() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(IntPoint.newSetQuery("foo", 3, 4, 5), fields); + assertEquals(asSet("foo"), fields); + } + + public void testFieldValue() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new FieldValueQuery("foo"), fields); + assertEquals(asSet("foo"), fields); + } + + public void testDocValuesNumbers() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new DocValuesNumbersQuery("foo", 5L), fields); + assertEquals(asSet("foo"), fields); + } + + public void testDocValuesRange() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(DocValuesRangeQuery.newLongRange("foo", 1L, 2L, true, true), fields); + assertEquals(asSet("foo"), fields); + } + + public void testMatchAllDocs() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new MatchAllDocsQuery(), fields); + assertEquals(Collections.emptySet(), fields); + } + + public void testMatchNoDocs() { + Set fields = new HashSet<>(); + FieldExtractor.extractFields(new MatchNoDocsQuery(), fields); + assertEquals(Collections.emptySet(), fields); + } + + public void testUnsupported() { + Set fields = new HashSet<>(); + expectThrows(UnsupportedOperationException.class, () -> { + FieldExtractor.extractFields(new AssertingQuery(random(), new MatchAllDocsQuery()), fields); + }); + } +}