Skip to content

Commit

Permalink
Deprecate permission over aliases (elastic#38059)
Browse files Browse the repository at this point in the history
This PR generates deprecation log entries for each Role Descriptor,
used for building a Role, when the Role Descriptor grants more privileges
for an alias compared to an index that the alias points to. This is done in
preparation for the removal of the ability to define privileges over aliases.
There is one log entry for each "role descriptor name"-"alias name" pair.
On such a notice, the administrator is expected to modify the Role Descriptor
definition so that the name pattern for index names does not cover aliases.

Caveats:
* Role Descriptors that are not used in any authorization process,
either because they are not mapped to any user or the user they are mapped to
is not used by clients, are not be checked.
* Role Descriptors are merged when building the effective Role that is used in
the authorization process. Therefore some Role Descriptors can overlap others,
so even if one matches aliases in a deprecated way, and it is reported as such,
it is not at risk from the breaking behavior in the current role mapping configuration
and index-alias configuration. It is still reported because it is a best practice to
change its definition, or remove offending aliases.
  • Loading branch information
albertzaharovits authored Apr 10, 2019
1 parent f83f72c commit 264ce54
Show file tree
Hide file tree
Showing 6 changed files with 655 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public IndicesPermission(Group... groups) {
this.groups = groups;
}

static Predicate<String> indexMatcher(Collection<String> indices) {
public static Predicate<String> indexMatcher(Collection<String> indices) {
Set<String> exactMatch = new HashSet<>();
List<String> nonExactMatch = new ArrayList<>();
for (String indexPattern : indices) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@
import org.elasticsearch.xpack.security.authz.interceptor.SearchRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.UpdateRequestInterceptor;
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
import org.elasticsearch.xpack.security.authz.store.DeprecationRoleDescriptorConsumer;
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
Expand Down Expand Up @@ -452,8 +453,10 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
threadPool);
components.add(apiKeyService);
final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore, reservedRolesStore,
privilegeStore, rolesProviders, threadPool.getThreadContext(), getLicenseState(), fieldPermissionsCache, apiKeyService);
privilegeStore, rolesProviders, threadPool.getThreadContext(), getLicenseState(), fieldPermissionsCache, apiKeyService,
new DeprecationRoleDescriptorConsumer(clusterService, threadPool));
securityIndex.get().addIndexStateListener(allRolesStore::onSecurityIndexStateChange);

