Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve shard level request cache efficiency (#69505) #69522

Merged
merged 4 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ public Boolean requestCache() {
return requestCache;
}

@Override
public void requestCache(Boolean requestCache) {
this.requestCache = requestCache;
}

@Override
public Boolean allowPartialSearchResults() {
return allowPartialSearchResults;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public interface ShardSearchRequest {

Boolean requestCache();

void requestCache(Boolean requestCache);

Boolean allowPartialSearchResults();

Scroll scroll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ public Boolean requestCache() {
return shardSearchLocalRequest.requestCache();
}

@Override
public void requestCache(Boolean requestCache) {
shardSearchLocalRequest.requestCache(requestCache);
}

@Override
public Boolean allowPartialSearchResults() {
return shardSearchLocalRequest.allowPartialSearchResults();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ public Boolean requestCache() {
return null;
}

@Override
public void requestCache(Boolean requestCache) {
}

@Override
public Boolean allowPartialSearchResults() {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ public Boolean requestCache() {
return null;
}

@Override
public void requestCache(Boolean requestCache) {
}

@Override
public Boolean allowPartialSearchResults() {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
import org.apache.logging.log4j.Logger;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.transport.TransportActionProxy;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.RequestInfo;
Expand All @@ -36,26 +38,35 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor implements Reques
@Override
public void intercept(RequestInfo requestInfo, AuthorizationEngine authorizationEngine, AuthorizationInfo authorizationInfo,
ActionListener<Void> listener) {
if (requestInfo.getRequest() instanceof IndicesRequest) {
if (requestInfo.getRequest() instanceof IndicesRequest && false == TransportActionProxy.isProxyAction(requestInfo.getAction())) {
IndicesRequest indicesRequest = (IndicesRequest) requestInfo.getRequest();
if (supports(indicesRequest) && licenseState.isDocumentAndFieldLevelSecurityAllowed()) {
final IndicesAccessControl indicesAccessControl =
threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
for (String index : indicesRequest.indices()) {
boolean fieldLevelSecurityEnabled = false;
boolean documentLevelSecurityEnabled = false;
final String[] requestIndices = indicesRequest.indices();
for (String index : requestIndices) {
IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(index);
if (indexAccessControl != null) {
boolean fieldLevelSecurityEnabled = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
boolean documentLevelSecurityEnabled = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
if (fieldLevelSecurityEnabled || documentLevelSecurityEnabled) {
logger.trace("intercepted request for index [{}] with field level access controls [{}] " +
"document level access controls [{}]. disabling conflicting features",
index, fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
return;
fieldLevelSecurityEnabled =
fieldLevelSecurityEnabled || indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
documentLevelSecurityEnabled =
documentLevelSecurityEnabled || indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
if (fieldLevelSecurityEnabled && documentLevelSecurityEnabled) {
break;
}
}
logger.trace("intercepted request for index [{}] without field or document level access controls", index);
}
if (fieldLevelSecurityEnabled || documentLevelSecurityEnabled) {
logger.trace("intercepted request for indices [{}] with field level access controls [{}] " +
"document level access controls [{}]. disabling conflicting features",
Strings.arrayToDelimitedString(requestIndices, ","), fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
return;
}
logger.trace("intercepted request for indices [{}] without field or document level access controls",
Strings.arrayToDelimitedString(requestIndices, ","));
}
}
listener.onResponse(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.internal.ShardSearchTransportRequest;
import org.elasticsearch.threadpool.ThreadPool;

/**
Expand All @@ -25,14 +27,26 @@ public SearchRequestInterceptor(ThreadPool threadPool, XPackLicenseState license
@Override
public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSecurityEnabled, boolean documentLevelSecurityEnabled,
ActionListener<Void> listener) {
final SearchRequest request = (SearchRequest) indicesRequest;
request.requestCache(false);

assert indicesRequest instanceof SearchRequest || indicesRequest instanceof ShardSearchTransportRequest
: "request must be either SearchRequest or ShardSearchTransportRequest";

final SearchSourceBuilder source;
if (indicesRequest instanceof SearchRequest) {
final SearchRequest request = (SearchRequest) indicesRequest;
request.requestCache(false);
source = request.source();
} else {
final ShardSearchTransportRequest request = (ShardSearchTransportRequest) indicesRequest;
request.requestCache(false);
source = request.source();
}

if (documentLevelSecurityEnabled) {
if (request.source() != null && request.source().suggest() != null) {
if (source != null && source.suggest() != null) {
listener.onFailure(new ElasticsearchSecurityException("Suggest isn't supported if document level security is enabled",
RestStatus.BAD_REQUEST));
} else if (request.source() != null && request.source().profile()) {
} else if (source != null && source.profile()) {
listener.onFailure(new ElasticsearchSecurityException("A search request cannot be profiled if document level security " +
"is enabled", RestStatus.BAD_REQUEST));
} else {
Expand All @@ -45,6 +59,6 @@ public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSec

@Override
public boolean supports(IndicesRequest request) {
return request instanceof SearchRequest;
return request instanceof SearchRequest || request instanceof ShardSearchTransportRequest;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ protected String configUsers() {
"user1:" + usersPasswdHashed + "\n" +
"user2:" + usersPasswdHashed + "\n" +
"user3:" + usersPasswdHashed + "\n" +
"user4:" + usersPasswdHashed + "\n";
"user4:" + usersPasswdHashed + "\n" +
"user5:" + usersPasswdHashed + "\n";
}

@Override
Expand All @@ -107,7 +108,8 @@ protected String configUsersRoles() {
"role1:user1,user2,user3\n" +
"role2:user1,user3\n" +
"role3:user2,user3\n" +
"role4:user4\n";
"role4:user4\n" +
"role5:user5\n";
}

@Override
Expand Down Expand Up @@ -140,7 +142,18 @@ protected String configRoles() {
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
// query that can match nested documents
" query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'";
" query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'\n" +
"role5:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: [ 'test' ]\n" +
" privileges: [ read ]\n" +
" query: '{\"term\" : {\"field2\" : \"value2\"}}'\n" +
" - names: [ 'fls-index' ]\n" +
" privileges: [ read ]\n" +
" field_security:\n" +
" grant: [ 'field1', 'other_field', 'suggest_field2' ]\n";

}

@Override
Expand Down Expand Up @@ -922,6 +935,15 @@ public void testSuggesters() throws Exception {
.endObject()).get();
refresh("test");

assertAcked(client().admin().indices().prepareCreate("fls-index")
.setSettings(Settings.builder()
.put("index.number_of_shards", 1)
.put("index.number_of_replicas", 0)
)
.addMapping("type1", "field1", "type=text", "suggest_field1", "type=text", "suggest_field2", "type=completion",
"yet_another", "type=text")
);

// Term suggester:
SearchResponse response = client()
.prepareSearch("test")
Expand All @@ -937,9 +959,12 @@ public void testSuggesters() throws Exception {
assertThat(termSuggestion.getEntries().get(0).getOptions().size(), equalTo(1));
assertThat(termSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

final String[] indices =
randomFrom(Arrays.asList(new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));

Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
.addSuggestion("_name1", new TermSuggestionBuilder("suggest_field1"))
Expand All @@ -962,7 +987,7 @@ public void testSuggesters() throws Exception {
assertThat(phraseSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch("test")
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
Expand All @@ -986,7 +1011,7 @@ public void testSuggesters() throws Exception {
assertThat(completionSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch("test")
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
Expand Down Expand Up @@ -1017,6 +1042,15 @@ public void testProfile() throws Exception {
.endObject()).get();
refresh("test");

assertAcked(client().admin().indices().prepareCreate("fls-index")
.setSettings(Settings.builder()
.put("index.number_of_shards", 1)
.put("index.number_of_replicas", 0)
)
.addMapping("type1", "field1", "type=text", "suggest_field1", "type=text", "suggest_field2", "type=completion",
"yet_another", "type=text")
);

SearchResponse response = client()
.prepareSearch("test")
.setProfile(true)
Expand All @@ -1033,9 +1067,11 @@ public void testProfile() throws Exception {
// ProfileResult profileResult = queryProfileShardResult.getQueryResults().get(0);
// assertThat(profileResult.getLuceneDescription(), equalTo("(other_field:value)^0.8"));

final String[] indices =
randomFrom(Arrays.asList(new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));
Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.setProfile(true)
.setQuery(new FuzzyQueryBuilder("other_field", "valeu"))
.get());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
setup:
- skip:
features: headers

- do:
cluster.health:
wait_for_status: yellow

- do:
xpack.security.put_user:
username: "dls_fls_user"
body: >
{
"password": "s3krit-password",
"roles" : [ "dls_fls_role" ]
}

---
teardown:
- do:
xpack.security.delete_user:
username: "dls_fls_user"
ignore: 404

---
"Search with document and field level security":
- do:
search:
rest_total_hits_as_int: true
request_cache: true
index: my_remote_cluster:shared_index

- match: { hits.total: 2}
- length: { hits.hits.0._source: 3 }
- match: { hits.hits.0._source.secret: "sesame" }

- do:
headers: { Authorization: "Basic ZGxzX2Zsc191c2VyOnMza3JpdC1wYXNzd29yZA==" }
search:
rest_total_hits_as_int: true
request_cache: true
index: my_remote_cluster:shared_index

- match: { hits.total: 1}
- length: { hits.hits.0._source: 2 }
- is_true: hits.hits.0._source.public
- match: { hits.hits.0._source.name: "doc 1" }
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ setup:
}
]
}

- do:
xpack.security.put_role:
name: "dls_fls_role"
body: >
{
"cluster": ["monitor"],
"indices": [
{
"names": ["shared_index"],
"privileges": ["read", "read_cross_cluster"],
"query": "{ \"term\": { \"public\" : true } }",
"field_security": { "grant": ["*"], "except": ["secret"] }
}
]
}
---
"Index data and search on the remote cluster":

Expand Down Expand Up @@ -196,3 +212,21 @@ setup:
"roles" : [ ]
}
- match: { user: { created: false } }

- do:
indices.create:
index: shared_index
body:
settings:
index:
number_of_shards: 1
number_of_replicas: 0

- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "shared_index", "_id": 1, "_type": "test_type"}}'
- '{"public": true, "name": "doc 1", "secret": "sesame"}'
- '{"index": {"_index": "shared_index", "_id": 2, "_type": "test_type"}}'
- '{"public": false, "name": "doc 2", "secret": "sesame"}'