Skip to content

Commit 40ad77b

Browse files
slobodanadamovicslobodanadamovic
authored
[Failure Store] Prevent explicit selectors in role index name patterns (#125843) (#125939)
Adding basic validation to prevent using `::` selectors when defining index permissions. Index names do not allow colon character (`:`), hence the index name patterns that would include double colon (`::`), would never match any of the index names. To avoid confusion, we are preventing using `::` in role index name patterns. For example, the `test-*::failures` will be rejected during `test-role` validation: ``` PUT /_security/role/test-role { "indices": [ { "names": ["test-*::failures"], "privileges": ["read"] } ] } ``` (cherry picked from commit 1f7e26c)
1 parent 6ab1b81 commit 40ad77b

File tree

3 files changed

+243
-58
lines changed

3 files changed

+243
-58
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidator.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package org.elasticsearch.xpack.core.security.action.role;
99

1010
import org.elasticsearch.action.ActionRequestValidationException;
11+
import org.elasticsearch.cluster.metadata.DataStream;
12+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1113
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
1214
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
1315
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
@@ -53,6 +55,11 @@ public static ActionRequestValidationException validate(
5355
} catch (IllegalArgumentException ile) {
5456
validationException = addValidationError(ile.getMessage(), validationException);
5557
}
58+
if (DataStream.isFailureStoreFeatureFlagEnabled()) {
59+
for (final String indexName : idp.getIndices()) {
60+
validationException = validateIndexNameExpression(indexName, validationException);
61+
}
62+
}
5663
}
5764
}
5865
final RoleDescriptor.RemoteIndicesPrivileges[] remoteIndicesPrivileges = roleDescriptor.getRemoteIndicesPrivileges();
@@ -71,6 +78,11 @@ public static ActionRequestValidationException validate(
7178
} catch (IllegalArgumentException ile) {
7279
validationException = addValidationError(ile.getMessage(), validationException);
7380
}
81+
if (DataStream.isFailureStoreFeatureFlagEnabled()) {
82+
for (String indexName : ridp.indicesPrivileges().getIndices()) {
83+
validationException = validateIndexNameExpression(indexName, validationException);
84+
}
85+
}
7486
}
7587
if (roleDescriptor.hasRemoteClusterPermissions()) {
7688
try {
@@ -118,4 +130,21 @@ public static ActionRequestValidationException validate(
118130
}
119131
return validationException;
120132
}
133+
134+
private static ActionRequestValidationException validateIndexNameExpression(
135+
String indexNameExpression,
136+
ActionRequestValidationException validationException
137+
) {
138+
if (IndexNameExpressionResolver.hasSelectorSuffix(indexNameExpression)) {
139+
validationException = addValidationError(
140+
"selectors ["
141+
+ IndexNameExpressionResolver.SelectorResolver.SELECTOR_SEPARATOR
142+
+ "] are not allowed in the index name expression ["
143+
+ indexNameExpression
144+
+ "]",
145+
validationException
146+
);
147+
}
148+
return validationException;
149+
}
121150
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidatorTests.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
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+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.core.security.action.role;
9+
10+
import org.elasticsearch.cluster.metadata.DataStream;
11+
import org.elasticsearch.test.ESTestCase;
12+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
13+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
14+
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
15+
16+
import java.util.Set;
17+
import java.util.stream.Collectors;
18+
19+
import static org.hamcrest.Matchers.containsInAnyOrder;
20+
import static org.hamcrest.Matchers.notNullValue;
21+
import static org.hamcrest.Matchers.nullValue;
22+
23+
public class RoleDescriptorRequestValidatorTests extends ESTestCase {
24+
25+
public void testSelectorsValidation() {
26+
assumeTrue("failure store feature flag must be enabled", DataStream.isFailureStoreFeatureFlagEnabled());
27+
String[] invalidIndexNames = {
28+
"index::failures",
29+
".fs-*::failures",
30+
".ds-*::data",
31+
"*::failures",
32+
"*::",
33+
"?::?",
34+
"test?-*::data",
35+
"test-*::*", // actual selector is not relevant and not validated
36+
"test::irrelevant",
37+
"::test",
38+
"test::",
39+
"::",
40+
":: ",
41+
" ::",
42+
":::",
43+
"::::",
44+
randomAlphaOfLengthBetween(5, 10) + "\u003a\u003afailures",
45+
randomAlphaOfLengthBetween(5, 10) + "\072\072failures" };
46+
for (String indexName : invalidIndexNames) {
47+
validateAndAssertSelectorNotAllowed(roleWithIndexPrivileges(indexName), indexName);
48+
validateAndAssertSelectorNotAllowed(roleWithRemoteIndexPrivileges(indexName), indexName);
49+
}
50+
51+
// these are not necessarily valid index names, but they should not trigger the selector validation
52+
String[] validIndexNames = {
53+
"index:failures", // single colon is allowed
54+
":failures",
55+
"no double colon",
56+
":",
57+
": :",
58+
"",
59+
" ",
60+
":\n:",
61+
null,
62+
"a:",
63+
":b:",
64+
"*",
65+
"c?-*",
66+
"d-*e",
67+
"f:g:h",
68+
"/[a-b]*test:[a-b]*:failures/", // while this regex can match test::failures, it is not rejected - doing so would be too complex
69+
randomIntBetween(-10, 10) + "",
70+
randomAlphaOfLengthBetween(1, 10),
71+
randomAlphanumericOfLength(10) };
72+
for (String indexName : validIndexNames) {
73+
validateAndAssertNoException(roleWithIndexPrivileges(indexName), indexName);
74+
validateAndAssertNoException(roleWithRemoteIndexPrivileges(indexName), indexName);
75+
}
76+
}
77+
78+
private static void validateAndAssertSelectorNotAllowed(RoleDescriptor roleDescriptor, String indexName) {
79+
var validationException = RoleDescriptorRequestValidator.validate(roleDescriptor);
80+
assertThat("expected validation exception for " + indexName, validationException, notNullValue());
81+
assertThat(
82+
validationException.validationErrors(),
83+
containsInAnyOrder("selectors [::] are not allowed in the index name expression [" + indexName + "]")
84+
);
85+
}
86+
87+
private static void validateAndAssertNoException(RoleDescriptor roleDescriptor, String indexName) {
88+
var validationException = RoleDescriptorRequestValidator.validate(roleDescriptor);
89+
assertThat("expected no validation exception for " + indexName, validationException, nullValue());
90+
}
91+
92+
private static RoleDescriptor roleWithIndexPrivileges(String... indices) {
93+
return new RoleDescriptor(
94+
"test-role",
95+
null,
96+
new IndicesPrivileges[] {
97+
IndicesPrivileges.builder()
98+
.allowRestrictedIndices(randomBoolean())
99+
.indices(indices)
100+
.privileges(randomSubsetOf(randomIntBetween(1, IndexPrivilege.names().size()), IndexPrivilege.names()))
101+
.build() },
102+
null,
103+
null,
104+
null,
105+
null,
106+
null,
107+
null,
108+
null,
109+
null,
110+
null
111+
);
112+
}
113+
114+
private static RoleDescriptor roleWithRemoteIndexPrivileges(String... indices) {
115+
Set<String> privileges = IndexPrivilege.names()
116+
.stream()
117+
.filter(p -> false == (p.equals("read_failure_store") || p.equals("manage_failure_store")))
118+
.collect(Collectors.toSet());
119+
return new RoleDescriptor(
120+
"remote-test-role",
121+
null,
122+
null,
123+
null,
124+
null,
125+
null,
126+
null,
127+
null,
128+
new RoleDescriptor.RemoteIndicesPrivileges[] {
129+
new RoleDescriptor.RemoteIndicesPrivileges(
130+
IndicesPrivileges.builder()
131+
.allowRestrictedIndices(randomBoolean())
132+
.indices(indices)
133+
.privileges(randomSubsetOf(randomIntBetween(1, privileges.size()), privileges))
134+
.build(),
135+
"my-remote-cluster"
136+
) },
137+
null,
138+
null,
139+
null
140+
);
141+
}
142+
}

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java‎

Lines changed: 72 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.stream.Collectors;
4949

5050
import static org.hamcrest.Matchers.containsInAnyOrder;
51+
import static org.hamcrest.Matchers.containsString;
5152
import static org.hamcrest.Matchers.equalTo;
5253
import static org.hamcrest.Matchers.hasItem;
5354
import static org.hamcrest.Matchers.is;
@@ -877,62 +878,56 @@ public void testHasPrivilegesWithApiKeys() throws IOException {
877878

878879
public void testRoleWithSelectorInIndexPattern() throws Exception {
879880
setupDataStream();
880-
881881
createUser("user", PASSWORD, "role");
882-
upsertRole("""
883-
{
884-
"cluster": ["all"],
885-
"indices": [
886-
{
887-
"names": ["*::failures"],
888-
"privileges": ["read"]
889-
}
890-
]
891-
}""", "role");
892-
createAndStoreApiKey("user", null);
893-
894-
expectThrows("user", new Search("test1::failures"), 403);
895-
expectSearch("user", new Search("*::failures"));
896-
897-
upsertRole("""
898-
{
899-
"cluster": ["all"],
900-
"indices": [
901-
{
902-
"names": ["test1::failures"],
903-
"privileges": ["read"]
904-
}
905-
]
906-
}""", "role");
882+
expectThrowsSelectorsNotAllowed(
883+
() -> upsertRole(
884+
Strings.format("""
885+
{
886+
"cluster": ["all"],
887+
"indices": [
888+
{
889+
"names": ["%s"],
890+
"privileges": ["%s"]
891+
}
892+
]
893+
}""", randomFrom("*::failures", "test1::failures", "test1::data", "*::data"), randomFrom("read", "read_failure_store")),
894+
"role",
895+
false
896+
)
897+
);
907898

908-
expectThrows("user", new Search("test1::failures"), 403);
909-
expectSearch("user", new Search("*::failures"));
899+
AssertionError bulkFailedError = expectThrows(
900+
AssertionError.class,
901+
() -> upsertRole(
902+
Strings.format("""
903+
{
904+
"cluster": ["all"],
905+
"indices": [
906+
{
907+
"names": ["%s"],
908+
"privileges": ["%s"]
909+
}
910+
]
911+
}""", randomFrom("*::failures", "test1::failures", "test1::data", "*::data"), randomFrom("read", "read_failure_store")),
912+
"role",
913+
true
914+
)
915+
);
916+
assertThat(bulkFailedError.getMessage(), containsString("selectors [::] are not allowed in the index name expression"));
910917

911-
upsertRole("""
918+
expectThrowsSelectorsNotAllowed(() -> createApiKey("user", Strings.format("""
912919
{
913-
"cluster": ["all"],
914-
"indices": [
915-
{
916-
"names": ["*::failures"],
917-
"privileges": ["read_failure_store"]
920+
"role": {
921+
"cluster": ["all"],
922+
"indices": [
923+
{
924+
"names": ["%s"],
925+
"privileges": ["%s"]
926+
}
927+
]
918928
}
919-
]
920-
}""", "role");
921-
expectThrows("user", new Search("test1::failures"), 403);
922-
expectSearch("user", new Search("*::failures"));
929+
}""", randomFrom("*::failures", "test1::failures", "test1::data", "*::data"), randomFrom("read", "read_failure_store"))));
923930

924-
upsertRole("""
925-
{
926-
"cluster": ["all"],
927-
"indices": [
928-
{
929-
"names": ["test1::failures"],
930-
"privileges": ["read_failure_store"]
931-
}
932-
]
933-
}""", "role");
934-
expectThrows("user", new Search("test1::failures"), 403);
935-
expectSearch("user", new Search("*::failures"));
936931
}
937932

938933
public void testFailureStoreAccess() throws Exception {
@@ -2489,7 +2484,7 @@ protected void createUser(String username, SecureString password, String... role
24892484
protected String createAndStoreApiKey(String username, @Nullable String roleDescriptors) throws IOException {
24902485
assertThat("API key already registered for user: " + username, apiKeys.containsKey(username), is(false));
24912486
apiKeys.put(username, createApiKey(username, roleDescriptors));
2492-
return createApiKey(username, roleDescriptors);
2487+
return apiKeys.get(username);
24932488
}
24942489

24952490
private String createApiKey(String username, String roleDescriptors) throws IOException {
@@ -2514,22 +2509,35 @@ private String createApiKey(String username, String roleDescriptors) throws IOEx
25142509
return (String) responseAsMap.get("encoded");
25152510
}
25162511

2517-
protected void upsertRole(String roleDescriptor, String roleName) throws IOException {
2518-
Request createRoleRequest = roleRequest(roleDescriptor, roleName);
2512+
protected Response upsertRole(String roleDescriptor, String roleName) throws IOException {
2513+
return upsertRole(roleDescriptor, roleName, randomBoolean());
2514+
}
2515+
2516+
protected Response upsertRole(String roleDescriptor, String roleName, boolean bulk) throws IOException {
2517+
Request createRoleRequest = roleRequest(roleDescriptor, roleName, bulk);
25192518
Response createRoleResponse = adminClient().performRequest(createRoleRequest);
25202519
assertOK(createRoleResponse);
2520+
if (bulk) {
2521+
Map<String, Object> flattenedResponse = Maps.flatten(responseAsMap(createRoleResponse), true, true);
2522+
if (flattenedResponse.containsKey("errors.count") && (int) flattenedResponse.get("errors.count") > 0) {
2523+
throw new AssertionError(
2524+
"Failed to create role [" + roleName + "], reason: " + flattenedResponse.get("errors.details." + roleName + ".reason")
2525+
);
2526+
}
2527+
}
2528+
return createRoleResponse;
25212529
}
25222530

2523-
protected Request roleRequest(String roleDescriptor, String roleName) {
2531+
protected Request roleRequest(String roleDescriptor, String roleName, boolean bulk) {
25242532
Request createRoleRequest;
2525-
if (randomBoolean()) {
2526-
createRoleRequest = new Request(randomFrom(HttpPut.METHOD_NAME, HttpPost.METHOD_NAME), "/_security/role/" + roleName);
2527-
createRoleRequest.setJsonEntity(roleDescriptor);
2528-
} else {
2533+
if (bulk) {
25292534
createRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role");
25302535
createRoleRequest.setJsonEntity(org.elasticsearch.core.Strings.format("""
25312536
{"roles": {"%s": %s}}
25322537
""", roleName, roleDescriptor));
2538+
} else {
2539+
createRoleRequest = new Request(randomFrom(HttpPut.METHOD_NAME, HttpPost.METHOD_NAME), "/_security/role/" + roleName);
2540+
createRoleRequest.setJsonEntity(roleDescriptor);
25332541
}
25342542
return createRoleRequest;
25352543
}
@@ -2592,4 +2600,10 @@ private void expectHasPrivilegesWithApiKey(String apiKey, String requestBody, St
25922600
Response response = performRequestWithApiKey(apiKey, req);
25932601
assertThat(responseAsMap(response), equalTo(mapFromJson(expectedResponse)));
25942602
}
2603+
2604+
private static void expectThrowsSelectorsNotAllowed(ThrowingRunnable runnable) {
2605+
ResponseException exception = expectThrows(ResponseException.class, runnable);
2606+
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(400));
2607+
assertThat(exception.getMessage(), containsString("selectors [::] are not allowed in the index name expression"));
2608+
}
25952609
}

0 commit comments

Comments
 (0)