Skip to content

Commit 6e1c8b6

Browse files
author
Yogesh Gaikwad
committed
Validate query field when creating roles
As of now the validation occurs at runtime when the query is being executed. I think the reason was due to the use of template queries which need runtime information as they need to be evaluated like user information. This commit adds validation for the role query but **not for the template query** as we do not have the runtime information required for evaluating the template query. This also corrects some tests and roles.yml files where the `query` field was not populated correctly. For validation, the query is evaluated (if not a template), parsed to build the `QueryBuilder` and verify if the query type is allowed. Closes #34252
1 parent 210b592 commit 6e1c8b6

File tree

20 files changed

+463
-192
lines changed

20 files changed

+463
-192
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private static Role randomRole(String roleName) {
190190
.name(roleName)
191191
.clusterPrivileges(randomSubsetOf(randomInt(3), Role.ClusterPrivilegeName.ALL_ARRAY))
192192
.indicesPrivileges(
193-
randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom(randomAlphaOfLength(3))))
193+
randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom("{\"match_all\": {}}")))
194194
.applicationResourcePrivileges(randomArray(3, ApplicationResourcePrivileges[]::new,
195195
() -> ApplicationResourcePrivilegesTests.createNewRandom(randomAlphaOfLength(3).toLowerCase(Locale.ROOT))))
196196
.runAsPrivilege(randomArray(3, String[]::new, () -> randomAlphaOfLength(3)));

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

+6-63
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,18 @@
1212
import org.apache.lucene.search.join.ToChildBlockJoinQuery;
1313
import org.elasticsearch.common.bytes.BytesReference;
1414
import org.elasticsearch.common.lucene.search.Queries;
15-
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
16-
import org.elasticsearch.common.xcontent.XContentFactory;
17-
import org.elasticsearch.common.xcontent.XContentParser;
18-
import org.elasticsearch.index.query.BoolQueryBuilder;
19-
import org.elasticsearch.index.query.BoostingQueryBuilder;
20-
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
21-
import org.elasticsearch.index.query.GeoShapeQueryBuilder;
2215
import org.elasticsearch.index.query.QueryBuilder;
2316
import org.elasticsearch.index.query.QueryRewriteContext;
2417
import org.elasticsearch.index.query.QueryShardContext;
2518
import org.elasticsearch.index.query.Rewriteable;
26-
import org.elasticsearch.index.query.TermsQueryBuilder;
27-
import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
2819
import org.elasticsearch.index.search.NestedHelper;
2920
import org.elasticsearch.index.shard.ShardId;
3021
import org.elasticsearch.script.ScriptService;
31-
import org.elasticsearch.xpack.core.security.authz.support.SecurityQueryTemplateEvaluator;
22+
import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
3223
import org.elasticsearch.xpack.core.security.user.User;
3324

3425
import java.io.IOException;
35-
import java.util.ArrayList;
3626
import java.util.Collections;
37-
import java.util.List;
3827
import java.util.Set;
3928
import java.util.function.Function;
4029

