Validate query field when creating roles

client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java

@@ -190,7 +190,7 @@ private static Role randomRole(String roleName) {
                 .name(roleName)
                 .clusterPrivileges(randomSubsetOf(randomInt(3), Role.ClusterPrivilegeName.ALL_ARRAY))
                 .indicesPrivileges(
-                        randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom(randomAlphaOfLength(3))))
+                        randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom("{\"match_all\": {}}")))
                 .applicationResourcePrivileges(randomArray(3, ApplicationResourcePrivileges[]::new,
                         () -> ApplicationResourcePrivilegesTests.createNewRandom(randomAlphaOfLength(3).toLowerCase(Locale.ROOT))))
                 .runAsPrivilege(randomArray(3, String[]::new, () -> randomAlphaOfLength(3)));

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissions.java

@@ -12,29 +12,18 @@
 import org.apache.lucene.search.join.ToChildBlockJoinQuery;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.lucene.search.Queries;
-import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
-import org.elasticsearch.common.xcontent.XContentFactory;
-import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.index.query.BoolQueryBuilder;
-import org.elasticsearch.index.query.BoostingQueryBuilder;
-import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
-import org.elasticsearch.index.query.GeoShapeQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryRewriteContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.query.Rewriteable;
-import org.elasticsearch.index.query.TermsQueryBuilder;
-import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
 import org.elasticsearch.index.search.NestedHelper;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.script.ScriptService;
-import org.elasticsearch.xpack.core.security.authz.support.SecurityQueryTemplateEvaluator;
+import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
 import org.elasticsearch.xpack.core.security.user.User;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
 import java.util.Set;
 import java.util.function.Function;
 
