Skip to content

Commit 3e378b2

Browse files
jasontedorjasontedor
committed
Allow query caching by default again
With the introduction of the default distribution, it means that by default the query cache is wrapped in the security implementation of the query cache. This cache does not allow caching if the request does not carry indices permissions. Yet, this will not happen if authorization is not allowed, which it is not by default. This means that with the introduction of the default distribution, query caching was disabled by default! This commit addresses this by checking if authorization is allowed and if not, delegating to the default indices query cache. Otherwise, we proceed as before with security. Additionally, we clear the cache on license state changes.
1 parent b7a63f7 commit 3e378b2

File tree

3 files changed

+97
-6
lines changed

3 files changed

+97
-6
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,6 @@
121121
import org.elasticsearch.xpack.core.security.authz.accesscontrol.SecurityIndexSearcherWrapper;
122122
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
123123
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
124-
import org.elasticsearch.xpack.security.action.privilege.TransportDeletePrivilegesAction;
125-
import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesAction;
126-
import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction;
127124
import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
128125
import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField;
129126
import org.elasticsearch.xpack.core.security.support.Automatons;
@@ -143,6 +140,9 @@
143140
import org.elasticsearch.xpack.security.action.interceptor.ResizeRequestInterceptor;
144141
import org.elasticsearch.xpack.security.action.interceptor.SearchRequestInterceptor;
145142
import org.elasticsearch.xpack.security.action.interceptor.UpdateRequestInterceptor;
143+
import org.elasticsearch.xpack.security.action.privilege.TransportDeletePrivilegesAction;
144+
import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesAction;
145+
import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction;
146146
import org.elasticsearch.xpack.security.action.realm.TransportClearRealmCacheAction;
147147
import org.elasticsearch.xpack.security.action.role.TransportClearRolesCacheAction;
148148
import org.elasticsearch.xpack.security.action.role.TransportDeleteRoleAction;
@@ -181,8 +181,8 @@
181181
import org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener;
182182
import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache;
183183
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
184-
import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
185184
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
185+
import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
186186
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
187187
import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor;
188188
import org.elasticsearch.xpack.security.rest.SecurityRestFilter;
@@ -672,7 +672,8 @@ public void onIndexModule(IndexModule module) {
672672
* This impl. disabled the query cache if field level security is used for a particular request. If we wouldn't do
673673
* forcefully overwrite the query cache implementation then we leave the system vulnerable to leakages of data to
674674
* unauthorized users. */
675-
module.forceQueryCacheProvider((settings, cache) -> new OptOutQueryCache(settings, cache, threadContext.get()));
675+
module.forceQueryCacheProvider(
676+
(settings, cache) -> new OptOutQueryCache(settings, cache, threadContext.get(), getLicenseState()));
676677
}
677678

678679
// in order to prevent scroll ids from being maliciously crafted and/or guessed, a listener is added that

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* or more contributor license agreements. Licensed under the Elastic License;
44
* you may not use this file except in compliance with the Elastic License.
55
*/
6+
67
package org.elasticsearch.xpack.security.authz.accesscontrol;
78

89
import org.apache.lucene.search.QueryCachingPolicy;
@@ -13,6 +14,7 @@
1314
import org.elasticsearch.index.IndexSettings;
1415
import org.elasticsearch.index.cache.query.QueryCache;
1516
import org.elasticsearch.indices.IndicesQueryCache;
17+
import org.elasticsearch.license.XPackLicenseState;
1618
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
1719
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
1820

@@ -29,12 +31,19 @@ public final class OptOutQueryCache extends AbstractIndexComponent implements Qu
2931
private final IndicesQueryCache indicesQueryCache;
3032
private final ThreadContext context;
3133
private final String indexName;
34+
private final XPackLicenseState licenseState;
3235