// to keep things simple, just invalidate all cached entries on license change. this happens so rarely that the impact should be
// minimal
getLicenseState().addListener(allRolesStore::invalidateAll);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -100,6 +101,7 @@ public class CompositeRolesStore {
private final NativeRolesStore nativeRolesStore;
private final NativePrivilegeStore privilegeStore;
private final XPackLicenseState licenseState;
private final Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer;
private final FieldPermissionsCache fieldPermissionsCache;
private final Cache<RoleKey, Role> roleCache;
private final Cache<String, Boolean> negativeLookupCache;
Expand All @@ -115,14 +117,15 @@ public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, Nat
ReservedRolesStore reservedRolesStore, NativePrivilegeStore privilegeStore,
List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> rolesProviders,
ThreadContext threadContext, XPackLicenseState licenseState, FieldPermissionsCache fieldPermissionsCache,
ApiKeyService apiKeyService) {
ApiKeyService apiKeyService, Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer) {
this.fileRolesStore = fileRolesStore;
fileRolesStore.addListener(this::invalidate);
this.nativeRolesStore = nativeRolesStore;
this.privilegeStore = privilegeStore;
this.licenseState = licenseState;
this.fieldPermissionsCache = fieldPermissionsCache;
this.apiKeyService = apiKeyService;
this.effectiveRoleDescriptorsConsumer = effectiveRoleDescriptorsConsumer;
CacheBuilder<RoleKey, Role> builder = CacheBuilder.builder();
final int cacheSize = CACHE_SIZE_SETTING.get(settings);
if (cacheSize >= 0) {
Expand Down Expand Up @@ -161,9 +164,9 @@ public void roles(Set<String> roleNames, ActionListener<Role> roleActionListener
rolesRetrievalResult -> {
final boolean missingRoles = rolesRetrievalResult.getMissingRoles().isEmpty() == false;
if (missingRoles) {
logger.debug("Could not find roles with names {}", rolesRetrievalResult.getMissingRoles());
logger.debug(() -> new ParameterizedMessage("Could not find roles with names {}",
rolesRetrievalResult.getMissingRoles()));
}

final Set<RoleDescriptor> effectiveDescriptors;
if (licenseState.isDocumentAndFieldLevelSecurityAllowed()) {
effectiveDescriptors = rolesRetrievalResult.getRoleDescriptors();
Expand All @@ -172,6 +175,11 @@ public void roles(Set<String> roleNames, ActionListener<Role> roleActionListener
.filter((rd) -> rd.isUsingDocumentOrFieldLevelSecurity() == false)
.collect(Collectors.toSet());
}
logger.trace(() -> new ParameterizedMessage("Exposing effective role descriptors [{}] for role names [{}]",
effectiveDescriptors, roleNames));
effectiveRoleDescriptorsConsumer.accept(Collections.unmodifiableCollection(effectiveDescriptors));
logger.trace(() -> new ParameterizedMessage("Building role from descriptors [{}] for role names [{}]",
effectiveDescriptors, roleNames));
buildThenMaybeCacheRole(roleKey, effectiveDescriptors, rolesRetrievalResult.getMissingRoles(),
rolesRetrievalResult.isSuccess(), invalidationCounter, roleActionListener);
},
Expand Down Expand Up @@ -287,7 +295,7 @@ public void getRoleDescriptors(Set<String> roleNames, ActionListener<Set<RoleDes
private void roleDescriptors(Set<String> roleNames, ActionListener<RolesRetrievalResult> rolesResultListener) {
final Set<String> filteredRoleNames = roleNames.stream().filter((s) -> {
if (negativeLookupCache.get(s) != null) {
logger.debug("Requested role [{}] does not exist (cached)", s);
logger.debug(() -> new ParameterizedMessage("Requested role [{}] does not exist (cached)", s));
return false;
} else {
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

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

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.automaton.Automaton;
import org.apache.lucene.util.automaton.Operations;
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission;
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.Locale;
import java.util.Map;
import java.util.Queue;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.RejectedExecutionException;
import java.util.function.Consumer;
import java.util.function.Predicate;

/**
* Inspects all aliases that have greater privileges than the indices that they point to and logs the role descriptor, granting privileges
* in this manner, as deprecated and requiring changes. This is done in preparation for the removal of the ability to define privileges over
* aliases. The log messages are generated asynchronously and do not generate deprecation response headers. One log entry is generated for
* each role descriptor and alias pair, and it contains all the indices for which privileges are a subset of those of the alias. In this
* case, the administrator has to adjust the index privileges definition of the respective role such that name patterns do not cover aliases
* (or rename aliases). If no logging is generated then the roles used for the current indices and aliases are not vulnerable to the
* subsequent breaking change. However, there could be role descriptors that are not used (not mapped to a user that is currently using the
* system) which are invisible to this check. Moreover, role descriptors can be dynamically added by role providers. In addition, role
* descriptors are merged when building the effective role, so a role-alias pair reported as deprecated might not actually have an impact if
* other role descriptors cover its indices. The check iterates over all indices and aliases for each role descriptor so it is quite
* expensive computationally. For this reason the check is done only once a day for each role. If the role definitions stay the same, the
* deprecations can change from one day to another only if aliases or indices are added.
*/
public final class DeprecationRoleDescriptorConsumer implements Consumer<Collection<RoleDescriptor>> {

private static final String ROLE_PERMISSION_DEPRECATION_STANZA = "Role [%s] contains index privileges covering the [%s] alias but"
+ " which do not cover some of the indices that it points to [%s]. Granting privileges over an alias and hence granting"
+ " privileges over all the indices that the alias points to is deprecated and will be removed in a future version of"
+ " Elasticsearch. Instead define permissions exclusively on index names or index name patterns.";

private static final Logger logger = LogManager.getLogger(DeprecationRoleDescriptorConsumer.class);

private final DeprecationLogger deprecationLogger;
private final ClusterService clusterService;
private final ThreadPool threadPool;
private final Object mutex;
private final Queue<RoleDescriptor> workQueue;
private boolean workerBusy;
private final Set<String> dailyRoleCache;

public DeprecationRoleDescriptorConsumer(ClusterService clusterService, ThreadPool threadPool) {
this(clusterService, threadPool, new DeprecationLogger(logger));
}

// package-private for testing
DeprecationRoleDescriptorConsumer(ClusterService clusterService, ThreadPool threadPool, DeprecationLogger deprecationLogger) {
this.deprecationLogger = deprecationLogger;
this.clusterService = clusterService;
this.threadPool = threadPool;
this.mutex = new Object();
this.workQueue = new LinkedList<>();
this.workerBusy = false;
// this String Set keeps "<date>-<role>" pairs so that we only log a role once a day.
this.dailyRoleCache = Collections.newSetFromMap(new LinkedHashMap<String, Boolean>() {
@Override
protected boolean removeEldestEntry(final Map.Entry<String, Boolean> eldest) {
return false == eldest.getKey().startsWith(todayISODate());
}
});
}

@Override
public void accept(Collection<RoleDescriptor> effectiveRoleDescriptors) {
synchronized (mutex) {
for (RoleDescriptor roleDescriptor : effectiveRoleDescriptors) {
if (dailyRoleCache.add(buildCacheKey(roleDescriptor))) {
workQueue.add(roleDescriptor);
}
}
if (false == workerBusy) {
workerBusy = true;
try {
// spawn another worker on the generic thread pool
threadPool.generic().execute(new AbstractRunnable() {

@Override
public void onFailure(Exception e) {
logger.warn("Failed to produce role deprecation messages", e);
synchronized (mutex) {
final boolean hasMoreWork = workQueue.peek() != null;
if (hasMoreWork) {
workerBusy = true; // just being paranoid :)
try {
threadPool.generic().execute(this);
} catch (RejectedExecutionException e1) {
workerBusy = false;
logger.warn("Failed to start working on role alias permisssion deprecation messages", e1);
}
} else {
workerBusy = false;
}
}
}

@Override
protected void doRun() throws Exception {
while (true) {
final RoleDescriptor workItem;
synchronized (mutex) {
workItem = workQueue.poll();
if (workItem == null) {
workerBusy = false;
break;
}
}
logger.trace("Begin role [" + workItem.getName() + "] check for alias permission deprecation");
// executing the check asynchronously will not conserve the generated deprecation response headers (which is
// what we want, because it's not the request that uses deprecated features, but rather the role definition.
// Furthermore, due to caching, we can't reliably associate response headers to every request).
logDeprecatedPermission(workItem);
logger.trace("Completed role [" + workItem.getName() + "] check for alias permission deprecation");
}
}
});
} catch (RejectedExecutionException e) {
workerBusy = false;
logger.warn("Failed to start working on role alias permisssion deprecation messages", e);
}
}
}
}

private void logDeprecatedPermission(RoleDescriptor roleDescriptor) {
final SortedMap<String, AliasOrIndex> aliasOrIndexMap = clusterService.state().metaData().getAliasAndIndexLookup();
final Map<String, Set<String>> privilegesByAliasMap = new HashMap<>();
// sort answer by alias for tests
final SortedMap<String, Set<String>> privilegesByIndexMap = new TreeMap<>();
// collate privileges by index and by alias separately
for (final IndicesPrivileges indexPrivilege : roleDescriptor.getIndicesPrivileges()) {
final Predicate<String> namePatternPredicate = IndicesPermission.indexMatcher(Arrays.asList(indexPrivilege.getIndices()));
for (final Map.Entry<String, AliasOrIndex> aliasOrIndex : aliasOrIndexMap.entrySet()) {
final String aliasOrIndexName = aliasOrIndex.getKey();
if (namePatternPredicate.test(aliasOrIndexName)) {
if (aliasOrIndex.getValue().isAlias()) {
final Set<String> privilegesByAlias = privilegesByAliasMap.computeIfAbsent(aliasOrIndexName,
k -> new HashSet<String>());
privilegesByAlias.addAll(Arrays.asList(indexPrivilege.getPrivileges()));
} else {
final Set<String> privilegesByIndex = privilegesByIndexMap.computeIfAbsent(aliasOrIndexName,
k -> new HashSet<String>());
privilegesByIndex.addAll(Arrays.asList(indexPrivilege.getPrivileges()));
}
}
}
}
// compute privileges Automaton for each alias and for each of the indices it points to
final Map<String, Automaton> indexAutomatonMap = new HashMap<>();
for (Map.Entry<String, Set<String>> privilegesByAlias : privilegesByAliasMap.entrySet()) {
final String aliasName = privilegesByAlias.getKey();
final Set<String> aliasPrivilegeNames = privilegesByAlias.getValue();
final Automaton aliasPrivilegeAutomaton = IndexPrivilege.get(aliasPrivilegeNames).getAutomaton();
final SortedSet<String> inferiorIndexNames = new TreeSet<>();
// check if the alias grants superiors privileges than the indices it points to
for (IndexMetaData indexMetadata : aliasOrIndexMap.get(aliasName).getIndices()) {
final String indexName = indexMetadata.getIndex().getName();
final Set<String> indexPrivileges = privilegesByIndexMap.get(indexName);
// null iff the index does not have *any* privilege
if (indexPrivileges != null) {
// compute automaton once per index no matter how many times it is pointed to
final Automaton indexPrivilegeAutomaton = indexAutomatonMap.computeIfAbsent(indexName,
i -> IndexPrivilege.get(indexPrivileges).getAutomaton());
if (false == Operations.subsetOf(indexPrivilegeAutomaton, aliasPrivilegeAutomaton)) {
inferiorIndexNames.add(indexName);
}
} else {
inferiorIndexNames.add(indexName);
}
}
// log inferior indices for this role, for this alias
if (false == inferiorIndexNames.isEmpty()) {
final String logMessage = String.format(Locale.ROOT, ROLE_PERMISSION_DEPRECATION_STANZA, roleDescriptor.getName(),
aliasName, String.join(", ", inferiorIndexNames));
deprecationLogger.deprecated(logMessage);
}
}
}

private static String todayISODate() {
return ZonedDateTime.now(ZoneOffset.UTC).format(DateTimeFormatter.BASIC_ISO_DATE);
}

// package-private for testing
static String buildCacheKey(RoleDescriptor roleDescriptor) {
return todayISODate() + "-" + roleDescriptor.getName();
}
}
Loading

0 comments on commit 264ce54

Please sign in to comment.