Skip to content

Commit bb130f5

Browse files
authored
Use separate BitSet cache in Doc Level Security (#43669)
Document level security was depending on the shared "BitsetFilterCache" which (by design) never expires its entries. However, when using DLS queries - particularly templated ones - the number (and memory usage) of generated bitsets can be significant. This change introduces a new cache specifically for BitSets used in DLS queries, that has memory usage constraints and access time expiry. The whole cache is automatically cleared if the role cache is cleared. Individual bitsets are cleared when the corresponding lucene index reader is closed. The cache defaults to 50MB, and entries expire if unused for 7 days.
1 parent 21e2735 commit bb130f5

File tree

10 files changed

+588
-141
lines changed

10 files changed

+588
-141
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.core.security.authz.accesscontrol;
8+
9+
import org.apache.logging.log4j.LogManager;
10+
import org.apache.logging.log4j.Logger;
11+
import org.apache.lucene.index.IndexReader;
12+
import org.apache.lucene.index.IndexReaderContext;
13+
import org.apache.lucene.index.LeafReaderContext;
14+
import org.apache.lucene.index.ReaderUtil;
15+
import org.apache.lucene.search.IndexSearcher;
16+
import org.apache.lucene.search.Query;
17+
import org.apache.lucene.search.ScoreMode;
18+
import org.apache.lucene.search.Scorer;
19+
import org.apache.lucene.search.Weight;
20+
import org.apache.lucene.util.Accountable;
21+
import org.apache.lucene.util.BitSet;
22+
import org.apache.lucene.util.FixedBitSet;
23+
import org.elasticsearch.common.Nullable;
24+
import org.elasticsearch.common.cache.Cache;
25+
import org.elasticsearch.common.cache.CacheBuilder;
26+
import org.elasticsearch.common.settings.Setting;
27+
import org.elasticsearch.common.settings.Setting.Property;
28+
import org.elasticsearch.common.settings.Settings;
29+
import org.elasticsearch.common.unit.ByteSizeUnit;
30+
import org.elasticsearch.common.unit.ByteSizeValue;
31+
import org.elasticsearch.common.unit.TimeValue;
32+
import org.elasticsearch.common.util.set.Sets;
33+
34+
import java.io.Closeable;
35+
import java.util.List;
36+
import java.util.Map;
37+
import java.util.Objects;
38+
import java.util.Set;
39+
import java.util.concurrent.ConcurrentHashMap;
40+
import java.util.concurrent.ExecutionException;
41+
42+
/**
43+
* This is a cache for {@link BitSet} instances that are used with the {@link DocumentSubsetReader}.
44+
* It is bounded by memory size and access time.
45+
*
46+
* @see org.elasticsearch.index.cache.bitset.BitsetFilterCache
47+
*/
48+
public final class DocumentSubsetBitsetCache implements IndexReader.ClosedListener, Closeable, Accountable {
49+
50+
/**
51+
* The TTL defaults to 1 week. We depend on the {@code max_bytes} setting to keep the cache to a sensible size, by evicting LRU
52+
* entries, however there is benefit in reclaiming memory by expiring bitsets that have not be used for some period of time.
53+
* Because {@link org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.Group#query} can be templated, it is
54+
* not uncommon for a query to only be used for a relatively short period of time (e.g. because a user's metadata changed, or because
55+
* that user is an infrequent user of Elasticsearch). This access time expiry helps free up memory in those circumstances even if the
56+
* cache is never filled.
57+
*/
58+
static final Setting<TimeValue> CACHE_TTL_SETTING =
59+
Setting.timeSetting("xpack.security.dls.bitset.cache.ttl", TimeValue.timeValueHours(24 * 7), Property.NodeScope);
60+
61+
static final Setting<ByteSizeValue> CACHE_SIZE_SETTING = Setting.byteSizeSetting("xpack.security.dls.bitset.cache.size",
62+
new ByteSizeValue(50, ByteSizeUnit.MB), Property.NodeScope);
63+
64+
private static final BitSet NULL_MARKER = new FixedBitSet(0);
65+
66+
private final Logger logger;
67+
private final Cache<BitsetCacheKey, BitSet> bitsetCache;
68+
private final Map<IndexReader.CacheKey, Set<BitsetCacheKey>> keysByIndex;
69+
70+
public DocumentSubsetBitsetCache(Settings settings) {
71+
this.logger = LogManager.getLogger(getClass());
72+
final TimeValue ttl = CACHE_TTL_SETTING.get(settings);
73+
final ByteSizeValue size = CACHE_SIZE_SETTING.get(settings);
74+
this.bitsetCache = CacheBuilder.<BitsetCacheKey, BitSet>builder()
75+
.setExpireAfterAccess(ttl)
76+
.setMaximumWeight(size.getBytes())
77+
.weigher((key, bitSet) -> bitSet == NULL_MARKER ? 0 : bitSet.ramBytesUsed()).build();
78+
this.keysByIndex = new ConcurrentHashMap<>();
79+
}
80+
81+
@Override
82+
public void onClose(IndexReader.CacheKey ownerCoreCacheKey) {
83+
final Set<BitsetCacheKey> keys = keysByIndex.remove(ownerCoreCacheKey);
84+
if (keys != null) {
85+
// Because this Set has been removed from the map, and the only update to the set is performed in a
86+
// Map#compute call, it should not be possible to get a concurrent modification here.
87+
keys.forEach(bitsetCache::invalidate);
88+
}
89+
}
90+
91+
@Override
92+
public void close() {
93+
clear("close");
94+
}
95+
96+
public void clear(String reason) {
97+
logger.debug("clearing all DLS bitsets because [{}]", reason);
98+
// Due to the order here, it is possible than a new entry could be added _after_ the keysByIndex map is cleared
99+
// but _before_ the cache is cleared. This would mean it sits orphaned in keysByIndex, but this is not a issue.
100+
// When the index is closed, the key will be removed from the map, and there will not be a corresponding item
101+
// in the cache, which will make the cache-invalidate a no-op.
102+
// Since the entry is not in the cache, if #getBitSet is called, it will be loaded, and the new key will be added
103+
// to the index without issue.
104+
keysByIndex.clear();
105+
bitsetCache.invalidateAll();
106+
}
107+
108+
int entryCount() {
109+
return this.bitsetCache.count();
110+
}
111+
112+
@Override
113+
public long ramBytesUsed() {
114+
return this.bitsetCache.weight();
115+
}
116+
117+
/**
118+
* Obtain the {@link BitSet} for the given {@code query} in the given {@code context}.
119+
* If there is a cached entry for that query and context, it will be returned.
120+
* Otherwise a new BitSet will be created and stored in the cache.
121+
* The returned BitSet may be null (e.g. if the query has no results).
122+
*/
123+
@Nullable
124+
public BitSet getBitSet(final Query query, final LeafReaderContext context) throws ExecutionException {
125+
final IndexReader.CacheHelper coreCacheHelper = context.reader().getCoreCacheHelper();
126+
if (coreCacheHelper == null) {
127+
throw new IllegalArgumentException("Reader " + context.reader() + " does not support caching");
128+
}
129+
coreCacheHelper.addClosedListener(this);
130+
final IndexReader.CacheKey indexKey = coreCacheHelper.getKey();
131+
final BitsetCacheKey cacheKey = new BitsetCacheKey(indexKey, query);
132+
133+
final BitSet bitSet = bitsetCache.computeIfAbsent(cacheKey, ignore1 -> {
134+
// This ensures all insertions into the set are guarded by ConcurrentHashMap's atomicity guarantees.
135+
keysByIndex.compute(indexKey, (ignore2, set) -> {
136+
if (set == null) {
137+
set = Sets.newConcurrentHashSet();
138+
}
139+
set.add(cacheKey);
140+
return set;
141+
});
142+
final IndexReaderContext topLevelContext = ReaderUtil.getTopLevelContext(context);
143+
final IndexSearcher searcher = new IndexSearcher(topLevelContext);
144+
searcher.setQueryCache(null);
145+
final Weight weight = searcher.createWeight(searcher.rewrite(query), ScoreMode.COMPLETE_NO_SCORES, 1f);
146+
Scorer s = weight.scorer(context);
147+
if (s == null) {
148+
// A cache loader is not allowed to return null, return a marker object instead.
149+
return NULL_MARKER;
150+
} else {
151+
return BitSet.of(s.iterator(), context.reader().maxDoc());
152+
}
153+
});
154+
if (bitSet == NULL_MARKER) {
155+
return null;
156+
} else {
157+
return bitSet;
158+
}
159+
}
160+
161+
public static List<Setting<?>> getSettings() {
162+
return List.of(CACHE_TTL_SETTING, CACHE_SIZE_SETTING);
163+
}
164+
165+
public Map<String, Object> usageStats() {
166+
final ByteSizeValue ram = new ByteSizeValue(ramBytesUsed(), ByteSizeUnit.BYTES);
167+
return Map.of(
168+
"count", entryCount(),
169+
"memory", ram.toString(),
170+
"memory_in_bytes", ram.getBytes()
171+
);
172+
}
173+
174+
private class BitsetCacheKey {
175+
final IndexReader.CacheKey index;
176+
final Query query;
177+
178+
private BitsetCacheKey(IndexReader.CacheKey index, Query query) {
179+
this.index = index;
180+
this.query = query;
181+
}
182+
183+
@Override
184+
public boolean equals(Object other) {
185+
if (this == other) {
186+
return true;
187+
}
188+
if (other == null || getClass() != other.getClass()) {
189+
return false;
190+
}
191+
final BitsetCacheKey that = (BitsetCacheKey) other;
192+
return Objects.equals(this.index, that.index) &&
193+
Objects.equals(this.query, that.query);
194+
}
195+
196+
@Override
197+
public int hashCode() {
198+
return Objects.hash(index, query);
199+
}
200+
201+
@Override
202+
public String toString() {
203+
return getClass().getSimpleName() + "(" + index + "," + query + ")";
204+
}
205+
}
206+
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java

+10-11
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.common.cache.Cache;
2222
import org.elasticsearch.common.cache.CacheBuilder;
2323
import org.elasticsearch.common.logging.LoggerMessageFormat;
24-
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
2524

2625
import java.io.IOException;
2726
import java.io.UncheckedIOException;
@@ -34,9 +33,9 @@
3433
*/
3534
public final class DocumentSubsetReader extends FilterLeafReader {
3635

37-
public static DocumentSubsetDirectoryReader wrap(DirectoryReader in, BitsetFilterCache bitsetFilterCache,
36+
public static DocumentSubsetDirectoryReader wrap(DirectoryReader in, DocumentSubsetBitsetCache bitsetCache,
3837
Query roleQuery) throws IOException {
39-
return new DocumentSubsetDirectoryReader(in, bitsetFilterCache, roleQuery);
38+
return new DocumentSubsetDirectoryReader(in, bitsetCache, roleQuery);
4039
}
4140

4241
/**
@@ -110,29 +109,29 @@ private static int getNumDocs(LeafReader reader, Query roleQuery, BitSet roleQue
110109
public static final class DocumentSubsetDirectoryReader extends FilterDirectoryReader {
111110

112111
private final Query roleQuery;
113-
private final BitsetFilterCache bitsetFilterCache;
112+
private final DocumentSubsetBitsetCache bitsetCache;
114113

115-
DocumentSubsetDirectoryReader(final DirectoryReader in, final BitsetFilterCache bitsetFilterCache, final Query roleQuery)
116-
throws IOException {
114+
DocumentSubsetDirectoryReader(final DirectoryReader in, final DocumentSubsetBitsetCache bitsetCache,
115+
final Query roleQuery) throws IOException {
117116
super(in, new SubReaderWrapper() {
118117
@Override
119118
public LeafReader wrap(LeafReader reader) {
120119
try {
121-
return new DocumentSubsetReader(reader, bitsetFilterCache, roleQuery);
120+
return new DocumentSubsetReader(reader, bitsetCache, roleQuery);
122121
} catch (Exception e) {
123122
throw ExceptionsHelper.convertToElastic(e);
124123
}
125124
}
126125
});
127-
this.bitsetFilterCache = bitsetFilterCache;
126+
this.bitsetCache = bitsetCache;
128127
this.roleQuery = roleQuery;
129128

130129
verifyNoOtherDocumentSubsetDirectoryReaderIsWrapped(in);
131130
}
132131

133132
@Override
134133
protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOException {
135-
return new DocumentSubsetDirectoryReader(in, bitsetFilterCache, roleQuery);
134+
return new DocumentSubsetDirectoryReader(in, bitsetCache, roleQuery);
136135
}
137136

138137
private static void verifyNoOtherDocumentSubsetDirectoryReaderIsWrapped(DirectoryReader reader) {
@@ -156,9 +155,9 @@ public CacheHelper getReaderCacheHelper() {
156155
private final BitSet roleQueryBits;
157156
private final int numDocs;
158157

159-
private DocumentSubsetReader(final LeafReader in, BitsetFilterCache bitsetFilterCache, final Query roleQuery) throws Exception {
158+
private DocumentSubsetReader(final LeafReader in, DocumentSubsetBitsetCache bitsetCache, final Query roleQuery) throws Exception {
160159
super(in);
161-
this.roleQueryBits = bitsetFilterCache.getBitSetProducer(roleQuery).getBitSet(in.getContext());
160+
this.roleQueryBits = bitsetCache.getBitSet(roleQuery, in.getContext());
162161
this.numDocs = getNumDocs(in, roleQuery, roleQueryBits);
163162
}
164163

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapper.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.elasticsearch.common.CheckedFunction;
1515
import org.elasticsearch.common.logging.LoggerMessageFormat;
1616
import org.elasticsearch.common.util.concurrent.ThreadContext;
17-
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
1817
import org.elasticsearch.index.query.QueryShardContext;
1918
import org.elasticsearch.index.shard.ShardId;
2019
import org.elasticsearch.index.shard.ShardUtils;
@@ -44,17 +43,17 @@ public class SecurityIndexReaderWrapper implements CheckedFunction<DirectoryRead
4443
private static final Logger logger = LogManager.getLogger(SecurityIndexReaderWrapper.class);
4544

4645
private final Function<ShardId, QueryShardContext> queryShardContextProvider;
47-
private final BitsetFilterCache bitsetFilterCache;
46+
private final DocumentSubsetBitsetCache bitsetCache;
4847
private final XPackLicenseState licenseState;
4948
private final ThreadContext threadContext;
5049
private final ScriptService scriptService;
5150

5251
public SecurityIndexReaderWrapper(Function<ShardId, QueryShardContext> queryShardContextProvider,
53-
BitsetFilterCache bitsetFilterCache, ThreadContext threadContext, XPackLicenseState licenseState,
52+
DocumentSubsetBitsetCache bitsetCache, ThreadContext threadContext, XPackLicenseState licenseState,
5453
ScriptService scriptService) {
5554
this.scriptService = scriptService;
5655
this.queryShardContextProvider = queryShardContextProvider;
57-
this.bitsetFilterCache = bitsetFilterCache;
56+
this.bitsetCache = bitsetCache;
5857
this.threadContext = threadContext;
5958
this.licenseState = licenseState;
6059
}
@@ -84,7 +83,7 @@ public DirectoryReader apply(final DirectoryReader reader) {
8483
if (documentPermissions != null && documentPermissions.hasDocumentLevelPermissions()) {
8584
BooleanQuery filterQuery = documentPermissions.filter(getUser(), scriptService, shardId, queryShardContextProvider);
8685
if (filterQuery != null) {
87-
wrappedReader = DocumentSubsetReader.wrap(wrappedReader, bitsetFilterCache, new ConstantScoreQuery(filterQuery));
86+
wrappedReader = DocumentSubsetReader.wrap(wrappedReader, bitsetCache, new ConstantScoreQuery(filterQuery));
8887
}
8988
}
9089

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.core.security.support;
8+
9+
import org.elasticsearch.common.cache.Cache;
10+
import org.elasticsearch.common.util.concurrent.ReleasableLock;
11+
12+
import java.util.Iterator;
13+
import java.util.concurrent.locks.ReadWriteLock;
14+
import java.util.concurrent.locks.ReentrantReadWriteLock;
15+
import java.util.function.Predicate;
16+
17+
/**
18+
* A utility class to facilitate iterating over (and modifying) a {@link org.elasticsearch.common.cache.Cache}.
19+
* The semantics of the cache are such that when iterating (with the potential to call {@link Iterator#remove()}), we must prevent any
20+
* other modifications.
21+
* This class provides the necessary methods to support this constraint in a clear manner.
22+
*/
23+
public class CacheIteratorHelper<K, V> {
24+
private final Cache<K, V> cache;
25+
private final ReleasableLock updateLock;
26+
private final ReleasableLock iteratorLock;
27+
28+
public CacheIteratorHelper(Cache<K, V> cache) {
29+
this.cache = cache;
30+
final ReadWriteLock lock = new ReentrantReadWriteLock();
31+
// the lock is used in an odd manner; when iterating over the cache we cannot have modifiers other than deletes using the
32+
// iterator but when not iterating we can modify the cache without external locking. When making normal modifications to the cache
33+
// the read lock is obtained so that we can allow concurrent modifications; however when we need to iterate over the keys or values
34+
// of the cache the write lock must obtained to prevent any modifications.
35+
updateLock = new ReleasableLock(lock.readLock());
36+
iteratorLock = new ReleasableLock(lock.writeLock());
37+
}
38+
39+
public ReleasableLock acquireUpdateLock() {
40+
return updateLock.acquire();
41+
}
42+
43+
private ReleasableLock acquireForIterator() {
44+
return iteratorLock.acquire();
45+
}
46+
47+
public void removeKeysIf(Predicate<K> removeIf) {
48+
// the cache cannot be modified while doing this operation per the terms of the cache iterator
49+
try (ReleasableLock ignored = this.acquireForIterator()) {
50+
Iterator<K> iterator = cache.keys().iterator();
51+
while (iterator.hasNext()) {
52+
K key = iterator.next();
53+
if (removeIf.test(key)) {
54+
iterator.remove();
55+
}
56+
}
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)