@@ -127,23 +116,21 @@ private static void buildRoleQuery(User user, ScriptService scriptService, Shard
                                        BooleanQuery.Builder filter) throws IOException {
         for (BytesReference bytesReference : queries) {
             QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId);
-            String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(bytesReference.utf8ToString(), scriptService, user);
-            try (XContentParser parser = XContentFactory.xContent(templateResult).createParser(queryShardContext.getXContentRegistry(),
-                    LoggingDeprecationHandler.INSTANCE, templateResult)) {
-                QueryBuilder queryBuilder = queryShardContext.parseInnerQueryBuilder(parser);
-                verifyRoleQuery(queryBuilder);
+            QueryBuilder queryBuilder = DLSRoleQueryValidator.validateAndVerifyRoleQuery(bytesReference, scriptService,
+                queryShardContext.getXContentRegistry(), user);
+            if (queryBuilder != null) {
                 failIfQueryUsesClient(queryBuilder, queryShardContext);
                 Query roleQuery = queryShardContext.toQuery(queryBuilder).query();
                 filter.add(roleQuery, SHOULD);
                 if (queryShardContext.getMapperService().hasNested()) {
                     NestedHelper nestedHelper = new NestedHelper(queryShardContext.getMapperService());
                     if (nestedHelper.mightMatchNestedDocs(roleQuery)) {
                         roleQuery = new BooleanQuery.Builder().add(roleQuery, FILTER)
-                                .add(Queries.newNonNestedFilter(), FILTER).build();
+                            .add(Queries.newNonNestedFilter(), FILTER).build();
                     }
                     // If access is allowed on root doc then also access is allowed on all nested docs of that root document:
                     BitSetProducer rootDocs = queryShardContext
-                            .bitsetFilter(Queries.newNonNestedFilter());
+                        .bitsetFilter(Queries.newNonNestedFilter());
                     ToChildBlockJoinQuery includeNestedDocs = new ToChildBlockJoinQuery(roleQuery, rootDocs);
                     filter.add(includeNestedDocs, SHOULD);
                 }
@@ -153,50 +140,6 @@ private static void buildRoleQuery(User user, ScriptService scriptService, Shard
         filter.setMinimumNumberShouldMatch(1);
     }
 
-    /**
-     * Checks whether the role query contains queries we know can't be used as DLS role query.
-     */
-    static void verifyRoleQuery(QueryBuilder queryBuilder) throws IOException {
-        if (queryBuilder instanceof TermsQueryBuilder) {
-            TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder;
-            if (termsQueryBuilder.termsLookup() != null) {
-                throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query");
-            }
-        } else if (queryBuilder instanceof GeoShapeQueryBuilder) {
-            GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder;
-            if (geoShapeQueryBuilder.shape() == null) {
-                throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't support as part of a role query");
-            }
-        } else if (queryBuilder.getName().equals("percolate")) {
-            // actually only if percolate query is referring to an existing document then this is problematic,
-            // a normal percolate query does work. However we can't check that here as this query builder is inside
-            // another module. So we don't allow the entire percolate query. I don't think users would ever use
-            // a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls.
-            throw new IllegalArgumentException("percolate query isn't support as part of a role query");
-        } else if (queryBuilder.getName().equals("has_child")) {
-            throw new IllegalArgumentException("has_child query isn't support as part of a role query");
-        } else if (queryBuilder.getName().equals("has_parent")) {
-            throw new IllegalArgumentException("has_parent query isn't support as part of a role query");
-        } else if (queryBuilder instanceof BoolQueryBuilder) {
-            BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
-            List<QueryBuilder> clauses = new ArrayList<>();
-            clauses.addAll(boolQueryBuilder.filter());
-            clauses.addAll(boolQueryBuilder.must());
-            clauses.addAll(boolQueryBuilder.mustNot());
-            clauses.addAll(boolQueryBuilder.should());
-            for (QueryBuilder clause : clauses) {
-                verifyRoleQuery(clause);
-            }
-        } else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
-            verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
-        } else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
-            verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
-        } else if (queryBuilder instanceof BoostingQueryBuilder) {
-            verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
-            verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
-        }
-    }
-
     /**
      * Fall back validation that verifies that queries during rewrite don't use
      * the client to make remote calls. In the case of DLS this can cause a dead

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java

@@ -0,0 +1,179 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * 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.core.security.authz.support;
+
+import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParseException;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.index.query.AbstractQueryBuilder;
+import org.elasticsearch.index.query.BoolQueryBuilder;
+import org.elasticsearch.index.query.BoostingQueryBuilder;
+import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
+import org.elasticsearch.index.query.GeoShapeQueryBuilder;
+import org.elasticsearch.index.query.QueryBuilder;
+import org.elasticsearch.index.query.TermsQueryBuilder;
+import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
+import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
+import org.elasticsearch.xpack.core.security.user.User;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * This class helps in evaluating the query field if it is template,
+ * validating the query and checking if the query type is allowed to be used in DLS role query.
+ */
+public final class DLSRoleQueryValidator {
+
+    private DLSRoleQueryValidator() {
+    }
+
+    /**
+     * Validates the query field in the {@link RoleDescriptor.IndicesPrivileges} only if it is not a template query.<br>
+     * It parses the query and builds the {@link QueryBuilder}, also checks if the query type is supported in DLS role query.
+     *
+     * @param indicesPrivileges {@link RoleDescriptor.IndicesPrivileges}
+     * @param xContentRegistry  {@link NamedXContentRegistry} for finding named queries
+     */
+    public static void validateQueryField(RoleDescriptor.IndicesPrivileges[] indicesPrivileges,
+                                          NamedXContentRegistry xContentRegistry) {
+        if (indicesPrivileges != null) {
+            for (int i = 0; i < indicesPrivileges.length; i++) {
+                BytesReference query = indicesPrivileges[i].getQuery();
+                try {
+                    if (query != null) {
+                        if (isTemplateQuery(query, xContentRegistry)) {
+                            // skip template query, this requires runtime information like 'User' information.
+                            continue;
+                        }
+
+                        validateAndVerifyRoleQuery(query.utf8ToString(), xContentRegistry);
+                    }
+                } catch (ParsingException | IllegalArgumentException |  IOException e) {
+                    throw new ElasticsearchParseException("failed to parse field 'query' for [" + i + "]th index privilege " +
+                        "from role descriptor", e);
+                }
+            }
+        }
+    }
+
+    private static boolean isTemplateQuery(BytesReference query, NamedXContentRegistry xContentRegistry) throws IOException {
+        try (XContentParser parser = XContentType.JSON.xContent().createParser(xContentRegistry,
+            LoggingDeprecationHandler.INSTANCE, query.utf8ToString())) {
+            XContentParser.Token token = parser.nextToken();
+            if (token != XContentParser.Token.START_OBJECT) {
+                throw new XContentParseException(parser.getTokenLocation(), "expected [" + XContentParser.Token.START_OBJECT + "] but " +
+                    "found [" + token + "] instead");
+            }
+            token = parser.nextToken();
+            if (token != XContentParser.Token.FIELD_NAME) {
+                throw new XContentParseException(parser.getTokenLocation(), "expected [" + XContentParser.Token.FIELD_NAME + "] with " +
+                    "value a query name or 'template' but found [" + token + "] instead");
+            }
+            String fieldName = parser.currentName();
+            if ("template".equals(fieldName)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Evaluates the query if it is a template and then validates the query by parsing
+     * and building the {@link QueryBuilder}. It also checks if the query type is
+     * supported in DLS role query.
+     *
+     * @param query            {@link BytesReference} query field from the role
+     * @param scriptService    {@link ScriptService} used for evaluation of a template query
+     * @param xContentRegistry {@link NamedXContentRegistry} for finding named queries
+     * @param user             {@link User} used when evaluation a template query
+     * @return {@link QueryBuilder} if the query is valid and allowed, in case {@link RoleDescriptor.IndicesPrivileges}
+     * * does not have a query field then it returns {@code null}.
+     */
+    @Nullable
+    public static QueryBuilder validateAndVerifyRoleQuery(BytesReference query, ScriptService scriptService,
+                                                          NamedXContentRegistry xContentRegistry, User user) {
+        QueryBuilder queryBuilder = null;
+        if (query != null) {
+            String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(query.utf8ToString(), scriptService,
+                user);
+            try {
+                queryBuilder = validateAndVerifyRoleQuery(templateResult, xContentRegistry);
+            } catch (ElasticsearchParseException | ParsingException | XContentParseException | IOException e) {
+                throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e);
+            }
+        }
+        return queryBuilder;
+    }
+
+    private static QueryBuilder validateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry) throws IOException {
+        QueryBuilder queryBuilder = null;
+        if (query != null) {
+            try (XContentParser parser = XContentFactory.xContent(query).createParser(xContentRegistry,
+                LoggingDeprecationHandler.INSTANCE, query)) {
+                queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser);
+                verifyRoleQuery(queryBuilder);
+            }
+        }
+        return queryBuilder;
+    }
+
+    /**
+     * Checks whether the role query contains queries we know can't be used as DLS role query.
+     *
+     * @param queryBuilder {@link QueryBuilder} for given query
+     */
+    public static void verifyRoleQuery(QueryBuilder queryBuilder) {
+        if (queryBuilder instanceof TermsQueryBuilder) {
+            TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder;
+            if (termsQueryBuilder.termsLookup() != null) {
+                throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query");
+            }
+        } else if (queryBuilder instanceof GeoShapeQueryBuilder) {
+            GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder;
+            if (geoShapeQueryBuilder.shape() == null) {
+                throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't supported as part of a role query");
+            }
+        } else if (queryBuilder.getName().equals("percolate")) {
+            // actually only if percolate query is referring to an existing document then this is problematic,
+            // a normal percolate query does work. However we can't check that here as this query builder is inside
+            // another module. So we don't allow the entire percolate query. I don't think users would ever use
+            // a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls.
+            throw new IllegalArgumentException("percolate query isn't supported as part of a role query");
+        } else if (queryBuilder.getName().equals("has_child")) {
+            throw new IllegalArgumentException("has_child query isn't supported as part of a role query");
+        } else if (queryBuilder.getName().equals("has_parent")) {
+            throw new IllegalArgumentException("has_parent query isn't supported as part of a role query");
+        } else if (queryBuilder instanceof BoolQueryBuilder) {
+            BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
+            List<QueryBuilder> clauses = new ArrayList<>();
+            clauses.addAll(boolQueryBuilder.filter());
+            clauses.addAll(boolQueryBuilder.must());
+            clauses.addAll(boolQueryBuilder.mustNot());
+            clauses.addAll(boolQueryBuilder.should());
+            for (QueryBuilder clause : clauses) {
+                verifyRoleQuery(clause);
+            }
+        } else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
+            verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
+        } else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
+            verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
+        } else if (queryBuilder instanceof BoostingQueryBuilder) {
+            verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
+            verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
+        }
+    }
+}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/SecurityQueryTemplateEvaluator.java

@@ -44,10 +44,8 @@ private SecurityQueryTemplateEvaluator() {
      * @return resultant query string after compiling and executing the script.
      * If the source does not contain template then it will return the query
      * source without any modifications.
-     * @throws IOException thrown when there is any error parsing the query
-     * string.
      */
-    public static String evaluateTemplate(final String querySource, final ScriptService scriptService, final User user) throws IOException {
+    public static String evaluateTemplate(final String querySource, final ScriptService scriptService, final User user) {
         // EMPTY is safe here because we never use namedObject
         try (XContentParser parser = XContentFactory.xContent(querySource).createParser(NamedXContentRegistry.EMPTY,
                 LoggingDeprecationHandler.INSTANCE, querySource)) {
@@ -76,6 +74,8 @@ public static String evaluateTemplate(final String querySource, final ScriptServ
             } else {
                 return querySource;
             }
+        } catch (IOException ioe) {
+            throw new ElasticsearchParseException("failed to parse query");
         }
     }