Skip to content

Commit

Permalink
Lazy compute the index access control (#88708)
Browse files Browse the repository at this point in the history
The access control was always computed eagerly, even in cases when
it was not necessary (e.g. shard is not accessed on the node doing
the authorization or in cases when authorization is denied).

This commit defers the computation to when it's really needed in order
to avoid that the actual work is done on the network worker thread.
  • Loading branch information
slobodanadamovic committed Jan 17, 2023
1 parent 30faac8 commit 06552d0
Show file tree
Hide file tree
Showing 17 changed files with 724 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.elasticsearch.xpack.core.security.authz.accesscontrol;

import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.util.CachedSupplier;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.Nullable;
Expand All @@ -23,6 +24,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier;

/**
* Encapsulates the field and document permissions per concrete index based on the current request.
Expand All @@ -36,11 +38,19 @@ public class IndicesAccessControl {
public static final IndicesAccessControl DENIED = new IndicesAccessControl(false, Collections.emptyMap());

private final boolean granted;
private final Map<String, IndexAccessControl> indexPermissions;
private final CachedSupplier<Map<String, IndexAccessControl>> indexPermissionsSupplier;

public IndicesAccessControl(boolean granted, Map<String, IndexAccessControl> indexPermissions) {
this(granted, () -> Objects.requireNonNull(indexPermissions));
}

public IndicesAccessControl(boolean granted, Supplier<Map<String, IndexAccessControl>> indexPermissionsSupplier) {
this.granted = granted;
this.indexPermissions = Objects.requireNonNull(indexPermissions);
this.indexPermissionsSupplier = new CachedSupplier<>(Objects.requireNonNull(indexPermissionsSupplier));
}

protected IndicesAccessControl(IndicesAccessControl copy) {
this(copy.granted, copy.indexPermissionsSupplier);
}

/**
Expand All @@ -49,7 +59,7 @@ public IndicesAccessControl(boolean granted, Map<String, IndexAccessControl> ind
*/
@Nullable
public IndexAccessControl getIndexPermissions(String index) {
return indexPermissions.get(index);
return this.getAllIndexPermissions().get(index);
}

public boolean hasIndexPermissions(String index) {
Expand All @@ -66,7 +76,7 @@ public boolean isGranted() {
public DlsFlsUsage getFieldAndDocumentLevelSecurityUsage() {
boolean hasFls = false;
boolean hasDls = false;
for (IndexAccessControl iac : indexPermissions.values()) {
for (IndexAccessControl iac : this.getAllIndexPermissions().values()) {
if (iac.fieldPermissions.hasFieldLevelSecurity()) {
hasFls = true;
}
Expand Down Expand Up @@ -99,7 +109,16 @@ public List<String> getIndicesWithDocumentLevelSecurity() {
}

private List<String> getIndexNames(Predicate<IndexAccessControl> predicate) {
return indexPermissions.entrySet().stream().filter(entry -> predicate.test(entry.getValue())).map(Map.Entry::getKey).toList();
return this.getAllIndexPermissions()
.entrySet()
.stream()
.filter(entry -> predicate.test(entry.getValue()))
.map(Map.Entry::getKey)
.toList();
}

private Map<String, IndexAccessControl> getAllIndexPermissions() {
return this.indexPermissionsSupplier.get();
}

public enum DlsFlsUsage {
Expand Down Expand Up @@ -246,22 +265,26 @@ public IndicesAccessControl limitIndicesAccessControl(IndicesAccessControl limit
} else {
isGranted = false;
}
Set<String> indexes = indexPermissions.keySet();
Set<String> otherIndexes = limitedByIndicesAccessControl.indexPermissions.keySet();
Set<String> commonIndexes = Sets.intersection(indexes, otherIndexes);

Map<String, IndexAccessControl> indexPermissionsMap = Maps.newMapWithExpectedSize(commonIndexes.size());
for (String index : commonIndexes) {
IndexAccessControl indexAccessControl = getIndexPermissions(index);
IndexAccessControl limitedByIndexAccessControl = limitedByIndicesAccessControl.getIndexPermissions(index);
indexPermissionsMap.put(index, indexAccessControl.limitIndexAccessControl(limitedByIndexAccessControl));
}
return new IndicesAccessControl(isGranted, indexPermissionsMap);

final Supplier<Map<String, IndexAccessControl>> limitedIndexPermissions = () -> {
Set<String> indexes = this.getAllIndexPermissions().keySet();
Set<String> otherIndexes = limitedByIndicesAccessControl.getAllIndexPermissions().keySet();
Set<String> commonIndexes = Sets.intersection(indexes, otherIndexes);

Map<String, IndexAccessControl> indexPermissionsMap = Maps.newMapWithExpectedSize(commonIndexes.size());
for (String index : commonIndexes) {
IndexAccessControl indexAccessControl = getIndexPermissions(index);
IndexAccessControl limitedByIndexAccessControl = limitedByIndicesAccessControl.getIndexPermissions(index);
indexPermissionsMap.put(index, indexAccessControl.limitIndexAccessControl(limitedByIndexAccessControl));
}
return indexPermissionsMap;
};
return new IndicesAccessControl(isGranted, limitedIndexPermissions);
}

@Override
public String toString() {
return "IndicesAccessControl{" + "granted=" + granted + ", indexPermissions=" + indexPermissions + '}';
return "IndicesAccessControl{" + "granted=" + granted + ", indexPermissions=" + indexPermissionsSupplier.get() + '}';
}

public static IndicesAccessControl allowAll() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Predicate;
import java.util.function.Supplier;

Expand Down Expand Up @@ -373,20 +374,20 @@ public IndicesAccessControl authorize(
}

final Map<String, IndexResource> resources = Maps.newMapWithExpectedSize(requestedIndicesOrAliases.size());
int totalResourceCount = 0;
final AtomicInteger totalResourceCountHolder = new AtomicInteger(0);

for (String indexOrAlias : requestedIndicesOrAliases) {
final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(indexOrAlias));
resources.put(resource.name, resource);
totalResourceCount += resource.size();
totalResourceCountHolder.getAndAdd(resource.size());
}

final boolean overallGranted = isActionGranted(action, resources);

final Map<String, IndicesAccessControl.IndexAccessControl> indexPermissions = buildIndicesAccessControl(
final Supplier<Map<String, IndicesAccessControl.IndexAccessControl>> indexPermissions = () -> buildIndicesAccessControl(
action,
resources,
totalResourceCount,
totalResourceCountHolder.get(),
fieldPermissionsCache
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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.integration;

import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.license.GetFeatureUsageRequest;
import org.elasticsearch.license.GetFeatureUsageResponse;
import org.elasticsearch.license.TransportGetFeatureUsageAction;
import org.elasticsearch.test.SecurityIntegTestCase;

import java.util.HashSet;
import java.util.Set;

import static org.elasticsearch.xpack.core.security.SecurityField.DOCUMENT_LEVEL_SECURITY_FEATURE;
import static org.elasticsearch.xpack.core.security.SecurityField.FIELD_LEVEL_SECURITY_FEATURE;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.in;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

abstract class AbstractDocumentAndFieldLevelSecurityTests extends SecurityIntegTestCase {

private static final Set<String> DLS_FLS_FEATURE_NAMES = Set.of(
DOCUMENT_LEVEL_SECURITY_FEATURE.getName(),
FIELD_LEVEL_SECURITY_FEATURE.getName()
);

protected static void assertOnlyDlsTracked() {
Set<String> features = fetchFeatureUsageFromAllNodes();
assertThat(DOCUMENT_LEVEL_SECURITY_FEATURE.getName(), is(in(features)));
assertThat(FIELD_LEVEL_SECURITY_FEATURE.getName(), not(is(in(features))));
}

protected static void assertOnlyFlsTracked() {
Set<String> features = fetchFeatureUsageFromAllNodes();
assertThat(FIELD_LEVEL_SECURITY_FEATURE.getName(), is(in(features)));
assertThat(DOCUMENT_LEVEL_SECURITY_FEATURE.getName(), not(is(in(features))));
}

protected static void assertDlsFlsTracked() {
assertThat(DLS_FLS_FEATURE_NAMES, everyItem(is(in(fetchFeatureUsageFromAllNodes()))));
}

protected static void assertDlsFlsNotTrackedAcrossAllNodes() {
assertThat(fetchFeatureUsageFromAllNodes(), not(containsInAnyOrder(DLS_FLS_FEATURE_NAMES)));
}

protected static void assertDlsFlsNotTrackedOnCoordOnlyNode() {
assertThat(fetchFeatureUsageFromCoordOnlyNode(), not(containsInAnyOrder(DLS_FLS_FEATURE_NAMES)));
}

protected static Set<String> fetchFeatureUsageFromAllNodes() {
final Set<String> result = new HashSet<>();
// Nodes are chosen at random when test is executed,
// hence we have to aggregate feature usage across all nodes in the cluster.
Set.of(internalCluster().getNodeNames()).stream().forEach(node -> { result.addAll(fetchFeatureUsageFromNode(client(node))); });
return result;
}

protected static Set<String> fetchFeatureUsageFromCoordOnlyNode() {
return fetchFeatureUsageFromNode(internalCluster().coordOnlyNodeClient());
}

protected static Set<String> fetchFeatureUsageFromNode(Client client) {
final Set<String> result = new HashSet<>();
PlainActionFuture<GetFeatureUsageResponse> listener = new PlainActionFuture<>();
client.execute(TransportGetFeatureUsageAction.TYPE, new GetFeatureUsageRequest(), listener);
GetFeatureUsageResponse response = listener.actionGet();
for (var feature : response.getFeatures()) {
result.add(feature.getName());
}
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* 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.integration;

import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Strings;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.SecuritySettingsSourceField;
import org.elasticsearch.xpack.core.XPackSettings;

import java.util.Collections;

import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER;
import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;

@ESIntegTestCase.ClusterScope(numClientNodes = 1)
public class DocumentLevelSecurityFeatureUsageTests extends AbstractDocumentAndFieldLevelSecurityTests {

protected static final SecureString USERS_PASSWD = SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;

@Override
protected String configUsers() {
final String usersPasswdHashed = new String(getFastStoredHashAlgoForTests().hash(USERS_PASSWD));
return super.configUsers() + Strings.format("""
user1:%s
user2:%s
""", usersPasswdHashed, usersPasswdHashed);
}

@Override
protected String configUsersRoles() {
return super.configUsersRoles() + """
role1:user1
role2:user1
role3:user2
""";
}

@Override
protected String configRoles() {
return super.configRoles() + """
role1:
cluster: [ none ]
indices:
- names: '*'
privileges: [ none ]
role2:
cluster:
- all
indices:
- names: '*'
privileges:
- all
query:
term:
field1: value1
role3:
cluster: [ all ]
indices:
- names: '*'
privileges: [ ALL ]
""";
}

@Override
public Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal, otherSettings))
.put(XPackSettings.DLS_FLS_ENABLED.getKey(), true)
.put(XPackSettings.AUDIT_ENABLED.getKey(), false) // Just to make logs less noisy
.build();
}

public void testDlsFeatureUsageTracking() throws Exception {
assertAcked(
client().admin().indices().prepareCreate("test").setMapping("field1", "type=text", "field2", "type=text", "field3", "type=text")
);
client().prepareIndex("test").setId("1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get();
client().prepareIndex("test").setId("2").setSource("field2", "value2").setRefreshPolicy(IMMEDIATE).get();
client().prepareIndex("test").setId("3").setSource("field3", "value3").setRefreshPolicy(IMMEDIATE).get();

SearchResponse response = internalCluster().coordOnlyNodeClient()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD)))
.prepareSearch("test")
.setQuery(randomBoolean() ? QueryBuilders.termQuery("field1", "value1") : QueryBuilders.matchAllQuery())
.get();
assertHitCount(response, 1);
assertSearchHits(response, "1");

// coordinating only node should not tack DLS/FLS feature usage
assertDlsFlsNotTrackedOnCoordOnlyNode();
// only DLS feature should have been tracked across all data nodes
assertOnlyDlsTracked();
}

public void testDlsFlsFeatureUsageNotTracked() {
assertAcked(
client().admin().indices().prepareCreate("test").setMapping("id", "type=keyword", "field1", "type=text", "field2", "type=text")
);
client().prepareIndex("test").setId("1").setSource("id", "1", "field1", "value1").setRefreshPolicy(IMMEDIATE).get();
client().prepareIndex("test").setId("2").setSource("id", "2", "field2", "value2").setRefreshPolicy(IMMEDIATE).get();

// Running a search with user2 (which has role3 without DLS/FLS) should not trigger feature tracking.
SearchResponse response = internalCluster().coordOnlyNodeClient()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.get();
assertHitCount(response, 2);
assertSearchHits(response, "1", "2");

assertDlsFlsNotTrackedAcrossAllNodes();
}

}

0 comments on commit 06552d0

Please sign in to comment.