WIP on FLS and Field Masking optimization

src/integrationTest/java/org/opensearch/security/FlsAndFieldMaskingLogsTests.java

@@ -0,0 +1,136 @@
+/*
+* Copyright OpenSearch Contributors
+* SPDX-License-Identifier: Apache-2.0
+*
+* The OpenSearch Contributors require contributions made to
+* this file be licensed under the Apache-2.0 license or a
+* compatible open source license.
+*
+*/
+package org.opensearch.security;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.opensearch.action.admin.indices.create.CreateIndexRequest;
+import org.opensearch.action.index.IndexRequest;
+import org.opensearch.action.index.IndexResponse;
+import org.opensearch.action.search.SearchRequest;
+import org.opensearch.action.search.SearchResponse;
+import org.opensearch.client.Client;
+import org.opensearch.client.RestHighLevelClient;
+import org.opensearch.plugin.mapper.MapperSizePlugin;
+import org.opensearch.test.framework.TestSecurityConfig;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.log.LogsRule;
+
+import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
+import static org.opensearch.client.RequestOptions.DEFAULT;
+import static org.opensearch.security.FlsAndFieldMaskingTests.assertSearchHitsDoNotContainField;
+import static org.opensearch.security.Song.FIELD_STARS;
+import static org.opensearch.security.Song.SONGS;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+import static org.opensearch.test.framework.cluster.SearchRequestFactory.searchRequestWithScroll;
+
+@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class FlsAndFieldMaskingLogsTests {
+
+    static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS);
+    static final TestSecurityConfig.Role FLS_ROLE = new TestSecurityConfig.Role("fls_exclude_stars_reader").clusterPermissions(
+        "cluster_composite_ops_ro"
+    ).indexPermissions("read").fls("~".concat(FIELD_STARS)).on("*");
+    static final TestSecurityConfig.User FLS_USER = new TestSecurityConfig.User("fls_user").roles(FLS_ROLE);
+
+    @ClassRule
+    public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
+        .anonymousAuth(false)
+        .nodeSettings(
+            Map.of("plugins.security.restapi.roles_enabled", List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()))
+        )
+        .plugin(MapperSizePlugin.class)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(ADMIN_USER, FLS_USER)
+        .build();
+
+    private static List<String> createIndexWithDocs(String indexName, Song... songs) {
+        try (Client client = cluster.getInternalNodeClient()) {
+            client.admin()
+                .indices()
+                .create(new CreateIndexRequest(indexName).mapping(Map.of("_size", Map.of("enabled", true))))
+                .actionGet();
+            return Stream.of(songs).map(song -> {
+                IndexResponse response = client.index(new IndexRequest(indexName).setRefreshPolicy(IMMEDIATE).source(song.asMap()))
+                    .actionGet();
+                return response.getId();
+            }).collect(Collectors.toList());
+        }
+    }
+
+    @Rule
+    public LogsRule valveLogsRule = new LogsRule("org.opensearch.security.configuration.DlsFlsValveImpl");
+
+    @BeforeClass
+    public static void createTestData() {
+        createIndexWithDocs("fls_index", SONGS[0], SONGS[1]);
+        createIndexWithDocs("fls_index_2", SONGS[0], SONGS[1]);
+        createIndexWithDocs("other_index", SONGS[0], SONGS[1]);
+    }
+
+    @Test
+    public void testFilteredFlsDlsConfigOnConcreteIndex() throws IOException {
+        String indexName = "fls_index";
+        try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(FLS_USER)) {
+            // search
+            SearchResponse searchResponse = restHighLevelClient.search(new SearchRequest(indexName), DEFAULT);
+
+            valveLogsRule.assertThatContainExactly(
+                "Filtered DLS/FLS Config: EvaluatedDlsFlsConfig [dlsQueriesByIndex={}, flsByIndex={fls_index=[~stars]}, fieldMaskingByIndex={}]"
+            );
+
+            assertSearchHitsDoNotContainField(searchResponse, FIELD_STARS);
+        }
+    }
+
+    @Test
+    public void testFilteredFlsDlsConfigOnIndexPattern() throws IOException {
+        try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(FLS_USER)) {
+            // search with index pattern
+            SearchResponse searchResponse = restHighLevelClient.search(new SearchRequest("fls_index*"), DEFAULT);
+
+            valveLogsRule.assertThatContainExactly(
+                "Filtered DLS/FLS Config: EvaluatedDlsFlsConfig [dlsQueriesByIndex={}, flsByIndex={fls_index=[~stars], fls_index_2=[~stars]}, fieldMaskingByIndex={}]"
+            );
+
+            assertSearchHitsDoNotContainField(searchResponse, FIELD_STARS);
+        }
+    }
+
+    @Test
+    public void testFilteredFlsDlsConfigWithScroll() throws IOException {
+        String indexName = "fls_index";
+        try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(FLS_USER)) {
+            // scroll
+            SearchResponse searchResponse = restHighLevelClient.search(searchRequestWithScroll(indexName, 1), DEFAULT);
+
+            valveLogsRule.assertThatContainExactly(
+                "Filtered DLS/FLS Config: EvaluatedDlsFlsConfig [dlsQueriesByIndex={}, flsByIndex={fls_index=[~stars]}, fieldMaskingByIndex={}]"
+            );
+
+            assertSearchHitsDoNotContainField(searchResponse, FIELD_STARS);
+        }
+    }
+
+}

