Fix bug where admin can read system index

Signed-off-by: Derek Ho <[email protected]>
derek-ho · Oct 3, 2024 · c0efd45 · c0efd45
1 parent 830b341
commit c0efd45
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
@@ -80,4 +80,29 @@ public void adminShouldNotBeAbleToDeleteSecurityIndex() {
             assertThat(response4.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
         }
     }
+
+    @Test
+    public void adminShouldNotBeAbleToReadSecurityIndex() {
+        // Create system index and index a dummy document as the super admin user, data returned to super admin
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
+            HttpResponse response1 = client.put(".system-index1");
+
+            assertThat(response1.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
+            String doc = "{\"field\":\"value\"}";
+            HttpResponse adminPostResponse = client.postJson(".system-index1/_doc/1?refresh=true", doc);
+            assertThat(adminPostResponse.getStatusCode(), equalTo(RestStatus.CREATED.getStatus()));
+            HttpResponse response2 = client.get(".system-index1/_search");
+
+            assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
+            assertThat(response2.getBody(), response2.getBody().contains("\"hits\":{\"total\":{\"value\":1,\"relation\":\"eq\"}"));
+        }
+
+        // Regular users should not be able to read it
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+            // regular user cannot read system index
+            HttpResponse response1 = client.get(".system-index1/_search");
+
+            assertThat(response1.getBody(), response1.getBody().contains("\"hits\":{\"total\":{\"value\":0,\"relation\":\"eq\"}"));
+        }
+    }
 }
diff --git a/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
@@ -39,6 +39,7 @@
 import org.opensearch.core.common.transport.TransportAddress;
 import org.opensearch.core.index.Index;
 import org.opensearch.index.IndexService;
+import org.opensearch.indices.SystemIndexRegistry;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.ConfigModel;
 import org.opensearch.security.securityconf.SecurityRoles;
@@ -152,7 +153,8 @@ protected final boolean isBlockedProtectedIndexRequest() {
     }
 
     protected final boolean isBlockedSystemIndexRequest() {
-        boolean isSystemIndex = systemIndexMatcher.test(index.getName());
+        boolean matchesSystemIndexRegisteredWithCore = !SystemIndexRegistry.matchesSystemIndexPattern(Set.of(index.getName())).isEmpty();
+        boolean isSystemIndex = systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore;
         if (!isSystemIndex) {
             return false;
         }
@@ -161,7 +163,7 @@ protected final boolean isBlockedSystemIndexRequest() {
             final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
             if (user == null) {
                 // allow request without user from plugin.
-                return systemIndexMatcher.test(index.getName());
+                return systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore;
             }
             final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
             final Set<String> mappedRoles = evaluator.mapRoles(user, caller);