From bfa9199efcf327d348c6f028310a215b10a535d0 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 4 Sep 2018 17:50:31 -0400 Subject: [PATCH] Allow query caching by default again (#33328) 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. --- .../xpack/security/Security.java | 11 ++- .../authz/accesscontrol/OptOutQueryCache.java | 18 +++- .../accesscontrol/OptOutQueryCacheTests.java | 97 +++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 347f5b620781b..532a499e7b8bd 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -123,9 +123,6 @@ import org.elasticsearch.xpack.core.security.authz.accesscontrol.SecurityIndexSearcherWrapper; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; -import org.elasticsearch.xpack.security.action.privilege.TransportDeletePrivilegesAction; -import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesAction; -import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -145,6 +142,9 @@ import org.elasticsearch.xpack.security.action.interceptor.ResizeRequestInterceptor; import org.elasticsearch.xpack.security.action.interceptor.SearchRequestInterceptor; import org.elasticsearch.xpack.security.action.interceptor.UpdateRequestInterceptor; +import org.elasticsearch.xpack.security.action.privilege.TransportDeletePrivilegesAction; +import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesAction; +import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction; import org.elasticsearch.xpack.security.action.realm.TransportClearRealmCacheAction; import org.elasticsearch.xpack.security.action.role.TransportClearRolesCacheAction; import org.elasticsearch.xpack.security.action.role.TransportDeleteRoleAction; @@ -183,8 +183,8 @@ import org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener; import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache; import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore; -import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.elasticsearch.xpack.security.authz.store.FileRolesStore; +import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor; import org.elasticsearch.xpack.security.rest.SecurityRestFilter; @@ -698,7 +698,8 @@ public void onIndexModule(IndexModule module) { * This impl. disabled the query cache if field level security is used for a particular request. If we wouldn't do * forcefully overwrite the query cache implementation then we leave the system vulnerable to leakages of data to * unauthorized users. */ - module.forceQueryCacheProvider((settings, cache) -> new OptOutQueryCache(settings, cache, threadContext.get())); + module.forceQueryCacheProvider( + (settings, cache) -> new OptOutQueryCache(settings, cache, threadContext.get(), getLicenseState())); } // in order to prevent scroll ids from being maliciously crafted and/or guessed, a listener is added that diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java index e15ff2f4d0c67..a49bfdfbe1666 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java @@ -3,6 +3,7 @@ * 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.QueryCachingPolicy; @@ -13,6 +14,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.cache.query.QueryCache; import org.elasticsearch.indices.IndicesQueryCache; +import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; @@ -29,12 +31,19 @@ public final class OptOutQueryCache extends AbstractIndexComponent implements Qu private final IndicesQueryCache indicesQueryCache; private final ThreadContext context; private final String indexName; + private final XPackLicenseState licenseState; - public OptOutQueryCache(IndexSettings indexSettings, IndicesQueryCache indicesQueryCache, ThreadContext context) { + public OptOutQueryCache( + final IndexSettings indexSettings, + final IndicesQueryCache indicesQueryCache, + final ThreadContext context, + final XPackLicenseState licenseState) { super(indexSettings); this.indicesQueryCache = indicesQueryCache; this.context = Objects.requireNonNull(context, "threadContext must not be null"); this.indexName = indexSettings.getIndex().getName(); + this.licenseState = Objects.requireNonNull(licenseState, "licenseState"); + licenseState.addListener(() -> this.clear("license state changed")); } @Override @@ -50,6 +59,12 @@ public void clear(String reason) { @Override public Weight doCache(Weight weight, QueryCachingPolicy policy) { + // TODO: this is not concurrently safe since the license state can change between reads + if (licenseState.isSecurityEnabled() == false || licenseState.isAuthAllowed() == false) { + logger.debug("not opting out of the query cache; authorization is not allowed"); + return indicesQueryCache.doCache(weight, policy); + } + IndicesAccessControl indicesAccessControl = context.getTransient( AuthorizationServiceField.INDICES_PERMISSIONS_KEY); if (indicesAccessControl == null) { @@ -96,4 +111,5 @@ static boolean cachingIsSafe(Weight weight, IndicesAccessControl.IndexAccessCont // we can cache, all fields are ok return true; } + } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java index fe180c9c5ccee..1d6d524cbbb70 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java @@ -11,10 +11,19 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.QueryCachingPolicy; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.indices.IndicesQueryCache; +import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition; @@ -24,6 +33,12 @@ import java.io.IOException; import java.util.HashSet; +import static org.mockito.Matchers.same; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + /** Simple tests for opt out query cache*/ public class OptOutQueryCacheTests extends ESTestCase { IndexSearcher searcher; @@ -107,6 +122,88 @@ public void testOptOutQueryCacheSafetyCheck() throws IOException { assertFalse(OptOutQueryCache.cachingIsSafe(weight, permissions)); } + public void testOptOutQueryCacheSecurityIsNotEnabled() { + final Settings.Builder settings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0); + final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build(); + final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY); + final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + final XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.isSecurityEnabled()).thenReturn(false); + when(licenseState.isAuthAllowed()).thenReturn(randomBoolean()); + final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState); + final Weight weight = mock(Weight.class); + final QueryCachingPolicy policy = mock(QueryCachingPolicy.class); + cache.doCache(weight, policy); + verify(indicesQueryCache).doCache(same(weight), same(policy)); + } + + public void testOptOutQueryCacheAuthIsNotAllowed() { + final Settings.Builder settings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0); + final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build(); + final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY); + final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + final XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.isSecurityEnabled()).thenReturn(randomBoolean()); + when(licenseState.isAuthAllowed()).thenReturn(false); + final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState); + final Weight weight = mock(Weight.class); + final QueryCachingPolicy policy = mock(QueryCachingPolicy.class); + cache.doCache(weight, policy); + verify(indicesQueryCache).doCache(same(weight), same(policy)); + } + + public void testOptOutQueryCacheNoIndicesPermissions() { + final Settings.Builder settings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0); + final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build(); + final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY); + final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + final XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.isSecurityEnabled()).thenReturn(true); + when(licenseState.isAuthAllowed()).thenReturn(true); + final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState); + final Weight weight = mock(Weight.class); + final QueryCachingPolicy policy = mock(QueryCachingPolicy.class); + final Weight w = cache.doCache(weight, policy); + assertSame(w, weight); + verifyNoMoreInteractions(indicesQueryCache); + } + + public void testOptOutQueryCacheIndexDoesNotHaveFieldLevelSecurity() { + final Settings.Builder settings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0); + final IndexMetaData indexMetaData = IndexMetaData.builder("index").settings(settings).build(); + final IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY); + final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + final IndicesAccessControl.IndexAccessControl indexAccessControl = mock(IndicesAccessControl.IndexAccessControl.class); + when(indexAccessControl.getFieldPermissions()).thenReturn(new FieldPermissions()); + final IndicesAccessControl indicesAccessControl = mock(IndicesAccessControl.class); + when(indicesAccessControl.getIndexPermissions("index")).thenReturn(indexAccessControl); + threadContext.putTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY, indicesAccessControl); + final XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.isSecurityEnabled()).thenReturn(true); + when(licenseState.isAuthAllowed()).thenReturn(true); + final OptOutQueryCache cache = new OptOutQueryCache(indexSettings, indicesQueryCache, threadContext, licenseState); + final Weight weight = mock(Weight.class); + final QueryCachingPolicy policy = mock(QueryCachingPolicy.class); + cache.doCache(weight, policy); + verify(indicesQueryCache).doCache(same(weight), same(policy)); + } + private static FieldPermissionsDefinition fieldPermissionDef(String[] granted, String[] denied) { return new FieldPermissionsDefinition(granted, denied); }