src/integrationTest/java/org/opensearch/security/FlsAndFieldMaskingTests.java

@@ -560,7 +560,7 @@ private static TestSecurityConfig.User createUserWithRole(String userName, TestS
         return user;
     }
 
-    private static void assertSearchHitsDoNotContainField(SearchResponse response, String excludedField) {
+    static void assertSearchHitsDoNotContainField(SearchResponse response, String excludedField) {
         assertThat(response, isSuccessfulSearchResponse());
         assertThat(response.getHits().getHits().length, greaterThan(0));
         IntStream.range(0, response.getHits().getHits().length)

src/integrationTest/resources/log4j2-test.properties

@@ -34,6 +34,10 @@ logger.securityflsdlsindexsearcherwrapper.name = org.opensearch.security.configu
 logger.securityflsdlsindexsearcherwrapper.level = debug
 logger.securityflsdlsindexsearcherwrapper.appenderRef.capturing.ref = logCapturingAppender
 
+logger.dlsflsvalveimpl.name = org.opensearch.security.configuration.DlsFlsValveImpl
+logger.dlsflsvalveimpl.level = debug
+logger.dlsflsvalveimpl.appenderRef.capturing.ref = logCapturingAppender
+
 #Required by tests:
 # org.opensearch.security.IpBruteForceAttacksPreventionTests
 # org.opensearch.security.UserBruteForceAttacksPreventionTests

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -1095,7 +1095,8 @@ public Collection<Object> createComponents(
                 clusterService,
                 resolver,
                 xContentRegistry,
-                threadPool.getThreadContext()
+                threadPool.getThreadContext(),
+                cih
             );
             auditLog = new AuditLogImpl(settings, configPath, localClient, threadPool, resolver, clusterService, environment);
             privilegesInterceptor = new PrivilegesInterceptorImpl(resolver, clusterService, localClient, threadPool);

src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java

@@ -90,6 +90,17 @@ public Boolean hasNode(DiscoveryNode node) {
         return nodes.nodeExists(node) ? Boolean.TRUE : Boolean.FALSE;
     }
 
+    public Version getMinimumNodeVersion() {
+        if (nodes == null) {
+            if (log.isDebugEnabled()) {
+                log.debug("Cluster Info Holder not initialized yet for 'nodes'");
+            }
+            return null;
+        }
+
+        return nodes.getMinNodeVersion();
+    }
+
     public String getClusterName() {
         return this.clusterName;
     }

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

@@ -34,6 +34,7 @@
 import org.opensearch.OpenSearchException;
 import org.opensearch.OpenSearchSecurityException;
 import org.opensearch.SpecialPermission;
+import org.opensearch.Version;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.DocWriteRequest;
 import org.opensearch.action.RealtimeRequest;
@@ -98,14 +99,16 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve {
     private final boolean dfmEmptyOverwritesAll;
     private final NamedXContentRegistry namedXContentRegistry;
     private volatile ConfigModel configModel;
+    private final ClusterInfoHolder clusterInfoHolder;
 
     public DlsFlsValveImpl(
         Settings settings,
         Client nodeClient,
         ClusterService clusterService,
         IndexNameExpressionResolver resolver,
         NamedXContentRegistry namedXContentRegistry,
-        ThreadContext threadContext
+        ThreadContext threadContext,
+        ClusterInfoHolder clusterInfoHolder
     ) {
         super();
         this.nodeClient = nodeClient;
@@ -116,6 +119,7 @@ public DlsFlsValveImpl(
         this.dlsQueryParser = new DlsQueryParser(namedXContentRegistry);
         this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false);
         this.namedXContentRegistry = namedXContentRegistry;
+        this.clusterInfoHolder = clusterInfoHolder;
     }
 
     @Subscribe
@@ -189,11 +193,20 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<
             }
         }
 
+        EvaluatedDlsFlsConfig dlsFlsConfigHeaders = filteredDlsFlsConfig;
+        if (clusterInfoHolder.getMinimumNodeVersion().onOrBefore(Version.V_2_17_0)) {
+            dlsFlsConfigHeaders = evaluatedDlsFlsConfig;
+        }
+
         if (!doFilterLevelDls) {
-            setDlsHeaders(evaluatedDlsFlsConfig, request);
+            setDlsHeaders(dlsFlsConfigHeaders, request);
         }
 
-        setFlsHeaders(evaluatedDlsFlsConfig, request);
+        setFlsHeaders(dlsFlsConfigHeaders, request);
+
+        if (log.isDebugEnabled()) {
+            log.debug("Filtered DLS/FLS Config: " + filteredDlsFlsConfig);
+        }
 
         if (filteredDlsFlsConfig.isEmpty()) {
             return true;
@@ -504,6 +517,7 @@ private Mode getDlsModeHeader() {
         }
     }
 
+    @SuppressWarnings("unchecked")
     private void setFlsHeaders(EvaluatedDlsFlsConfig dlsFls, ActionRequest request) {
         if (!dlsFls.getFieldMaskingByIndex().isEmpty()) {
             Map<String, Set<String>> maskedFieldsMap = dlsFls.getFieldMaskingByIndex();
@@ -519,19 +533,12 @@ private void setFlsHeaders(EvaluatedDlsFlsConfig dlsFls, ActionRequest request)
             } else {
 
                 if (threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER) != null) {
-                    if (!maskedFieldsMap.equals(
-                        Base64Helper.deserializeObject(
-                            threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER),
-                            threadContext.getTransient(ConfigConstants.USE_JDK_SERIALIZATION)
-                        )
-                    )) {
-                        throw new OpenSearchSecurityException(
-                            ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER + " does not match (SG 901D)"
-                        );
-                    } else {
-                        if (log.isDebugEnabled()) {
-                            log.debug(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER + " already set");
-                        }
+                    Map<String, Set<String>> deserializedMap = (Map<String, Set<String>>) Base64Helper.deserializeObject(
+                        threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER),
+                        threadContext.getTransient(ConfigConstants.USE_JDK_SERIALIZATION)
+                    );
+                    if (log.isDebugEnabled()) {
+                        log.debug(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER + " already set");
                     }
                 } else {
                     threadContext.putHeader(
@@ -558,26 +565,12 @@ private void setFlsHeaders(EvaluatedDlsFlsConfig dlsFls, ActionRequest request)
                 }
             } else {
                 if (threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER) != null) {
-                    if (!flsFields.equals(
-                        Base64Helper.deserializeObject(
-                            threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER),
-                            threadContext.getTransient(ConfigConstants.USE_JDK_SERIALIZATION)
-                        )
-                    )) {
-                        throw new OpenSearchSecurityException(
-                            ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER
-                                + " does not match (SG 901D) "
-                                + flsFields
-                                + "---"
-                                + Base64Helper.deserializeObject(
-                                    threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER),
-                                    threadContext.getTransient(ConfigConstants.USE_JDK_SERIALIZATION)
-                                )
-                        );
-                    } else {
-                        if (log.isDebugEnabled()) {
-                            log.debug(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER + " already set");
-                        }
+                    Map<String, Set<String>> deserializedMap = (Map<String, Set<String>>) Base64Helper.deserializeObject(
+                        threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER),
+                        threadContext.getTransient(ConfigConstants.USE_JDK_SERIALIZATION)
+                    );
+                    if (log.isDebugEnabled()) {
+                        log.debug(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER + " already set");
                     }
                 } else {
                     threadContext.putHeader(

src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java

@@ -309,7 +309,7 @@ private void resolveIndexPatterns(
                 final boolean isDebugEnabled = log.isDebugEnabled();
                 try {
                     matchingAllIndices = Arrays.asList(
-                        resolver.concreteIndexNames(state, indicesOptions, localRequestedPatterns.toArray(new String[0]))
+                        resolver.concreteIndexNames(state, indicesOptions, true, localRequestedPatterns.toArray(new String[0]))
                     );
                     matchingDataStreams = resolver.dataStreamNames(state, indicesOptions, localRequestedPatterns.toArray(new String[0]));