[Security] BulkShardRequest may have multiple indices (elastic/x-pack-elasticsearch#2742)

If a bulk update references aliases rather than concrete indices,
it is possible that a single shard level request could have multiple distinct "index names", potentially including "date math".
Those names will resolve to the same concrete index, but they might have different privileges.

Original commit: elastic/x-pack-elasticsearch@34cfd11df8
This commit is contained in:
Tim Vernum 2017-10-26 15:34:58 +11:00 committed by GitHub
parent 70a38ec545
commit 8985625ea5
9 changed files with 577 additions and 48 deletions

View File

@ -38,6 +38,7 @@ import org.elasticsearch.action.termvectors.MultiTermVectorsAction;
import org.elasticsearch.action.update.UpdateAction; import org.elasticsearch.action.update.UpdateAction;
import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Setting.Property;
@ -327,7 +328,7 @@ public class AuthorizationService extends AbstractComponent {
assert request instanceof BulkShardRequest assert request instanceof BulkShardRequest
: "Action " + action + " requires " + BulkShardRequest.class + " but was " + request.getClass(); : "Action " + action + " requires " + BulkShardRequest.class + " but was " + request.getClass();
authorizeBulkItems(authentication, action, (BulkShardRequest) request, permission, metaData, localIndices); authorizeBulkItems(authentication, (BulkShardRequest) request, permission, metaData, localIndices, authorizedIndices);
} }
grant(authentication, action, originalRequest); grant(authentication, action, originalRequest);
@ -343,28 +344,48 @@ public class AuthorizationService extends AbstractComponent {
* {@link DocWriteRequest.OpType types}, the number of distinct authorization checks that need to be performed is very small, but the * {@link DocWriteRequest.OpType types}, the number of distinct authorization checks that need to be performed is very small, but the
* results must be cached, to avoid adding a high overhead to each bulk request. * results must be cached, to avoid adding a high overhead to each bulk request.
*/ */
private void authorizeBulkItems(Authentication authentication, String action, BulkShardRequest request, Role permission, private void authorizeBulkItems(Authentication authentication, BulkShardRequest request, Role permission,
MetaData metaData, Set<String> indices) { MetaData metaData, Set<String> indices, AuthorizedIndices authorizedIndices) {
if (indices.size() != 1) { // Maps original-index -> expanded-index-name (expands date-math, but not aliases)
final String message = "Action " + action + " should operate on exactly 1 local index but was " + indices.size(); final Map<String, String> resolvedIndexNames = new HashMap<>();
assert false : message; // Maps (resolved-index , action) -> is-granted
throw new IllegalStateException(message); final Map<Tuple<String, String>, Boolean> indexActionAuthority = new HashMap<>();
}
final String index = indices.iterator().next();
final Map<String, Boolean> actionAuthority = new HashMap<>();
for (BulkItemRequest item : request.items()) { for (BulkItemRequest item : request.items()) {
String resolvedIndex = resolvedIndexNames.computeIfAbsent(item.index(), key -> {
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.resolveIndicesAndAliases(item.request(), metaData,
authorizedIndices);
if (resolvedIndices.getRemote().size() != 0) {
throw illegalArgument("Bulk item should not write to remote indices, but request writes to "
+ String.join(",", resolvedIndices.getRemote()));
}
if (resolvedIndices.getLocal().size() != 1) {
throw illegalArgument("Bulk item should write to exactly 1 index, but request writes to "
+ String.join(",", resolvedIndices.getLocal()));
}
final String resolved = resolvedIndices.getLocal().get(0);
if (indices.contains(resolved) == false) {
throw illegalArgument("Found bulk item that writes to index " + resolved + " but the request writes to " + indices);
}
return resolved;
});
final String itemAction = getAction(item); final String itemAction = getAction(item);
final boolean granted = actionAuthority.computeIfAbsent(itemAction, key -> { final Tuple<String, String> indexAndAction = new Tuple<>(resolvedIndex, itemAction);
final IndicesAccessControl itemAccessControl = permission.authorize(itemAction, indices, metaData, fieldPermissionsCache); final boolean granted = indexActionAuthority.computeIfAbsent(indexAndAction, key -> {
final IndicesAccessControl itemAccessControl = permission.authorize(itemAction, Collections.singleton(resolvedIndex),
metaData, fieldPermissionsCache);
return itemAccessControl.isGranted(); return itemAccessControl.isGranted();
}); });
if (granted == false) { if (granted == false) {
item.abort(index, denial(authentication, itemAction, request)); item.abort(resolvedIndex, denial(authentication, itemAction, request));
} }
} }
} }
private IllegalArgumentException illegalArgument(String message) {
assert false : message;
return new IllegalArgumentException(message);
}
private static String getAction(BulkItemRequest item) { private static String getAction(BulkItemRequest item) {
final DocWriteRequest docWriteRequest = item.request(); final DocWriteRequest docWriteRequest = item.request();
switch (docWriteRequest.opType()) { switch (docWriteRequest.opType()) {

View File

@ -102,7 +102,7 @@ public class IndicesAndAliasesResolver {
return resolveIndicesAndAliases((IndicesRequest) request, metaData, authorizedIndices); return resolveIndicesAndAliases((IndicesRequest) request, metaData, authorizedIndices);
} }
private ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData,
AuthorizedIndices authorizedIndices) { AuthorizedIndices authorizedIndices) {
boolean indicesReplacedWithNoIndices = false; boolean indicesReplacedWithNoIndices = false;
final ResolvedIndices indices; final ResolvedIndices indices;

View File

@ -5,6 +5,7 @@
*/ */
package org.elasticsearch.xpack.security.authz; package org.elasticsearch.xpack.security.authz;
import java.text.SimpleDateFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.HashMap; import java.util.HashMap;
@ -129,7 +130,10 @@ import org.elasticsearch.xpack.security.user.ElasticUser;
import org.elasticsearch.xpack.security.user.SystemUser; import org.elasticsearch.xpack.security.user.SystemUser;
import org.elasticsearch.xpack.security.user.User; import org.elasticsearch.xpack.security.user.User;
import org.elasticsearch.xpack.security.user.XPackUser; import org.elasticsearch.xpack.security.user.XPackUser;
import org.joda.time.Instant;
import org.joda.time.format.DateTimeFormat;
import org.junit.Before; import org.junit.Before;
import org.mockito.Mockito;
import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException;
import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException;
@ -202,7 +206,8 @@ public class AuthorizationServiceTests extends ESTestCase {
private void authorize(Authentication authentication, String action, TransportRequest request) { private void authorize(Authentication authentication, String action, TransportRequest request) {
PlainActionFuture future = new PlainActionFuture(); PlainActionFuture future = new PlainActionFuture();
AuthorizationUtils.AsyncAuthorizer authorizer = new AuthorizationUtils.AsyncAuthorizer(authentication, future, AuthorizationUtils.AsyncAuthorizer authorizer = new AuthorizationUtils.AsyncAuthorizer(authentication, future,
(userRoles, runAsRoles) -> {authorizationService.authorize(authentication, action, request, userRoles, runAsRoles); (userRoles, runAsRoles) -> {
authorizationService.authorize(authentication, action, request, userRoles, runAsRoles);
future.onResponse(null); future.onResponse(null);
}); });
authorizer.authorize(authorizationService); authorizer.authorize(authorizationService);
@ -895,6 +900,59 @@ public class AuthorizationServiceTests extends ESTestCase {
() -> authorize(createAuthentication(userDenied), action, request), action, "userDenied"); () -> authorize(createAuthentication(userDenied), action, request), action, "userDenied");
} }
public void testAuthorizationOfIndividualBulkItems() {
final String action = BulkAction.NAME + "[s]";
final BulkItemRequest[] items = {
new BulkItemRequest(1, new DeleteRequest("concrete-index", "doc", "c1")),
new BulkItemRequest(2, new IndexRequest("concrete-index", "doc", "c2")),
new BulkItemRequest(3, new DeleteRequest("alias-1", "doc", "a1a")),
new BulkItemRequest(4, new IndexRequest("alias-1", "doc", "a1b")),
new BulkItemRequest(5, new DeleteRequest("alias-2", "doc", "a2a")),
new BulkItemRequest(6, new IndexRequest("alias-2", "doc", "a2b"))
};
final ShardId shardId = new ShardId("concrete-index", UUID.randomUUID().toString(), 1);
final TransportRequest request = new BulkShardRequest(shardId, WriteRequest.RefreshPolicy.IMMEDIATE, items);
User user = new User("user", "my-role");
roleMap.put("my-role", new RoleDescriptor("my-role", null, new IndicesPrivileges[] {
IndicesPrivileges.builder().indices("concrete-index").privileges("all").build(),
IndicesPrivileges.builder().indices("alias-1").privileges("index").build(),
IndicesPrivileges.builder().indices("alias-2").privileges("delete").build()
}, null));
mockEmptyMetaData();
authorize(createAuthentication(user), action, request);
verify(auditTrail).accessDenied(user, DeleteAction.NAME, request); // alias-1 delete
verify(auditTrail).accessDenied(user, IndexAction.NAME, request); // alias-2 index
verify(auditTrail).accessGranted(user, action, request); // bulk request is allowed
verifyNoMoreInteractions(auditTrail);
}
public void testAuthorizationOfIndividualBulkItemsWithDateMath() {
final String action = BulkAction.NAME + "[s]";
final BulkItemRequest[] items = {
new BulkItemRequest(1, new IndexRequest("<datemath-{now/M{YYYY}}>", "doc", "dy1")),
new BulkItemRequest(2, new DeleteRequest("<datemath-{now/d{YYYY}}>", "doc", "dy2")), // resolves to same as above
new BulkItemRequest(3, new IndexRequest("<datemath-{now/M{YYYY.MM}}>", "doc", "dm1")),
new BulkItemRequest(4, new DeleteRequest("<datemath-{now/d{YYYY.MM}}>", "doc", "dm2")), // resolves to same as above
};
final ShardId shardId = new ShardId("concrete-index", UUID.randomUUID().toString(), 1);
final TransportRequest request = new BulkShardRequest(shardId, WriteRequest.RefreshPolicy.IMMEDIATE, items);
User user = new User("user", "my-role");
roleMap.put("my-role", new RoleDescriptor("my-role", null, new IndicesPrivileges[] {
IndicesPrivileges.builder().indices("datemath-*").privileges("index").build()
}, null));
mockEmptyMetaData();
authorize(createAuthentication(user), action, request);
verify(auditTrail, Mockito.times(2)).accessDenied(user, DeleteAction.NAME, request); // both deletes should fail
verify(auditTrail).accessGranted(user, action, request); // bulk request is allowed
verifyNoMoreInteractions(auditTrail);
}
private BulkShardRequest createBulkShardRequest(String indexName, TriFunction<String, String, String, DocWriteRequest<?>> req) { private BulkShardRequest createBulkShardRequest(String indexName, TriFunction<String, String, String, DocWriteRequest<?>> req) {
final BulkItemRequest[] items = { new BulkItemRequest(1, req.apply(indexName, "type", "id")) }; final BulkItemRequest[] items = { new BulkItemRequest(1, req.apply(indexName, "type", "id")) };
return new BulkShardRequest(new ShardId(indexName, UUID.randomUUID().toString(), 1), WriteRequest.RefreshPolicy.IMMEDIATE, items); return new BulkShardRequest(new ShardId(indexName, UUID.randomUUID().toString(), 1), WriteRequest.RefreshPolicy.IMMEDIATE, items);

View File

@ -0,0 +1,312 @@
---
setup:
- skip:
features: headers
- do:
cluster.health:
wait_for_status: yellow
- do:
xpack.security.put_role:
name: "mixed_role"
body: >
{
"indices": [
{ "names": ["can_read_1", "can_read_2" ], "privileges": ["read"] },
{ "names": ["can_write_1", "can_write_2", "can_write_3" ], "privileges": ["read", "write"] }
]
}
- do:
xpack.security.put_user:
username: "test_user"
body: >
{
"password" : "x-pack-test-password",
"roles" : [ "mixed_role" ],
"full_name" : "user with mixed privileges to multiple indices"
}
- do:
indices.create:
index: read_index
body:
settings:
index:
number_of_shards: 1
number_of_replicas: 0
mappings:
doc:
properties:
name:
type: "keyword"
- do:
indices.create:
index: write_index_1
body:
settings:
index:
number_of_shards: 1
number_of_replicas: 0
mappings:
doc:
properties:
name:
type: "keyword"
- do:
indices.create:
index: write_index_2
body:
settings:
index:
number_of_shards: 1
number_of_replicas: 0
mappings:
doc:
properties:
name:
type: "keyword"
- do:
indices.put_alias:
index: read_index
name: can_read_1
- do:
indices.put_alias:
index: read_index
name: can_read_2
- do:
indices.put_alias:
index: write_index_1
name: can_write_1
- do:
indices.put_alias:
index: write_index_1
name: can_write_2
- do:
indices.put_alias:
index: write_index_2
name: can_write_3
---
teardown:
- do:
xpack.security.delete_user:
username: "test_user"
ignore: 404
- do:
xpack.security.delete_role:
name: "mixed_role"
ignore: 404
- do:
indices.delete_alias:
index: "read_index"
name: [ "can_read_1", "can_read_2" ]
ignore: 404
- do:
indices.delete_alias:
index: "write_index_1"
name: [ "can_write_1", "can_write_2" ]
ignore: 404
- do:
indices.delete:
index: [ "write_index_1", "read_index" ]
ignore: 404
---
"Test indexing documents into an alias, when permitted":
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
id: 1
index: can_write_1
type: doc
body: >
{
"name" : "doc1"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
id: 2
index: can_write_2
type: doc
body: >
{
"name" : "doc2"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "3"}}'
- '{"name": "doc3"}'
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "4"}}'
- '{"name": "doc4"}'
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "5"}}'
- '{"name": "doc5"}'
- '{"index": {"_index": "can_write_2", "_type": "doc", "_id": "6"}}'
- '{"name": "doc6"}'
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "7"}}'
- '{"name": "doc7"}'
- '{"index": {"_index": "can_write_2", "_type": "doc", "_id": "8"}}'
- '{"name": "doc8"}'
- '{"index": {"_index": "can_write_3", "_type": "doc", "_id": "9"}}'
- '{"name": "doc9"}'
- do: # superuser
search:
index: write_index_1
- match: { hits.total: 8 }
- do: # superuser
search:
index: write_index_2
- match: { hits.total: 1 }
---
"Test indexing documents into an alias, when forbidden":
- do:
catch: forbidden
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
refresh: true
id: 7
index: can_read_1
type: doc
body: >
{
"name" : "doc7"
}
- do:
catch: forbidden
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
refresh: true
id: 8
index: can_read_2
type: doc
body: >
{
"name" : "doc8"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_read_1", "_type": "doc", "_id": "9"}}'
- '{"name": "doc9"}'
- '{"index": {"_index": "can_read_1", "_type": "doc", "_id": "10"}}'
- '{"name": "doc10"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 403 }
- match: { items.1.index.error.type: "security_exception" }
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_read_1", "_type": "doc", "_id": "11"}}'
- '{"name": "doc11"}'
- '{"index": {"_index": "can_read_2", "_type": "doc", "_id": "12"}}'
- '{"name": "doc12"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 403 }
- match: { items.1.index.error.type: "security_exception" }
- do: # superuser
search:
index: read_index
- match: { hits.total: 0 }
---
"Test bulk indexing into an alias when only some are allowed":
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_read_1", "_type": "doc", "_id": "13"}}'
- '{"name": "doc13"}'
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "14"}}'
- '{"name": "doc14"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 201 }
- do: # superuser
search:
index: write_index_1
body: { "query": { "term": { "_id": "14" } } }
- match: { hits.total: 1 }
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
refresh: true
body:
- '{"index": {"_index": "can_read_1", "_type": "doc", "_id": "15"}}'
- '{"name": "doc15"}'
- '{"index": {"_index": "can_write_1", "_type": "doc", "_id": "16"}}'
- '{"name": "doc16"}'
- '{"index": {"_index": "can_read_2", "_type": "doc", "_id": "17"}}'
- '{"name": "doc17"}'
- '{"index": {"_index": "can_write_2", "_type": "doc", "_id": "18"}}'
- '{"name": "doc18"}'
- '{"index": {"_index": "can_write_3", "_type": "doc", "_id": "19"}}'
- '{"name": "doc19"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 201 }
- match: { items.2.index.status: 403 }
- match: { items.2.index.error.type: "security_exception" }
- match: { items.3.index.status: 201 }
- match: { items.4.index.status: 201 }
- do: # superuser
search:
index: write_index_1
body: { "query": { "terms": { "_id": [ "16", "18" ] } } }
- match: { hits.total: 2 }
- do: # superuser
search:
index: write_index_2
body: { "query": { "terms": { "_id": [ "19" ] } } }
- match: { hits.total: 1 }

