Security: fix dynamic mapping updates with aliases (#30787)

jaymode · jaymode · commit 2b3370330817 · 2018-05-24T10:41:33.000-06:00
This commit fixes an issue with dynamic mapping updates when an index operation is performed against an alias and when the user only has permissions to the alias. Dynamic mapping updates resolve the concrete index early to prevent issues so the information about the alias that the triggering operation was being executed against is lost. When security is enabled and a user only has privileges to the alias, this dynamic mapping update would be rejected as it is executing against the concrete index and not the alias. In order to handle this situation, the security code needs to look at the concrete index and the authorized indices of the user; if the concrete index is not authorized the code will attempt to find an alias that the user has permissions to update the mappings of. Closes #30597
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java
@@ -14,11 +14,14 @@
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.support.IndicesOptions;
+import org.elasticsearch.cluster.metadata.AliasMetaData;
 import org.elasticsearch.cluster.metadata.AliasOrIndex;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -35,14 +38,15 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER;
 
-public class IndicesAndAliasesResolver {
+class IndicesAndAliasesResolver {
 
     //`*,-*` what we replace indices with if we need Elasticsearch to return empty responses without throwing exception
     private static final String[] NO_INDICES_ARRAY = new String[] { "*", "-*" };
@@ -51,7 +55,7 @@ public class IndicesAndAliasesResolver {
     private final IndexNameExpressionResolver nameExpressionResolver;
     private final RemoteClusterResolver remoteClusterResolver;
 
-    public IndicesAndAliasesResolver(Settings settings, ClusterService clusterService) {
+    IndicesAndAliasesResolver(Settings settings, ClusterService clusterService) {
         this.nameExpressionResolver = new IndexNameExpressionResolver(settings);
         this.remoteClusterResolver = new RemoteClusterResolver(settings, clusterService.getClusterSettings());
     }
@@ -85,7 +89,7 @@ public IndicesAndAliasesResolver(Settings settings, ClusterService clusterServic
      * Otherwise, <em>N</em> will be added to the <em>local</em> index list.
      */
 
-    public ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) {
+    ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) {
         if (request instanceof IndicesAliasesRequest) {
             ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
             IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request;
@@ -116,7 +120,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
              */
             assert indicesRequest.indices() == null || indicesRequest.indices().length == 0
                     : "indices are: " + Arrays.toString(indicesRequest.indices()); // Arrays.toString() can handle null values - all good
-            resolvedIndicesBuilder.addLocal(((PutMappingRequest) indicesRequest).getConcreteIndex().getName());
+            resolvedIndicesBuilder.addLocal(getPutMappingIndexOrAlias((PutMappingRequest) indicesRequest, authorizedIndices, metaData));
         } else if (indicesRequest instanceof IndicesRequest.Replaceable) {
             IndicesRequest.Replaceable replaceable = (IndicesRequest.Replaceable) indicesRequest;
             final boolean replaceWildcards = indicesRequest.indicesOptions().expandWildcardsOpen()
@@ -213,7 +217,48 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
         return resolvedIndicesBuilder.build();
     }
 
-    public static boolean allowsRemoteIndices(IndicesRequest request) {
+    /**
+     * Special handling of the value to authorize for a put mapping request. Dynamic put mapping
+     * requests use a concrete index, but we allow permissions to be defined on aliases so if the
+     * request's concrete index is not in the list of authorized indices, then we need to look to
+     * see if this can be authorized against an alias
+     */
+    static String getPutMappingIndexOrAlias(PutMappingRequest request, AuthorizedIndices authorizedIndices, MetaData metaData) {
+        final String concreteIndexName = request.getConcreteIndex().getName();
+        final List<String> authorizedIndicesList = authorizedIndices.get();
+
+        // validate that the concrete index exists, otherwise there is no remapping that we could do
+        final AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(concreteIndexName);
+        final String resolvedAliasOrIndex;
+        if (aliasOrIndex == null) {
+            resolvedAliasOrIndex = concreteIndexName;
+        } else if (aliasOrIndex.isAlias()) {
+            throw new IllegalStateException("concrete index [" + concreteIndexName + "] is an alias but should not be");
+        } else if (authorizedIndicesList.contains(concreteIndexName)) {
+            // user is authorized to put mappings for this index
+            resolvedAliasOrIndex = concreteIndexName;
+        } else {
+            // the user is not authorized to put mappings for this index, but could have been
+            // authorized for a write using an alias that triggered a dynamic mapping update
+            ImmutableOpenMap<String, List<AliasMetaData>> foundAliases =
+                metaData.findAliases(Strings.EMPTY_ARRAY, new String[] { concreteIndexName });
+            List<AliasMetaData> aliasMetaData = foundAliases.get(concreteIndexName);
+            if (aliasMetaData != null) {
+                Optional<String> foundAlias = aliasMetaData.stream()
+                    .map(AliasMetaData::alias)
+                    .filter(authorizedIndicesList::contains)
+                    .filter(aliasName -> metaData.getAliasAndIndexLookup().get(aliasName).getIndices().size() == 1)
+                    .findFirst();
+                resolvedAliasOrIndex = foundAlias.orElse(concreteIndexName);
+            } else {
+                resolvedAliasOrIndex = concreteIndexName;
+            }
+        }
+
+        return resolvedAliasOrIndex;
+    }
+
+    static boolean allowsRemoteIndices(IndicesRequest request) {
         return request instanceof SearchRequest || request instanceof FieldCapabilitiesRequest
                 || request instanceof GraphExploreRequest;
     }
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java
@@ -39,10 +39,12 @@
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.search.internal.ShardSearchTransportRequest;
 import org.elasticsearch.test.ESTestCase;
@@ -150,7 +152,10 @@ public void setup() {
                 new IndicesPrivileges[] { IndicesPrivileges.builder().indices(authorizedIndices).privileges("all").build() }, null));
         roleMap.put("dash", new RoleDescriptor("dash", null,
                 new IndicesPrivileges[] { IndicesPrivileges.builder().indices(dashIndices).privileges("all").build() }, null));
-        roleMap.put("test", new RoleDescriptor("role", new String[] { "monitor" }, null, null));
+        roleMap.put("test", new RoleDescriptor("test", new String[] { "monitor" }, null, null));
+        roleMap.put("alias_read_write", new RoleDescriptor("alias_read_write", null,
+            new IndicesPrivileges[] { IndicesPrivileges.builder().indices("barbaz", "foofoobar").privileges("read", "write").build() },
+            null));
         roleMap.put(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR);
         final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
         doAnswer((i) -> {
@@ -652,7 +657,7 @@ public void testResolveWildcardsIndicesAliasesRequestNoMatchingIndices() {
         request.addAliasAction(AliasActions.add().alias("alias2").index("bar*"));
         request.addAliasAction(AliasActions.add().alias("alias3").index("non_matching_*"));
         //if a single operation contains wildcards and ends up being resolved to no indices, it makes the whole request fail
-        expectThrows(IndexNotFoundException.class, 
+        expectThrows(IndexNotFoundException.class,
                 () -> resolveIndices(request, buildAuthorizedIndices(user, IndicesAliasesAction.NAME)));
     }
 
@@ -1181,10 +1186,10 @@ public void testIndicesExists() {
             assertNoIndices(request, resolveIndices(request,
                     buildAuthorizedIndices(userNoIndices, IndicesExistsAction.NAME)));
         }
-        
+
         {
             IndicesExistsRequest request = new IndicesExistsRequest("does_not_exist");
-            
+
             assertNoIndices(request, resolveIndices(request,
                     buildAuthorizedIndices(user, IndicesExistsAction.NAME)));
         }
@@ -1229,7 +1234,7 @@ public void testNonXPackUserAccessingSecurityIndex() {
             List<String> indices = resolveIndices(request, authorizedIndices).getLocal();
             assertThat(indices, not(hasItem(SecurityLifecycleServiceField.SECURITY_INDEX_NAME)));
         }
-        
+
         {
             IndicesAliasesRequest aliasesRequest = new IndicesAliasesRequest();
             aliasesRequest.addAliasAction(AliasActions.add().alias("security_alias1").index("*"));
@@ -1318,6 +1323,21 @@ public void testAliasDateMathExpressionNotSupported() {
         assertThat(request.aliases(), arrayContainingInAnyOrder("<datetime-{now/M}>"));
     }
 
+    public void testDynamicPutMappingRequestFromAlias() {
+        PutMappingRequest request = new PutMappingRequest(Strings.EMPTY_ARRAY).setConcreteIndex(new Index("foofoo", UUIDs.base64UUID()));
+        User user = new User("alias-writer", "alias_read_write");
+        AuthorizedIndices authorizedIndices = buildAuthorizedIndices(user, PutMappingAction.NAME);
+
+        String putMappingIndexOrAlias = IndicesAndAliasesResolver.getPutMappingIndexOrAlias(request, authorizedIndices, metaData);
+        assertEquals("barbaz", putMappingIndexOrAlias);
+
+        // multiple indices map to an alias so we can only return the concrete index
+        final String index = randomFrom("foo", "foobar");
+        request = new PutMappingRequest(Strings.EMPTY_ARRAY).setConcreteIndex(new Index(index, UUIDs.base64UUID()));
+        putMappingIndexOrAlias = IndicesAndAliasesResolver.getPutMappingIndexOrAlias(request, authorizedIndices, metaData);
+        assertEquals(index, putMappingIndexOrAlias);
+    }
+
     // TODO with the removal of DeleteByQuery is there another way to test resolving a write action?
 
 
diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/security/authz/30_dynamic_put_mapping.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/security/authz/30_dynamic_put_mapping.yml
@@ -0,0 +1,90 @@
+---
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+          wait_for_status: yellow
+
+  - do:
+      xpack.security.put_role:
+        name: "alias_write_role"
+        body:  >
+            {
+              "indices": [
+                { "names": ["write_alias"], "privileges": ["write"] }
+              ]
+            }
+
+  - do:
+      xpack.security.put_user:
+        username: "test_user"
+        body:  >
+          {
+            "password" : "x-pack-test-password",
+            "roles" : [ "alias_write_role" ],
+            "full_name" : "user with privileges to write via alias"
+          }
+
+  - do:
+      indices.create:
+        index: write_index_1
+        body:
+          settings:
+            index:
+              number_of_shards:   1
+              number_of_replicas: 0
+
+  - do:
+      indices.put_alias:
+        index: write_index_1
+        name: write_alias
+
+---
+teardown:
+  - do:
+      xpack.security.delete_user:
+        username: "test_user"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_role:
+        name: "alias_write_role"
+        ignore: 404
+
+  - do:
+      indices.delete_alias:
+        index: "write_index_1"
+        name: [ "write_alias" ]
+        ignore: 404
+
+  - do:
+      indices.delete:
+        index: [ "write_index_1" ]
+        ignore: 404
+
+---
+"Test indexing documents into an alias with dynamic mappings":
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      create:
+        id: 1
+        index: write_alias
+        type: doc
+        body: >
+          {
+            "name" : "doc1"
+          }
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      create:
+        id: 2
+        index: write_alias
+        type: doc
+        body: >
+          {
+            "name2" : "doc2"
+          }
