Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 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 @@ -47,6 +47,7 @@
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -1388,12 +1389,8 @@ public void testDlsFls() throws Exception {
Map.of(dataIndexName, Set.of("@timestamp", "age"))
);

// FLS sort of applies to failure store
// TODO this will change with FLS handling
assertSearchResponseContainsExpectedIndicesAndFields(
performRequest(user, new Search("test1::failures").toSearchRequest()),
Map.of(failureIndexName, Set.of("@timestamp"))
);
// FLS does not apply to failure store
expectFlsDlsError(() -> performRequest(user, new Search("test1::failures").toSearchRequest()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've hit the ready button too soon. I still want to test cover the case when users have direct access to .fs-* indices and accessing without ::failures selector.


upsertRole(Strings.format("""
{
Expand Down Expand Up @@ -1422,12 +1419,8 @@ public void testDlsFls() throws Exception {
Map.of(dataIndexName, Set.of("@timestamp", "age"))
);

// FLS sort of applies to failure store
// TODO this will change with FLS handling
assertSearchResponseContainsExpectedIndicesAndFields(
performRequest(user, new Search("test1::failures").toSearchRequest()),
Map.of(failureIndexName, Set.of("@timestamp"))
);
// FLS does not apply to failure store
expectFlsDlsError(() -> performRequest(user, new Search("test1::failures").toSearchRequest()));

upsertRole("""
{
Expand Down Expand Up @@ -1473,7 +1466,8 @@ public void testDlsFls() throws Exception {
}""", role);
// DLS applies and no docs match the query
expectSearch(user, new Search(randomFrom("test1", "test1::data")));
expectSearch(user, new Search("test1::failures"));
// DLS is not applicable to failure store
expectFlsDlsError(() -> performRequest(user, new Search("test1::failures").toSearchRequest()));

upsertRole("""
{
Expand All @@ -1488,7 +1482,8 @@ public void testDlsFls() throws Exception {
}""", role);
// DLS applies and doc matches the query
expectSearch(user, new Search(randomFrom("test1", "test1::data")), dataIndexDocId);
expectSearch(user, new Search("test1::failures"));
// DLS is not applicable to failure store
expectFlsDlsError(() -> performRequest(user, new Search("test1::failures").toSearchRequest()));

upsertRole("""
{
Expand Down Expand Up @@ -1827,4 +1822,14 @@ private Tuple<String, String> getSingleDataAndFailureIndices(String dataStreamNa
assertThat(indices.v2().size(), equalTo(1));
return new Tuple<>(indices.v1().get(0), indices.v2().get(0));
}

private static void expectFlsDlsError(ThrowingRunnable runnable) {
var exception = expectThrows(ResponseException.class, runnable);
assertThat(
exception.getMessage(),
containsString(
"Failure store access is not allowed for users who have field or document level security enabled on one of the indices"
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.bootstrap.BootstrapCheck;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexTemplateMetadata;
import org.elasticsearch.cluster.metadata.ProjectId;
Expand Down Expand Up @@ -328,6 +329,7 @@
import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache;
import org.elasticsearch.xpack.security.authz.interceptor.BulkShardRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.DlsFlsLicenseRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.FailureStoreRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.IndicesAliasesRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.ResizeRequestInterceptor;
Expand Down Expand Up @@ -1139,6 +1141,9 @@ Collection<Object> createComponents(
new ValidateRequestInterceptor(threadPool, getLicenseState())
)
);
if (DataStream.isFailureStoreFeatureFlagEnabled()) {
requestInterceptors.add(new FailureStoreRequestInterceptor(threadPool, getLicenseState()));
}
}
requestInterceptors = Collections.unmodifiableSet(requestInterceptors);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.authz.interceptor;

import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.action.support.IndexComponentSelector;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;

import java.util.Map;

public class FailureStoreRequestInterceptor extends FieldAndDocumentLevelSecurityRequestInterceptor {

public FailureStoreRequestInterceptor(ThreadPool threadPool, XPackLicenseState licenseState) {
super(threadPool.getThreadContext(), licenseState);
}

@Override
void disableFeatures(
IndicesRequest indicesRequest,
Map<String, IndicesAccessControl.IndexAccessControl> indicesAccessControlByIndex,
ActionListener<Void> listener
) {
for (var indexAccessControl : indicesAccessControlByIndex.entrySet()) {
if (hasFailuresSelectorSuffix(indexAccessControl.getKey()) && hasDlsFlsPermissions(indexAccessControl.getValue())) {
listener.onFailure(
new ElasticsearchSecurityException(
"Failure store access is not allowed for users who have "
+ "field or document level security enabled on one of the indices",
RestStatus.BAD_REQUEST
)
);
return;
}
}
listener.onResponse(null);
}

@Override
boolean supports(IndicesRequest request) {
if (request.indicesOptions().allowSelectors()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want request.indicesOptions().allowSelectors() here as there may be cases where you can access the failure index but selectors are not allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! This started with intention to prevent access with ::failures selector but turned into "let's prevent access to failure indices regardless if selectors are used or not". I'll adjust this.

for (String index : request.indices()) {
if (hasFailuresSelectorSuffix(index)) {
return true;
}
}
}
return false;
}

private boolean hasFailuresSelectorSuffix(String name) {
return IndexNameExpressionResolver.hasSelectorSuffix(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use IndexNameExpressionResolver.hasSelector(name, IndexComponentSelector.FAILURES) here instead, to avoid splitting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Fine to include the bigger check with splitting as an assertion since it does additional validation, but hasSelector should be faster so better suited for prod-code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Will make the change.

&& IndexComponentSelector.getByKey(
IndexNameExpressionResolver.splitSelectorExpression(name).v2()
) == IndexComponentSelector.FAILURES;
}

private boolean hasDlsFlsPermissions(IndicesAccessControl.IndexAccessControl indexAccessControl) {
return indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions()
|| indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
}

}