Skip to content

Commit

Permalink
AllowAll for indicesAccessControl (#78498)
Browse files Browse the repository at this point in the history 
This PR adds a fast path for computing indicesAccessControl if 
the role has all access to all indices.

A role is considered to have all access to all indices if any of its
IndicesPermission#Group satisfy the following criteria:

1. Any of the index patterns is a simple match-all wildcard, i.e. "*"
2. It allows access to restricted indices
3. It grants the "all" index privilege
4. It has no DLS or FLS

An example of such role is the builtin superuser role.

Note the fastpath does not apply to roles that have "effective" but not
direct "all access of all indices". For example, if the "effective"
access is achieved by combining multiple Groups belong to the role, or
combining multiple index patterns within a single Group. This fast path
is provided so that we have a reference baseline for authorization
related performance which is useful for both production use and
troubleshooting.
  • Loading branch information
@ywangd
ywangd committed Oct 18, 2021
1 parent 778c569 commit 5ae1518
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 86 deletions.
38 changes: 37 additions & 1 deletion .../java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
*/
public class IndicesAccessControl {

public static final IndicesAccessControl ALLOW_ALL = new IndicesAccessControl(true, Collections.emptyMap());
public static final IndicesAccessControl ALLOW_NO_INDICES = new IndicesAccessControl(true,
Collections.singletonMap(IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER,
new IndicesAccessControl.IndexAccessControl(true, new FieldPermissions(), DocumentPermissions.allowAll())));
Expand Down Expand Up @@ -249,6 +248,12 @@ public int hashCode() {
* @return {@link IndicesAccessControl}
*/
public IndicesAccessControl limitIndicesAccessControl(IndicesAccessControl limitedByIndicesAccessControl) {
if (this instanceof AllowAllIndicesAccessControl) {
return limitedByIndicesAccessControl;
} else if (limitedByIndicesAccessControl instanceof AllowAllIndicesAccessControl) {
return this;
}

final boolean granted;
if (this.granted == limitedByIndicesAccessControl.granted) {
granted = this.granted;
Expand All @@ -275,4 +280,35 @@ public String toString() {
", indexPermissions=" + indexPermissions +
'}';
}

public static IndicesAccessControl allowAll() {
return AllowAllIndicesAccessControl.INSTANCE;
}

private static class AllowAllIndicesAccessControl extends IndicesAccessControl {

private static final IndicesAccessControl INSTANCE = new AllowAllIndicesAccessControl();

private final IndexAccessControl allowAllIndexAccessControl = new IndexAccessControl(true, null, null);

private AllowAllIndicesAccessControl() {
super(true, null);
}

@Override
public IndexAccessControl getIndexPermissions(String index) {
return allowAllIndexAccessControl;
}

@Override
public boolean isGranted() {
return true;
}

@Override
public Collection<?> getDeniedIndices() {
return Set.of();
}
}

}
22 changes: 19 additions & 3 deletions ...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,16 @@ public boolean canHaveBackingIndices() {
/**
* Authorizes the provided action against the provided indices, given the current cluster metadata
*/
public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
public IndicesAccessControl authorize(
String action,
Set<String> requestedIndicesOrAliases,
Map<String, IndexAbstraction> lookup,
FieldPermissionsCache fieldPermissionsCache
) {
// Short circuit if the indicesPermission allows all access to every index
if (Arrays.stream(groups).anyMatch(Group::isTotal)) {
return IndicesAccessControl.allowAll();
}

final List<IndexResource> resources = new ArrayList<>(requestedIndicesOrAliases.size());
int totalResourceCount = 0;
Expand Down Expand Up @@ -469,6 +473,7 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
}
}

boolean overallGranted = true;
Map<String, IndicesAccessControl.IndexAccessControl> indexPermissions = new HashMap<>(grantedBuilder.size());
for (Map.Entry<String, Boolean> entry : grantedBuilder.entrySet()) {
String index = entry.getKey();
Expand All @@ -488,10 +493,13 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
} else {
fieldPermissions = FieldPermissions.DEFAULT;
}
if (entry.getValue() == false) {
overallGranted = false;
}
indexPermissions.put(index, new IndicesAccessControl.IndexAccessControl(entry.getValue(), fieldPermissions,
(roleQueries != null) ? DocumentPermissions.filteredBy(roleQueries) : DocumentPermissions.allowAll()));
}
return unmodifiableMap(indexPermissions);
return new IndicesAccessControl(overallGranted, unmodifiableMap(indexPermissions));
}

private boolean isConcreteRestrictedIndex(String indexPattern) {
Expand All @@ -515,7 +523,7 @@ public static class Group {
private final IndexPrivilege privilege;
private final Predicate<String> actionMatcher;
private final String[] indices;
private final Predicate<String> indexNameMatcher;
private final StringMatcher indexNameMatcher;
private final Supplier<Automaton> indexNameAutomaton;
private final FieldPermissions fieldPermissions;
private final Set<BytesReference> query;
Expand Down Expand Up @@ -583,6 +591,14 @@ public boolean allowRestrictedIndices() {
public Automaton getIndexMatcherAutomaton() {
return indexNameAutomaton.get();
}

boolean isTotal() {
return allowRestrictedIndices
&& indexNameMatcher.isTotal()
&& privilege == IndexPrivilege.ALL
&& query == null
&& false == fieldPermissions.hasFieldLevelSecurity();
}
}

private static class DocumentLevelPermissions {
Expand Down
1 change: 0 additions & 1 deletion ...ore/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public IndicesAccessControl authorize(String action, Set<String> requestedIndice
super.authorize(action, requestedIndicesOrAliases, aliasAndIndexLookup, fieldPermissionsCache);
IndicesAccessControl limitedByIndicesAccessControl = limitedBy.authorize(action, requestedIndicesOrAliases, aliasAndIndexLookup,
fieldPermissionsCache);

return indicesAccessControl.limitIndicesAccessControl(limitedByIndicesAccessControl);
}

Expand Down
14 changes: 1 addition & 13 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,7 @@ public ResourcePrivilegesMap checkApplicationResourcePrivileges(final String app
public IndicesAccessControl authorize(String action, Set<String> requestedIndicesOrAliases,
Map<String, IndexAbstraction> aliasAndIndexLookup,
FieldPermissionsCache fieldPermissionsCache) {
Map<String, IndicesAccessControl.IndexAccessControl> indexPermissions = indices.authorize(
action, requestedIndicesOrAliases, aliasAndIndexLookup, fieldPermissionsCache
);

// At least one role / indices permission set need to match with all the requested indices/aliases:
boolean granted = true;
for (Map.Entry<String, IndicesAccessControl.IndexAccessControl> entry : indexPermissions.entrySet()) {
if (entry.getValue().isGranted() == false) {
granted = false;
break;
}
}
return new IndicesAccessControl(granted, indexPermissions);
return indices.authorize(action, requestedIndicesOrAliases, aliasAndIndexLookup, fieldPermissionsCache);
}

@Override
Expand Down
4 changes: 4 additions & 0 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/StringMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ public boolean test(String s) {
return predicate.test(s);
}

public boolean isTotal() {
return predicate == ALWAYS_TRUE_PREDICATE;
}

// For testing
Predicate<String> getPredicate() {
return predicate;
Expand Down
41 changes: 20 additions & 21 deletions .../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.core.textstructure.action.FindStructureAction;
import org.elasticsearch.xpack.core.ml.action.FlushJobAction;
import org.elasticsearch.xpack.core.ml.action.ForecastJobAction;
Expand Down Expand Up @@ -167,7 +168,6 @@
import org.elasticsearch.xpack.core.security.action.user.PutUserAction;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl.IndexAccessControl;
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
import org.elasticsearch.xpack.core.security.authz.permission.Role;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
Expand Down Expand Up @@ -214,7 +214,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;

import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.RESTRICTED_INDICES_AUTOMATON;
Expand Down Expand Up @@ -1135,12 +1134,12 @@ private void assertMonitoringOnRestrictedIndices(Role role) {
GetSettingsAction.NAME, IndicesShardStoresAction.NAME, RecoveryAction.NAME);
for (final String indexMonitoringActionName : indexMonitoringActionNamesList) {
String asyncSearchIndex = XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2);
final Map<String, IndexAccessControl> authzMap = role.indices().authorize(indexMonitoringActionName,
final IndicesAccessControl iac = role.indices().authorize(indexMonitoringActionName,
Sets.newHashSet(internalSecurityIndex, RestrictedIndicesNames.SECURITY_MAIN_ALIAS, asyncSearchIndex),
metadata.getIndicesLookup(), fieldPermissionsCache);
assertThat(authzMap.get(internalSecurityIndex).isGranted(), is(true));
assertThat(authzMap.get(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat(authzMap.get(asyncSearchIndex).isGranted(), is(true));
assertThat(iac.getIndexPermissions(internalSecurityIndex).isGranted(), is(true));
assertThat(iac.getIndexPermissions(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat(iac.getIndexPermissions(asyncSearchIndex).isGranted(), is(true));
}
}

Expand Down Expand Up @@ -1233,25 +1232,25 @@ public void testSuperuserRole() {

FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
SortedMap<String, IndexAbstraction> lookup = metadata.getIndicesLookup();
Map<String, IndexAccessControl> authzMap =
IndicesAccessControl iac =
superuserRole.indices().authorize(SearchAction.NAME, Sets.newHashSet("a1", "ba"), lookup, fieldPermissionsCache);
assertThat(authzMap.get("a1").isGranted(), is(true));
assertThat(authzMap.get("b").isGranted(), is(true));
authzMap =
assertThat(iac.getIndexPermissions("a1").isGranted(), is(true));
assertThat(iac.getIndexPermissions("b").isGranted(), is(true));
iac =
superuserRole.indices().authorize(DeleteIndexAction.NAME, Sets.newHashSet("a1", "ba"), lookup, fieldPermissionsCache);
assertThat(authzMap.get("a1").isGranted(), is(true));
assertThat(authzMap.get("b").isGranted(), is(true));
authzMap = superuserRole.indices().authorize(IndexAction.NAME, Sets.newHashSet("a2", "ba"), lookup, fieldPermissionsCache);
assertThat(authzMap.get("a2").isGranted(), is(true));
assertThat(authzMap.get("b").isGranted(), is(true));
authzMap = superuserRole.indices()
assertThat(iac.getIndexPermissions("a1").isGranted(), is(true));
assertThat(iac.getIndexPermissions("b").isGranted(), is(true));
iac = superuserRole.indices().authorize(IndexAction.NAME, Sets.newHashSet("a2", "ba"), lookup, fieldPermissionsCache);
assertThat(iac.getIndexPermissions("a2").isGranted(), is(true));
assertThat(iac.getIndexPermissions("b").isGranted(), is(true));
iac = superuserRole.indices()
.authorize(UpdateSettingsAction.NAME, Sets.newHashSet("aaaaaa", "ba"), lookup, fieldPermissionsCache);
assertThat(authzMap.get("aaaaaa").isGranted(), is(true));
assertThat(authzMap.get("b").isGranted(), is(true));
authzMap = superuserRole.indices().authorize(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME, SearchAction.NAME),
assertThat(iac.getIndexPermissions("aaaaaa").isGranted(), is(true));
assertThat(iac.getIndexPermissions("b").isGranted(), is(true));
iac = superuserRole.indices().authorize(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME, SearchAction.NAME),
Sets.newHashSet(RestrictedIndicesNames.SECURITY_MAIN_ALIAS), lookup, fieldPermissionsCache);
assertThat(authzMap.get(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat(authzMap.get(internalSecurityIndex).isGranted(), is(true));
assertThat(iac.getIndexPermissions(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat(iac.getIndexPermissions(internalSecurityIndex).isGranted(), is(true));
assertTrue(superuserRole.indices().check(SearchAction.NAME));
assertFalse(superuserRole.indices().check("unknown"));

Expand Down
4 changes: 2 additions & 2 deletions ...n/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ private void authorizeAction(final RequestInfo requestInfo, final String request
if (ClusterPrivilegeResolver.isClusterAction(action)) {
final ActionListener<AuthorizationResult> clusterAuthzListener =
wrapPreservingContext(new AuthorizationResultListener<>(result -> {
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.allowAll());
listener.onResponse(null);
}, listener::onFailure, requestInfo, requestId, authzInfo), threadContext);
authzEngine.authorizeClusterAction(requestInfo, authzInfo, ActionListener.wrap(result -> {
Expand Down Expand Up @@ -514,7 +514,7 @@ private void authorizeSystemUser(final Authentication authentication, final Stri
final TransportRequest request, final ActionListener<Void> listener) {
final AuditTrail auditTrail = auditTrailService.get();
if (SystemUser.isAuthorized(action)) {
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.allowAll());
threadContext.putTransient(AUTHORIZATION_INFO_KEY, SYSTEM_AUTHZ_INFO);
auditTrail.accessGranted(requestId, authentication, action, request, SYSTEM_AUTHZ_INFO);
listener.onResponse(null);
Expand Down
Loading

0 comments on commit 5ae1518

Please sign in to comment.