From 4f86f6fb38313a7cdd9e50f9eecd630c32b164e5 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 3 Jul 2020 12:16:43 +0200 Subject: [PATCH] Submit async search to not require read privilege (#58942) When we execute search against remote indices, the remote indices are authorized on the remote cluster and not on the CCS cluster. When we introduced submit async search we added a check that requires that the user running it has the privilege to execute it on some index. That prevents users from executing async searches against remote indices unless they also have read access on the CCS cluster, which is common when the CCS cluster holds no data. The solution is to let the submit async search go through as we already do for get and delete async search. Note that the inner search action will still check that the user can access local indices, and remote indices on the remote cluster, like search always does. --- .../xpack/security/authz/RBACEngine.java | 14 +-- .../build.gradle | 2 +- .../test/multi_cluster/10_basic.yml | 114 +++++++++++++++++- .../test/remote_cluster/10_basic.yml | 23 ++++ 4 files changed, 136 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index 8098235c0ff..b7de9cbb04d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -32,8 +32,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.transport.TransportActionProxy; import org.elasticsearch.transport.TransportRequest; -import org.elasticsearch.xpack.core.eql.EqlAsyncActionNames; import org.elasticsearch.xpack.core.async.DeleteAsyncResultAction; +import org.elasticsearch.xpack.core.eql.EqlAsyncActionNames; import org.elasticsearch.xpack.core.search.action.GetAsyncSearchAction; import org.elasticsearch.xpack.core.search.action.SubmitAsyncSearchAction; import org.elasticsearch.xpack.core.security.action.GetApiKeyAction; @@ -268,16 +268,8 @@ public class RBACEngine implements AuthorizationEngine { listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.ALLOW_NO_INDICES)); } } else if (isAsyncRelatedAction(action)) { - if (SubmitAsyncSearchAction.NAME.equals(action)) { - // we check if the user has any indices permission when submitting an async-search request in order to be - // able to fail the request early. Fine grained index-level permissions are handled by the search action - // that is triggered internally by the submit API. - authorizeIndexActionName(action, authorizationInfo, null, listener); - } else { - // async-search actions other than submit have a custom security layer that checks if the current user is - // the same as the user that submitted the original request so we can skip security here. - listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.ALLOW_NO_INDICES)); - } + //index-level permissions are handled by the search action that is triggered internally by the submit API. + listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.ALLOW_NO_INDICES)); } else { assert false : "only scroll and async-search related requests are known indices api that don't " + "support retrieving the indices they relate to"; diff --git a/x-pack/qa/multi-cluster-search-security/build.gradle b/x-pack/qa/multi-cluster-search-security/build.gradle index 9a5eab3147f..0b440faac2e 100644 --- a/x-pack/qa/multi-cluster-search-security/build.gradle +++ b/x-pack/qa/multi-cluster-search-security/build.gradle @@ -9,7 +9,7 @@ dependencies { restResources { restApi { - includeXpack 'security' + includeXpack 'security', 'async_search' } } diff --git a/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yml b/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yml index 1537d361a3a..083b41ba5c4 100644 --- a/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yml +++ b/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yml @@ -22,11 +22,26 @@ setup: "cluster": [], "indices": [ { - "names": ["local_index", "my_remote_cluster:test_i*", "my_remote_cluster:aliased_test_index", "test_remote_cluster:test_i*", "my_remote_cluster:secure_alias"], + "names": ["local_index"], "privileges": ["read"] } ] } + + - do: + security.put_user: + username: "remote" + body: > + { + "password": "s3krit", + "roles" : [ "remote_ccs" ] + } + - do: + security.put_role: + name: "remote_ccs" + body: > + { + } --- teardown: - do: @@ -107,6 +122,58 @@ teardown: - match: { aggregations.cluster.buckets.0.key: "remote_cluster" } - match: { aggregations.cluster.buckets.0.doc_count: 6 } + - do: + headers: { Authorization: "Basic am9lOnMza3JpdA==" } + async_search.submit: + index: local_index,my_remote_cluster:test_index + wait_for_completion_timeout: 10s + keep_on_completion: true + body: + query: + term: + f1: remote_cluster + aggs: + cluster: + terms: + field: f1.keyword + + - set: { id: id } + - match: { is_partial: false } + - match: { response._clusters.total: 2} + - match: { response._clusters.successful: 2} + - match: { response._clusters.skipped: 0} + - match: { response._shards.total: 5 } + - match: { response.hits.total.value: 6} + - match: { response.hits.total.relation: eq} + - match: { response.hits.hits.0._index: "my_remote_cluster:test_index"} + - length: { response.aggregations.cluster.buckets: 1 } + - match: { response.aggregations.cluster.buckets.0.key: "remote_cluster" } + - match: { response.aggregations.cluster.buckets.0.doc_count: 6 } + + - do: + headers: { Authorization: "Basic am9lOnMza3JpdA==" } + async_search.get: + id: "$id" + + - match: { is_partial: false } + - match: { response._clusters.total: 2} + - match: { response._clusters.successful: 2} + - match: { response._clusters.skipped: 0} + - match: { response._shards.total: 5 } + - match: { response.hits.total.value: 6} + - match: { response.hits.total.relation: eq} + - match: { response.hits.hits.0._index: "my_remote_cluster:test_index"} + - length: { response.aggregations.cluster.buckets: 1 } + - match: { response.aggregations.cluster.buckets.0.key: "remote_cluster" } + - match: { response.aggregations.cluster.buckets.0.doc_count: 6 } + + - do: + headers: { Authorization: "Basic am9lOnMza3JpdA==" } + async_search.delete: + id: "$id" + + - match: { acknowledged: true } + - do: headers: { Authorization: "Basic am9lOnMza3JpdA==" } search: @@ -130,7 +197,7 @@ teardown: # Test wildcard in cluster name - do: - headers: { Authorization: "Basic am9lOnMza3JpdA==" } + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } search: rest_total_hits_as_int: true index: "my_*:test_index" @@ -201,7 +268,7 @@ teardown: # Test wildcard that matches multiple (two) cluster names - do: - headers: { Authorization: "Basic am9lOnMza3JpdA==" } + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } search: rest_total_hits_as_int: true index: "*_remote_cluster:test_ind*" @@ -216,7 +283,7 @@ teardown: "Search an filtered alias on the remote cluster": - do: - headers: { Authorization: "Basic am9lOnMza3JpdA==" } + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } search: rest_total_hits_as_int: true index: my_remote_cluster:aliased_test_index @@ -233,7 +300,7 @@ teardown: "Search across clusters via a secured alias": - do: - headers: { Authorization: "Basic am9lOnMza3JpdA==" } + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } search: rest_total_hits_as_int: true index: my_remote_cluster:secure_alias # TODO make this a wildcard once @@ -246,3 +313,40 @@ teardown: - is_true: hits.hits.0._source.secure - match: { hits.hits.0._index: "my_remote_cluster:secured_via_alias" } +--- +"Async search against filtered alias on the remote cluster": + + - do: + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } + async_search.submit: + index: my_remote_cluster:aliased_test_index + wait_for_completion_timeout: 10s + keep_on_completion: true + + - set: { id: id } + - match: { is_partial: false } + - match: { response._clusters.total: 1} + - match: { response._clusters.successful: 1} + - match: { response._shards.total: 3 } + - length: { response.hits.hits: 2 } + - match: { response.hits.hits.0._source.filter_field: 1 } + - match: { response.hits.hits.0._index: "my_remote_cluster:test_index" } + + - do: + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } + async_search.get: + id: "$id" + + - match: { is_partial: false } + - is_true: response._clusters + - match: { response._shards.total: 3 } + - length: { response.hits.hits: 2 } + - match: { response.hits.hits.0._source.filter_field: 1 } + - match: { response.hits.hits.0._index: "my_remote_cluster:test_index" } + + - do: + headers: { Authorization: "Basic cmVtb3RlOnMza3JpdA==" } + async_search.delete: + id: "$id" + + - match: { acknowledged: true } diff --git a/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml b/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml index 32549c586d3..4cd5a8e4344 100644 --- a/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml +++ b/x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml @@ -28,6 +28,29 @@ setup: } ] } + + - do: + security.put_user: + username: "remote" + body: > + { + "password": "s3krit", + "roles" : [ "remote_ccs" ] + } + - do: + security.put_role: + name: "remote_ccs" + body: > + { + "cluster": ["monitor"], + "indices": [ + { + "names": ["single_doc_index", "secure_alias", "test_index", "aliased_test_index", "field_caps_index_1", + "field_caps_index_3"], + "privileges": ["read", "read_cross_cluster"] + } + ] + } --- "Index data and search on the remote cluster":