@@ -127,23 +116,21 @@ private static void buildRoleQuery(User user, ScriptService scriptService, Shard
127116
BooleanQuery.Builder filter) throws IOException {
128117
for (BytesReference bytesReference : queries) {
129118
QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId);
130-
String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(bytesReference.utf8ToString(), scriptService, user);
131-
try (XContentParser parser = XContentFactory.xContent(templateResult).createParser(queryShardContext.getXContentRegistry(),
132-
LoggingDeprecationHandler.INSTANCE, templateResult)) {
133-
QueryBuilder queryBuilder = queryShardContext.parseInnerQueryBuilder(parser);
134-
verifyRoleQuery(queryBuilder);
119+
QueryBuilder queryBuilder = DLSRoleQueryValidator.validateAndVerifyRoleQuery(bytesReference, scriptService,
120+
queryShardContext.getXContentRegistry(), user);
121+
if (queryBuilder != null) {
135122
failIfQueryUsesClient(queryBuilder, queryShardContext);
136123
Query roleQuery = queryShardContext.toQuery(queryBuilder).query();
137124
filter.add(roleQuery, SHOULD);
138125
if (queryShardContext.getMapperService().hasNested()) {
139126
NestedHelper nestedHelper = new NestedHelper(queryShardContext.getMapperService());
140127
if (nestedHelper.mightMatchNestedDocs(roleQuery)) {
141128
roleQuery = new BooleanQuery.Builder().add(roleQuery, FILTER)
142-
.add(Queries.newNonNestedFilter(), FILTER).build();
129+
.add(Queries.newNonNestedFilter(), FILTER).build();
143130
}
144131
// If access is allowed on root doc then also access is allowed on all nested docs of that root document:
145132
BitSetProducer rootDocs = queryShardContext
146-
.bitsetFilter(Queries.newNonNestedFilter());
133+
.bitsetFilter(Queries.newNonNestedFilter());
147134
ToChildBlockJoinQuery includeNestedDocs = new ToChildBlockJoinQuery(roleQuery, rootDocs);
148135
filter.add(includeNestedDocs, SHOULD);
149136
}
@@ -153,50 +140,6 @@ private static void buildRoleQuery(User user, ScriptService scriptService, Shard
153140
filter.setMinimumNumberShouldMatch(1);
154141
}
155142

156-
/**
157-
* Checks whether the role query contains queries we know can't be used as DLS role query.
158-
*/
159-
static void verifyRoleQuery(QueryBuilder queryBuilder) throws IOException {
160-
if (queryBuilder instanceof TermsQueryBuilder) {
161-
TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder;
162-
if (termsQueryBuilder.termsLookup() != null) {
163-
throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query");
164-
}
165-
} else if (queryBuilder instanceof GeoShapeQueryBuilder) {
166-
GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder;
167-
if (geoShapeQueryBuilder.shape() == null) {
168-
throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't support as part of a role query");
169-
}
170-
} else if (queryBuilder.getName().equals("percolate")) {
171-
// actually only if percolate query is referring to an existing document then this is problematic,
172-
// a normal percolate query does work. However we can't check that here as this query builder is inside
173-
// another module. So we don't allow the entire percolate query. I don't think users would ever use
174-
// a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls.
175-
throw new IllegalArgumentException("percolate query isn't support as part of a role query");
176-
} else if (queryBuilder.getName().equals("has_child")) {
177-
throw new IllegalArgumentException("has_child query isn't support as part of a role query");
178-
} else if (queryBuilder.getName().equals("has_parent")) {
179-
throw new IllegalArgumentException("has_parent query isn't support as part of a role query");
180-
} else if (queryBuilder instanceof BoolQueryBuilder) {
181-
BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
182-
List<QueryBuilder> clauses = new ArrayList<>();
183-
clauses.addAll(boolQueryBuilder.filter());
184-
clauses.addAll(boolQueryBuilder.must());
185-
clauses.addAll(boolQueryBuilder.mustNot());
186-
clauses.addAll(boolQueryBuilder.should());
187-
for (QueryBuilder clause : clauses) {
188-
verifyRoleQuery(clause);
189-
}
190-
} else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
191-
verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
192-
} else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
193-
verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
194-
} else if (queryBuilder instanceof BoostingQueryBuilder) {
195-
verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
196-
verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
197-
}
198-
}
199-
200143
/**
201144
* Fall back validation that verifies that queries during rewrite don't use
202145
* the client to make remote calls. In the case of DLS this can cause a dead
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.core.security.authz.support;
7+
8+
import org.elasticsearch.ElasticsearchParseException;
9+
import org.elasticsearch.common.Nullable;
10+
import org.elasticsearch.common.ParsingException;
11+
import org.elasticsearch.common.bytes.BytesReference;
12+
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
13+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
14+
import org.elasticsearch.common.xcontent.XContentFactory;
15+
import org.elasticsearch.common.xcontent.XContentParseException;
16+
import org.elasticsearch.common.xcontent.XContentParser;
17+
import org.elasticsearch.index.query.AbstractQueryBuilder;
18+
import org.elasticsearch.index.query.BoolQueryBuilder;
19+
import org.elasticsearch.index.query.BoostingQueryBuilder;
20+
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
21+
import org.elasticsearch.index.query.GeoShapeQueryBuilder;
22+
import org.elasticsearch.index.query.QueryBuilder;
23+
import org.elasticsearch.index.query.TermsQueryBuilder;
24+
import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
25+
import org.elasticsearch.script.ScriptService;
26+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
27+
import org.elasticsearch.xpack.core.security.user.User;
28+
29+
import java.io.IOException;
30+
import java.util.ArrayList;
31+
import java.util.List;
32+
33+
/**
34+
* This class helps in evaluating the query field if it is template
35+
* and checking if the query type is allowed to be used in DLS role query.
36+
*/
37+
public class DLSRoleQueryValidator {
38+
39+
/**
40+
* Evaluates the query field in the {@link RoleDescriptor.IndicesPrivileges} only if it is not a template
41+
* query.<br>
42+
* It parses the query and builds the {@link QueryBuilder}, also checks if the query type is supported in DLS role query.
43+
*
44+
* @param indicesPrivileges {@link RoleDescriptor.IndicesPrivileges}
45+
* @param xContentRegistry {@link NamedXContentRegistry} for finding named queries
46+
*/
47+
public static void validateQueryField(RoleDescriptor.IndicesPrivileges[] indicesPrivileges,
48+
NamedXContentRegistry xContentRegistry) {
49+
if (indicesPrivileges != null) {
50+
for (RoleDescriptor.IndicesPrivileges idp : indicesPrivileges) {
51+
BytesReference query = idp.getQuery();
52+
if (query != null) {
53+
if (isTemplateQuery(query, xContentRegistry)) {
54+
// skip template query, this requires runtime information like 'User' information.
55+
continue;
56+
}
57+
58+
validateAndVerifyRoleQuery(query.utf8ToString(), xContentRegistry);
59+
}
60+
}
61+
}
62+
}
63+
64+
private static boolean isTemplateQuery(BytesReference query, NamedXContentRegistry xContentRegistry) {
65+
try {
66+
try (XContentParser parser = XContentFactory.xContent(query.utf8ToString()).createParser(xContentRegistry,
67+
LoggingDeprecationHandler.INSTANCE, query.utf8ToString())) {
68+
expectedToken(parser.nextToken(), parser, XContentParser.Token.START_OBJECT);
69+
expectedToken(parser.nextToken(), parser, XContentParser.Token.FIELD_NAME);
70+
String fieldName = parser.currentName();
71+
if ("template".equals(fieldName)) {
72+
return true;
73+
}
74+
}
75+
} catch (XContentParseException | IOException e) {
76+
throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e);
77+
}
78+
return false;
79+
}
80+
81+
private static void expectedToken(XContentParser.Token read, XContentParser parser, XContentParser.Token expected) {
82+
if (read != expected) {
83+
throw new XContentParseException(parser.getTokenLocation(),
84+
"expected [" + expected + "] but found [" + read + "] instead");
85+
}
86+
}
87+
88+
/**
89+
* Evaluates the query if it is a template and then validates the query by parsing
90+
* and building the {@link QueryBuilder}. It also checks if the query type is
91+
* supported in DLS role query.
92+
*
93+
* @param query {@link BytesReference} query field from the role
94+
* @param scriptService {@link ScriptService} used for evaluation of a template query
95+
* @param xContentRegistry {@link NamedXContentRegistry} for finding named queries
96+
* @param user {@link User} used when evaluation a template query
97+
* @return {@link QueryBuilder} if the query is valid and allowed, in case {@link RoleDescriptor.IndicesPrivileges}
98+
* * does not have a query field then it returns {@code null}.
99+
*/
100+
@Nullable
101+
public static QueryBuilder validateAndVerifyRoleQuery(BytesReference query, ScriptService scriptService,
102+
NamedXContentRegistry xContentRegistry, User user) {
103+
QueryBuilder queryBuilder = null;
104+
if (query != null) {
105+
try {
106+
String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(query.utf8ToString(), scriptService,
107+
user);
108+
queryBuilder = validateAndVerifyRoleQuery(templateResult, xContentRegistry);
109+
} catch (ElasticsearchParseException | ParsingException | XContentParseException | IOException e) {
110+
throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e);
111+
}
112+
}
113+
return queryBuilder;
114+
}
115+
116+
private static QueryBuilder validateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry) {
117+
QueryBuilder queryBuilder = null;
118+
if (query != null) {
119+
try {
120+
try (XContentParser parser = XContentFactory.xContent(query).createParser(xContentRegistry,
121+
LoggingDeprecationHandler.INSTANCE, query)) {
122+
queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser);
123+
verifyRoleQuery(queryBuilder);
124+
}
125+
} catch (ElasticsearchParseException | ParsingException | XContentParseException | IOException e) {
126+
throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e);
127+
}
128+
}
129+
return queryBuilder;
130+
}
131+
132+
/**
133+
* Checks whether the role query contains queries we know can't be used as DLS role query.
134+
*
135+
* @param queryBuilder {@link QueryBuilder} for given query
136+
*/
137+
public static void verifyRoleQuery(QueryBuilder queryBuilder) {
138+
if (queryBuilder instanceof TermsQueryBuilder) {
139+
TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder;
140+
if (termsQueryBuilder.termsLookup() != null) {
141+
throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query");
142+
}
143+
} else if (queryBuilder instanceof GeoShapeQueryBuilder) {
144+
GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder;
145+
if (geoShapeQueryBuilder.shape() == null) {
146+
throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't support as part of a role query");
147+
}
148+
} else if (queryBuilder.getName().equals("percolate")) {
149+
// actually only if percolate query is referring to an existing document then this is problematic,
150+
// a normal percolate query does work. However we can't check that here as this query builder is inside
151+
// another module. So we don't allow the entire percolate query. I don't think users would ever use
152+
// a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls.
153+
throw new IllegalArgumentException("percolate query isn't support as part of a role query");
154+
} else if (queryBuilder.getName().equals("has_child")) {
155+
throw new IllegalArgumentException("has_child query isn't support as part of a role query");
156+
} else if (queryBuilder.getName().equals("has_parent")) {
157+
throw new IllegalArgumentException("has_parent query isn't support as part of a role query");
158+
} else if (queryBuilder instanceof BoolQueryBuilder) {
159+
BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
160+
List<QueryBuilder> clauses = new ArrayList<>();
161+
clauses.addAll(boolQueryBuilder.filter());
162+
clauses.addAll(boolQueryBuilder.must());
163+
clauses.addAll(boolQueryBuilder.mustNot());
164+
clauses.addAll(boolQueryBuilder.should());
165+
for (QueryBuilder clause : clauses) {
166+
verifyRoleQuery(clause);
167+
}
168+
} else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
169+
verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
170+
} else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
171+
verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
172+
} else if (queryBuilder instanceof BoostingQueryBuilder) {
173+
verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
174+
verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
175+
}
176+
}
177+
}

0 commit comments

Comments
 (0)