33-
public OptOutQueryCache(IndexSettings indexSettings, IndicesQueryCache indicesQueryCache, ThreadContext context) {
36+
public OptOutQueryCache(
37+
final IndexSettings indexSettings,
38+
final IndicesQueryCache indicesQueryCache,
39+
final ThreadContext context,
40+
final XPackLicenseState licenseState) {
3441
super(indexSettings);
3542
this.indicesQueryCache = indicesQueryCache;
3643
this.context = Objects.requireNonNull(context, "threadContext must not be null");
3744
this.indexName = indexSettings.getIndex().getName();
45+
this.licenseState = Objects.requireNonNull(licenseState, "licenseState");
46+
licenseState.addListener(() -> this.clear("license state changed"));
3847
}
3948

4049
@Override
@@ -50,6 +59,11 @@ public void clear(String reason) {
5059

5160
@Override
5261
public Weight doCache(Weight weight, QueryCachingPolicy policy) {
62+
if (licenseState.isAuthAllowed() == false) {
63+
logger.debug("not opting out of the query cache; authorization is not allowed");
64+
return indicesQueryCache.doCache(weight, policy);
65+
}
66+
5367
IndicesAccessControl indicesAccessControl = context.getTransient(
5468
AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
5569
if (indicesAccessControl == null) {
@@ -96,4 +110,5 @@ static boolean cachingIsSafe(Weight weight, IndicesAccessControl.IndexAccessCont
96110
// we can cache, all fields are ok
97111
return true;
98112
}
113+
99114
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,19 @@
1111
import org.apache.lucene.search.BooleanClause;
1212
import org.apache.lucene.search.BooleanQuery;
1313
import org.apache.lucene.search.IndexSearcher;
14+
import org.apache.lucene.search.QueryCachingPolicy;
1415
import org.apache.lucene.search.TermQuery;
1516
import org.apache.lucene.search.Weight;
1617
import org.apache.lucene.store.Directory;
18+
import org.elasticsearch.Version;
19+
import org.elasticsearch.cluster.metadata.IndexMetaData;
20+
import org.elasticsearch.common.settings.Settings;
21+
import org.elasticsearch.common.util.concurrent.ThreadContext;
22+
import org.elasticsearch.index.IndexSettings;
23+
import org.elasticsearch.indices.IndicesQueryCache;
24+
import org.elasticsearch.license.XPackLicenseState;
1725
import org.elasticsearch.test.ESTestCase;
26+
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
1827
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
1928
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
2029
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition;
@@ -24,6 +33,12 @@
2433
import java.io.IOException;
2534
import java.util.HashSet;
2635

36+
import static org.mockito.Matchers.same;
37+
import static org.mockito.Mockito.mock;
38+
import static org.mockito.Mockito.verify;
39+
import static org.mockito.Mockito.verifyNoMoreInteractions;
40+
import static org.mockito.Mockito.when;
41+
2742
/** Simple tests for opt out query cache*/
2843
public class OptOutQueryCacheTests extends ESTestCase {
2944
IndexSearcher searcher;
@@ -107,6 +122,66 @@ public void testOptOutQueryCacheSafetyCheck() throws IOException {
107122
assertFalse(OptOutQueryCache.cachingIsSafe(weight, permissions));
108123
}
109124

125+
public void testOptOutQueryCacheAuthIsNotAllowed() {
126+
final Settings.Builder settings = Settings.builder()
127+
.put("index.version.created", Version.CURRENT)
128+
.put("index.number_of_shards", 1)
129+
.put("index.number_of_replicas", 0);
130+
final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build();
131+
final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
132+
final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class);
133+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
134+
final XPackLicenseState licenseState = mock(XPackLicenseState.class);
135+
when(licenseState.isAuthAllowed()).thenReturn(false);
136+
final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState);
137+
final Weight weight = mock(Weight.class);
138+
final QueryCachingPolicy policy = mock(QueryCachingPolicy.class);
139+
cache.doCache(weight, policy);
140+
verify(indicesQueryCache).doCache(same(weight), same(policy));
141+
}
142+
143+
public void testOptOutQueryCacheNoIndicesPermissions() {
144+
final Settings.Builder settings = Settings.builder()
145+
.put("index.version.created", Version.CURRENT)
146+
.put("index.number_of_shards", 1)
147+
.put("index.number_of_replicas", 0);
148+
final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build();
149+
final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
150+
final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class);
151+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
152+
final XPackLicenseState licenseState = mock(XPackLicenseState.class);
153+
when(licenseState.isAuthAllowed()).thenReturn(true);
154+
final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState);
155+
final Weight weight = mock(Weight.class);
156+
final QueryCachingPolicy policy = mock(QueryCachingPolicy.class);
157+
final Weight w = cache.doCache(weight, policy);
158+
assertSame(w, weight);
159+
verifyNoMoreInteractions(indicesQueryCache);
160+
}
161+
162+
public void testOptOutQueryCacheIndexDoesNotHaveFieldLevelSecurity() {
163+
final Settings.Builder settings = Settings.builder()
164+
.put("index.version.created", Version.CURRENT)
165+
.put("index.number_of_shards", 1)
166+
.put("index.number_of_replicas", 0);
167+
final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build();
168+
final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
169+
final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class);
170+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
171+
final IndicesAccessControl.IndexAccessControl indexAccessControl = mock(IndicesAccessControl.IndexAccessControl.class);
172+
when(indexAccessControl.getFieldPermissions()).thenReturn(new FieldPermissions());
173+
final IndicesAccessControl indicesAccessControl = mock(IndicesAccessControl.class);
174+
when(indicesAccessControl.getIndexPermissions("index")).thenReturn(indexAccessControl);
175+
threadContext.putTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY, indicesAccessControl);
176+
final XPackLicenseState licenseState = mock(XPackLicenseState.class);
177+
when(licenseState.isAuthAllowed()).thenReturn(true);
178+
final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState);
179+
final Weight weight = mock(Weight.class);
180+
final QueryCachingPolicy policy = mock(QueryCachingPolicy.class);
181+
cache.doCache(weight, policy);
182+
verify(indicesQueryCache).doCache(same(weight), same(policy));
183+
}
184+
110185
private static FieldPermissionsDefinition fieldPermissionDef(String[] granted, String[] denied) {
111186
return new FieldPermissionsDefinition(granted, denied);
112187
}

0 commit comments

Comments
 (0)