View File

@ -0,0 +1,138 @@
---
setup:
- skip:
features: headers
- do:
cluster.health:
wait_for_status: yellow
- do:
xpack.security.put_role:
name: "mixed_role"
body: >
{
"indices": [
{ "names": ["read-*" ], "privileges": ["read"] },
{ "names": ["write-*" ], "privileges": ["all"] }
]
}
- do:
xpack.security.put_user:
username: "test_user"
body: >
{
"password" : "x-pack-test-password",
"roles" : [ "mixed_role" ],
"full_name" : "user with mixed privileges to multiple indices"
}
---
teardown:
- do:
xpack.security.delete_user:
username: "test_user"
ignore: 404
- do:
xpack.security.delete_role:
name: "mixed_role"
ignore: 404
---
"Test indexing documents with datemath, when permitted":
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
index:
id: 1
index: "<write-{now/M}>"
type: doc
body: >
{
"name" : "doc1"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
body:
- '{"index": {"_index": "<write-{now/M{YYYY.MM}}>", "_type": "doc", "_id": "2"}}'
- '{"name": "doc2"}'
- '{"index": {"_index": "<write-{now/d}>", "_type": "doc", "_id": "3"}}'
- '{"name": "doc3"}'
- match: { errors: false }
- match: { items.0.index.status: 201 }
- match: { items.1.index.status: 201 }
- do: # superuser
indices.refresh:
index: "_all"
- do: # superuser
search:
index: "write-*"
- match: { hits.total: 3 }
---
"Test indexing documents with datemath, when forbidden":
- do:
catch: forbidden
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
index:
id: 4
index: "<read-{now/M}>"
type: doc
body: >
{
"name" : "doc4"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
body:
- '{"index": {"_index": "<read-{now/M{YYYY.MM}}>", "_type": "doc", "_id": "5"}}'
- '{"name": "doc5"}'
- '{"index": {"_index": "<read-{now/d}>", "_type": "doc", "_id": "6"}}'
- '{"name": "doc6"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 403 }
- match: { items.1.index.error.type: "security_exception" }
- do: # superuser
indices.refresh:
index: "_all"
- do: # superuser
search:
index: "read-*"
- match: { hits.total: 0 }
---
"Test bulk indexing with datemath when only some are allowed":
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
bulk:
body:
- '{"index": {"_index": "<read-{now/M{YYYY}}>", "_type": "doc", "_id": "7"}}'
- '{"name": "doc7"}'
- '{"index": {"_index": "<write-{now/M{YYYY}}>", "_type": "doc", "_id": "8"}}'
- '{"name": "doc8"}'
- match: { errors: true }
- match: { items.0.index.status: 403 }
- match: { items.0.index.error.type: "security_exception" }
- match: { items.1.index.status: 201 }
- do: # superuser
indices.refresh:
index: "_all"
- do: # superuser
search:
index: write-*
body: { "query": { "term": { "_id": "8" } } }
- match: { hits.total: 1 }