This is an automated email from the ASF dual-hosted git repository. samt pushed a commit to branch cassandra-3.11 in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit 8f33dc0f138731f2bbe9febaf209c584f783a497 Merge: c08ce93 63097a3 Author: Sam Tunnicliffe <[email protected]> AuthorDate: Fri Jul 5 19:02:28 2019 +0100 Merge branch 'cassandra-3.0' into cassandra-3.11 CHANGES.txt | 1 + src/java/org/apache/cassandra/auth/AuthCache.java | 22 ++- .../apache/cassandra/auth/CassandraAuthorizer.java | 23 ++- .../cassandra/auth/CassandraRoleManager.java | 45 +++--- .../cassandra/auth/PasswordAuthenticator.java | 45 ++---- .../apache/cassandra/auth/PermissionsCache.java | 10 +- src/java/org/apache/cassandra/auth/Roles.java | 25 +++- src/java/org/apache/cassandra/auth/RolesCache.java | 10 +- .../cassandra/auth/jmx/AuthorizationProxy.java | 15 -- .../exceptions/AuthenticationException.java | 5 + .../exceptions/UnauthorizedException.java | 5 + .../org/apache/cassandra/service/ClientState.java | 13 +- .../org/apache/cassandra/auth/AuthCacheTest.java | 164 +++++++++++++++++++++ 13 files changed, 274 insertions(+), 109 deletions(-) diff --cc CHANGES.txt index 0d0b759,364720d..ab5cb66 --- a/CHANGES.txt +++ b/CHANGES.txt @@@ -17,9 -14,10 +17,10 @@@ Merged from 3.0 * Avoid double closing the iterator to avoid overcounting the number of requests (CASSANDRA-15058) * Improve `nodetool status -r` speed (CASSANDRA-14847) * Improve merkle tree size and time on heap (CASSANDRA-14096) - * Add missing commands to nodetool-completion (CASSANDRA-14916) + * Add missing commands to nodetool_completion (CASSANDRA-14916) * Anti-compaction temporarily corrupts sstable state for readers (CASSANDRA-15004) - Merged from 2.2: +Merged from 2.2: + * Handle exceptions during authentication/authorization (CASSANDRA-15041) * Support cross version messaging in in-jvm upgrade dtests (CASSANDRA-15078) * Fix index summary redistribution cancellation (CASSANDRA-15045) * Refactor Circle CI configuration (CASSANDRA-14806) diff --cc src/java/org/apache/cassandra/auth/AuthCache.java index 02b3c0c,0000000..80664d1 mode 100644,000000..100644 --- a/src/java/org/apache/cassandra/auth/AuthCache.java +++ b/src/java/org/apache/cassandra/auth/AuthCache.java @@@ -1,196 -1,0 +1,212 @@@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.auth; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; + ++import com.google.common.base.Throwables; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListenableFutureTask; ++import com.google.common.util.concurrent.UncheckedExecutionException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; ++ +import org.apache.cassandra.utils.MBeanWrapper; + +import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor; + +public class AuthCache<K, V> implements AuthCacheMBean +{ + private static final Logger logger = LoggerFactory.getLogger(AuthCache.class); + + private static final String MBEAN_NAME_BASE = "org.apache.cassandra.auth:type="; + + private volatile LoadingCache<K, V> cache; + private ThreadPoolExecutor cacheRefreshExecutor; + + private final String name; + private final Consumer<Integer> setValidityDelegate; + private final Supplier<Integer> getValidityDelegate; + private final Consumer<Integer> setUpdateIntervalDelegate; + private final Supplier<Integer> getUpdateIntervalDelegate; + private final Consumer<Integer> setMaxEntriesDelegate; + private final Supplier<Integer> getMaxEntriesDelegate; + private final Function<K, V> loadFunction; + private final Supplier<Boolean> enableCache; + + protected AuthCache(String name, + Consumer<Integer> setValidityDelegate, + Supplier<Integer> getValidityDelegate, + Consumer<Integer> setUpdateIntervalDelegate, + Supplier<Integer> getUpdateIntervalDelegate, + Consumer<Integer> setMaxEntriesDelegate, + Supplier<Integer> getMaxEntriesDelegate, + Function<K, V> loadFunction, + Supplier<Boolean> enableCache) + { + this.name = name; + this.setValidityDelegate = setValidityDelegate; + this.getValidityDelegate = getValidityDelegate; + this.setUpdateIntervalDelegate = setUpdateIntervalDelegate; + this.getUpdateIntervalDelegate = getUpdateIntervalDelegate; + this.setMaxEntriesDelegate = setMaxEntriesDelegate; + this.getMaxEntriesDelegate = getMaxEntriesDelegate; + this.loadFunction = loadFunction; + this.enableCache = enableCache; + init(); + } + + protected void init() + { - this.cacheRefreshExecutor = new DebuggableThreadPoolExecutor(name + "Refresh", Thread.NORM_PRIORITY); ++ this.cacheRefreshExecutor = new DebuggableThreadPoolExecutor(name + "Refresh", Thread.NORM_PRIORITY) ++ { ++ protected void afterExecute(Runnable r, Throwable t) ++ { ++ // empty to avoid logging on background updates ++ } ++ }; + this.cache = initCache(null); + MBeanWrapper.instance.registerMBean(this, getObjectName()); + } + + protected String getObjectName() + { + return MBEAN_NAME_BASE + name; + } + - public V get(K k) throws ExecutionException ++ public V get(K k) + { + if (cache == null) + return loadFunction.apply(k); + - return cache.get(k); ++ try { ++ return cache.get(k); ++ } ++ catch (ExecutionException | UncheckedExecutionException e) ++ { ++ Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class); ++ throw Throwables.propagate(e); ++ } + } + + public void invalidate() + { + cache = initCache(null); + } + + public void invalidate(K k) + { + if (cache != null) + cache.invalidate(k); + } + + public void setValidity(int validityPeriod) + { + if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) + throw new UnsupportedOperationException("Remote configuration of auth caches is disabled"); + + setValidityDelegate.accept(validityPeriod); + cache = initCache(cache); + } + + public int getValidity() + { + return getValidityDelegate.get(); + } + + public void setUpdateInterval(int updateInterval) + { + if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) + throw new UnsupportedOperationException("Remote configuration of auth caches is disabled"); + + setUpdateIntervalDelegate.accept(updateInterval); + cache = initCache(cache); + } + + public int getUpdateInterval() + { + return getUpdateIntervalDelegate.get(); + } + + public void setMaxEntries(int maxEntries) + { + if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) + throw new UnsupportedOperationException("Remote configuration of auth caches is disabled"); + + setMaxEntriesDelegate.accept(maxEntries); + cache = initCache(cache); + } + + public int getMaxEntries() + { + return getMaxEntriesDelegate.get(); + } + + private LoadingCache<K, V> initCache(LoadingCache<K, V> existing) + { + if (!enableCache.get()) + return null; + + if (getValidity() <= 0) + return null; + + logger.info("(Re)initializing {} (validity period/update interval/max entries) ({}/{}/{})", + name, getValidity(), getUpdateInterval(), getMaxEntries()); + + LoadingCache<K, V> newcache = CacheBuilder.newBuilder() + .refreshAfterWrite(getUpdateInterval(), TimeUnit.MILLISECONDS) + .expireAfterWrite(getValidity(), TimeUnit.MILLISECONDS) + .maximumSize(getMaxEntries()) + .build(new CacheLoader<K, V>() + { + public V load(K k) + { + return loadFunction.apply(k); + } + + public ListenableFuture<V> reload(final K k, final V oldV) + { + ListenableFutureTask<V> task = ListenableFutureTask.create(() -> { + try + { + return loadFunction.apply(k); + } + catch (Exception e) + { + logger.trace("Error performing async refresh of auth data in {}", name, e); + throw e; + } + }); + cacheRefreshExecutor.execute(task); + return task; + } + }); + if (existing != null) + newcache.putAll(existing.asMap()); + return newcache; + } +} diff --cc src/java/org/apache/cassandra/auth/CassandraRoleManager.java index 932f1c6,aa98b7e..38862d9 --- a/src/java/org/apache/cassandra/auth/CassandraRoleManager.java +++ b/src/java/org/apache/cassandra/auth/CassandraRoleManager.java @@@ -499,23 -514,16 +515,16 @@@ public class CassandraRoleManager imple */ private Role getRole(String name) { - try - { - // If it exists, try the legacy users table in case the cluster - // is in the process of being upgraded and so is running with mixed - // versions of the authn schema. - if (Schema.instance.getCFMetaData(SchemaConstants.AUTH_KEYSPACE_NAME, "users") == null) - return getRoleFromTable(name, loadRoleStatement, ROW_TO_ROLE); - else - { - if (legacySelectUserStatement == null) - legacySelectUserStatement = prepareLegacySelectUserStatement(); - return getRoleFromTable(name, legacySelectUserStatement, LEGACY_ROW_TO_ROLE); - } - } - catch (RequestExecutionException | RequestValidationException e) + // If it exists, try the legacy users table in case the cluster + // is in the process of being upgraded and so is running with mixed + // versions of the authn schema. - if (Schema.instance.getCFMetaData(AuthKeyspace.NAME, "users") == null) ++ if (Schema.instance.getCFMetaData(SchemaConstants.AUTH_KEYSPACE_NAME, "users") == null) + return getRoleFromTable(name, loadRoleStatement, ROW_TO_ROLE); + else { - throw new RuntimeException(e); + if (legacySelectUserStatement == null) + legacySelectUserStatement = prepareLegacySelectUserStatement(); + return getRoleFromTable(name, legacySelectUserStatement, LEGACY_ROW_TO_ROLE); } } diff --cc src/java/org/apache/cassandra/auth/PasswordAuthenticator.java index 54f7985,2b65783..b1a7227 --- a/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java @@@ -98,59 -90,15 +96,40 @@@ public class PasswordAuthenticator impl private AuthenticatedUser authenticate(String username, String password) throws AuthenticationException { - try - { - String hash = cache.get(username); - if (!checkpw(password, hash)) - throw new AuthenticationException(String.format("Provided username %s and/or password are incorrect", username)); - - return new AuthenticatedUser(username); - } - catch (ExecutionException | UncheckedExecutionException e) - { - // the credentials were somehow invalid - either a non-existent role, or one without a defined password - if (e.getCause() instanceof NoSuchCredentialsException) - throw new AuthenticationException(String.format("Provided username %s and/or password are incorrect", username)); - - // an unanticipated exception occured whilst querying the credentials table - if (e.getCause() instanceof RequestExecutionException) - { - logger.trace("Error performing internal authentication", e); - throw new AuthenticationException(String.format("Error during authentication of user %s : %s", username, e.getMessage())); - } ++ String hash = cache.get(username); ++ if (!checkpw(password, hash)) ++ throw new AuthenticationException(String.format("Provided username %s and/or password are incorrect", username)); + - throw new RuntimeException(e); - } ++ return new AuthenticatedUser(username); + } + - private String queryHashedPassword(String username) throws NoSuchCredentialsException ++ private String queryHashedPassword(String username) throws AuthenticationException + { try { SelectStatement authenticationStatement = authenticationStatement(); - return doAuthenticate(username, password, authenticationStatement); + + ResultMessage.Rows rows = + authenticationStatement.execute(QueryState.forInternalCalls(), + QueryOptions.forInternalCalls(consistencyForRole(username), + Lists.newArrayList(ByteBufferUtil.bytes(username))), + System.nanoTime()); + + // If either a non-existent role name was supplied, or no credentials + // were found for that role we don't want to cache the result so we throw - // a specific, but unchecked, exception to keep LoadingCache happy. ++ // an exception. + if (rows.result.isEmpty()) - throw new NoSuchCredentialsException(); ++ throw new AuthenticationException(String.format("Provided username %s and/or password are incorrect", username)); + + UntypedResultSet result = UntypedResultSet.create(rows.result); + if (!result.one().has(SALTED_HASH)) - throw new NoSuchCredentialsException(); ++ throw new AuthenticationException(String.format("Provided username %s and/or password are incorrect", username)); + + return result.one().getString(SALTED_HASH); } catch (RequestExecutionException e) { -- logger.trace("Error performing internal authentication", e); - throw e; + throw new AuthenticationException("Unable to perform authentication: " + e.getMessage(), e); } } @@@ -293,36 -252,4 +272,30 @@@ password = new String(pass, StandardCharsets.UTF_8); } } + + private static class CredentialsCache extends AuthCache<String, String> implements CredentialsCacheMBean + { + private CredentialsCache(PasswordAuthenticator authenticator) + { + super("CredentialsCache", + DatabaseDescriptor::setCredentialsValidity, + DatabaseDescriptor::getCredentialsValidity, + DatabaseDescriptor::setCredentialsUpdateInterval, + DatabaseDescriptor::getCredentialsUpdateInterval, + DatabaseDescriptor::setCredentialsCacheMaxEntries, + DatabaseDescriptor::getCredentialsCacheMaxEntries, + authenticator::queryHashedPassword, + () -> true); + } + + public void invalidateCredentials(String roleName) + { + invalidate(roleName); + } + } + + public static interface CredentialsCacheMBean extends AuthCacheMBean + { + public void invalidateCredentials(String roleName); + } - - // Just a marker so we can identify that invalid credentials were the - // cause of a loading exception from the cache - private static final class NoSuchCredentialsException extends RuntimeException - { - } } diff --cc src/java/org/apache/cassandra/auth/PermissionsCache.java index 875c473,ddd6348..981ede8 --- a/src/java/org/apache/cassandra/auth/PermissionsCache.java +++ b/src/java/org/apache/cassandra/auth/PermissionsCache.java @@@ -18,35 -18,135 +18,27 @@@ package org.apache.cassandra.auth; import java.util.Set; - import java.util.concurrent.ExecutionException; -import java.util.concurrent.*; import org.apache.cassandra.config.DatabaseDescriptor; - -import com.google.common.base.Throwables; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.ListenableFutureTask; -import com.google.common.util.concurrent.UncheckedExecutionException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor; -import org.apache.cassandra.utils.MBeanWrapper; import org.apache.cassandra.utils.Pair; -public class PermissionsCache implements PermissionsCacheMBean +public class PermissionsCache extends AuthCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> implements PermissionsCacheMBean { - private static final Logger logger = LoggerFactory.getLogger(PermissionsCache.class); - - private final String MBEAN_NAME = "org.apache.cassandra.auth:type=PermissionsCache"; - - private final ThreadPoolExecutor cacheRefreshExecutor = new DebuggableThreadPoolExecutor("PermissionsCacheRefresh", - Thread.NORM_PRIORITY) - { - protected void afterExecute(Runnable r, Throwable t) - { - // empty to avoid logging on background updates - } - }; - private final IAuthorizer authorizer; - private volatile LoadingCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> cache; - public PermissionsCache(IAuthorizer authorizer) { - this.authorizer = authorizer; - this.cache = initCache(null); - MBeanWrapper.instance.registerMBean(this, MBEAN_NAME); + super("PermissionsCache", + DatabaseDescriptor::setPermissionsValidity, + DatabaseDescriptor::getPermissionsValidity, + DatabaseDescriptor::setPermissionsUpdateInterval, + DatabaseDescriptor::getPermissionsUpdateInterval, + DatabaseDescriptor::setPermissionsCacheMaxEntries, + DatabaseDescriptor::getPermissionsCacheMaxEntries, + (p) -> authorizer.authorize(p.left, p.right), + () -> DatabaseDescriptor.getAuthorizer().requireAuthorization()); } public Set<Permission> getPermissions(AuthenticatedUser user, IResource resource) { - if (cache == null) - return authorizer.authorize(user, resource); - -- try -- { - return get(Pair.create(user, resource)); - return cache.get(Pair.create(user, resource)); -- } - catch (ExecutionException e) - catch (ExecutionException | UncheckedExecutionException e) -- { - throw new RuntimeException(e); - Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class); - throw Throwables.propagate(e); -- } - } - - public void invalidate() - { - cache = initCache(null); - } - - public void setValidity(int validityPeriod) - { - DatabaseDescriptor.setPermissionsValidity(validityPeriod); - cache = initCache(cache); - } - - public int getValidity() - { - return DatabaseDescriptor.getPermissionsValidity(); - } - - public void setUpdateInterval(int updateInterval) - { - DatabaseDescriptor.setPermissionsUpdateInterval(updateInterval); - cache = initCache(cache); - } - - public int getUpdateInterval() - { - return DatabaseDescriptor.getPermissionsUpdateInterval(); - } - - private LoadingCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> initCache( - LoadingCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> existing) - { - if (authorizer instanceof AllowAllAuthorizer) - return null; - - if (DatabaseDescriptor.getPermissionsValidity() <= 0) - return null; - - LoadingCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> newcache = CacheBuilder.newBuilder() - .refreshAfterWrite(DatabaseDescriptor.getPermissionsUpdateInterval(), TimeUnit.MILLISECONDS) - .expireAfterWrite(DatabaseDescriptor.getPermissionsValidity(), TimeUnit.MILLISECONDS) - .maximumSize(DatabaseDescriptor.getPermissionsCacheMaxEntries()) - .build(new CacheLoader<Pair<AuthenticatedUser, IResource>, Set<Permission>>() - { - public Set<Permission> load(Pair<AuthenticatedUser, IResource> userResource) - { - return authorizer.authorize(userResource.left, userResource.right); - } - - public ListenableFuture<Set<Permission>> reload(final Pair<AuthenticatedUser, IResource> userResource, - final Set<Permission> oldValue) - { - ListenableFutureTask<Set<Permission>> task = ListenableFutureTask.create(new Callable<Set<Permission>>() - { - public Set<Permission>call() throws Exception - { - try - { - return authorizer.authorize(userResource.left, userResource.right); - } - catch (Exception e) - { - logger.trace("Error performing async refresh of user permissions", e); - throw e; - } - } - }); - cacheRefreshExecutor.execute(task); - return task; - } - }); - if (existing != null) - newcache.putAll(existing.asMap()); - return newcache; ++ return get(Pair.create(user, resource)); } } diff --cc src/java/org/apache/cassandra/auth/RolesCache.java index 8b9c322,a8dae21..a02828a --- a/src/java/org/apache/cassandra/auth/RolesCache.java +++ b/src/java/org/apache/cassandra/auth/RolesCache.java @@@ -18,34 -18,132 +18,26 @@@ package org.apache.cassandra.auth; import java.util.Set; - import java.util.concurrent.ExecutionException; -import java.util.concurrent.*; -import com.google.common.base.Throwables; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.ListenableFutureTask; -import com.google.common.util.concurrent.UncheckedExecutionException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor; import org.apache.cassandra.config.DatabaseDescriptor; -import org.apache.cassandra.utils.MBeanWrapper; -public class RolesCache implements RolesCacheMBean +public class RolesCache extends AuthCache<RoleResource, Set<RoleResource>> implements RolesCacheMBean { - private static final Logger logger = LoggerFactory.getLogger(RolesCache.class); - - private final String MBEAN_NAME = "org.apache.cassandra.auth:type=RolesCache"; - private final ThreadPoolExecutor cacheRefreshExecutor = new DebuggableThreadPoolExecutor("RolesCacheRefresh", - Thread.NORM_PRIORITY) - { - protected void afterExecute(Runnable r, Throwable t) - { - // empty to avoid logging on background updates - } - }; - private final IRoleManager roleManager; - private volatile LoadingCache<RoleResource, Set<RoleResource>> cache; - public RolesCache(IRoleManager roleManager) { - this.roleManager = roleManager; - this.cache = initCache(null); - MBeanWrapper.instance.registerMBean(this, MBEAN_NAME); + super("RolesCache", + DatabaseDescriptor::setRolesValidity, + DatabaseDescriptor::getRolesValidity, + DatabaseDescriptor::setRolesUpdateInterval, + DatabaseDescriptor::getRolesUpdateInterval, + DatabaseDescriptor::setRolesCacheMaxEntries, + DatabaseDescriptor::getRolesCacheMaxEntries, + (r) -> roleManager.getRoles(r, true), + () -> DatabaseDescriptor.getAuthenticator().requireAuthentication()); } public Set<RoleResource> getRoles(RoleResource role) { - if (cache == null) - return roleManager.getRoles(role, true); - -- try -- { - return get(role); - return cache.get(role); -- } - catch (ExecutionException e) - catch (ExecutionException | UncheckedExecutionException e) -- { - throw new RuntimeException(e); - Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class); - throw Throwables.propagate(e); -- } - } - - public void invalidate() - { - cache = initCache(null); - } - - public void setValidity(int validityPeriod) - { - DatabaseDescriptor.setRolesValidity(validityPeriod); - cache = initCache(cache); - } - - public int getValidity() - { - return DatabaseDescriptor.getRolesValidity(); - } - - public void setUpdateInterval(int updateInterval) - { - DatabaseDescriptor.setRolesUpdateInterval(updateInterval); - cache = initCache(cache); - } - - public int getUpdateInterval() - { - return DatabaseDescriptor.getRolesUpdateInterval(); - } - - - private LoadingCache<RoleResource, Set<RoleResource>> initCache(LoadingCache<RoleResource, Set<RoleResource>> existing) - { - if (!DatabaseDescriptor.getAuthenticator().requireAuthentication()) - return null; - - if (DatabaseDescriptor.getRolesValidity() <= 0) - return null; - - LoadingCache<RoleResource, Set<RoleResource>> newcache = CacheBuilder.newBuilder() - .refreshAfterWrite(DatabaseDescriptor.getRolesUpdateInterval(), TimeUnit.MILLISECONDS) - .expireAfterWrite(DatabaseDescriptor.getRolesValidity(), TimeUnit.MILLISECONDS) - .maximumSize(DatabaseDescriptor.getRolesCacheMaxEntries()) - .build(new CacheLoader<RoleResource, Set<RoleResource>>() - { - public Set<RoleResource> load(RoleResource primaryRole) - { - return roleManager.getRoles(primaryRole, true); - } - - public ListenableFuture<Set<RoleResource>> reload(final RoleResource primaryRole, - final Set<RoleResource> oldValue) - { - ListenableFutureTask<Set<RoleResource>> task; - task = ListenableFutureTask.create(new Callable<Set<RoleResource>>() - { - public Set<RoleResource> call() throws Exception - { - try - { - return roleManager.getRoles(primaryRole, true); - } catch (Exception e) - { - logger.trace("Error performing async refresh of user roles", e); - throw e; - } - } - }); - cacheRefreshExecutor.execute(task); - return task; - } - }); - if (existing != null) - newcache.putAll(existing.asMap()); - return newcache; ++ return get(role); } } diff --cc src/java/org/apache/cassandra/auth/jmx/AuthorizationProxy.java index f19b19f,0000000..7bfbf52 mode 100644,000000..100644 --- a/src/java/org/apache/cassandra/auth/jmx/AuthorizationProxy.java +++ b/src/java/org/apache/cassandra/auth/jmx/AuthorizationProxy.java @@@ -1,509 -1,0 +1,494 @@@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.auth.jmx; + +import java.lang.reflect.*; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.Principal; +import java.util.Set; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; +import javax.security.auth.Subject; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.auth.*; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.service.StorageService; + +/** + * Provides a proxy interface to the platform's MBeanServer instance to perform + * role-based authorization on method invocation. + * + * When used in conjunction with a suitable JMXAuthenticator, which attaches a CassandraPrincipal + * to authenticated Subjects, this class uses the configured IAuthorizer to verify that the + * subject has the required permissions to execute methods on the MBeanServer and the MBeans it + * manages. + * + * Because an ObjectName may contain wildcards, meaning it represents a set of individual MBeans, + * JMX resources don't fit well with the hierarchical approach modelled by other IResource + * implementations and utilised by ClientState::ensureHasPermission etc. To enable grants to use + * pattern-type ObjectNames, this class performs its own custom matching and filtering of resources + * rather than pushing that down to the configured IAuthorizer. To that end, during authorization + * it pulls back all permissions for the active subject, filtering them to retain only grants on + * JMXResources. It then uses ObjectName::apply to assert whether the target MBeans are wholly + * represented by the resources with permissions. This means that it cannot use the PermissionsCache + * as IAuthorizer can, so it manages its own cache locally. + * + * Methods are split into 2 categories; those which are to be invoked on the MBeanServer itself + * and those which apply to MBean instances. Actually, this is somewhat of a construct as in fact + * *all* invocations are performed on the MBeanServer instance, the distinction is made here on + * those methods which take an ObjectName as their first argument and those which do not. + * Invoking a method of the former type, e.g. MBeanServer::getAttribute(ObjectName name, String attribute), + * implies that the caller is concerned with a specific MBean. Conversely, invoking a method such as + * MBeanServer::getDomains is primarily a function of the MBeanServer itself. This class makes + * such a distinction in order to identify which JMXResource the subject requires permissions on. + * + * Certain operations are never allowed for users and these are recorded in a blacklist so that we + * can short circuit authorization process if one is attempted by a remote subject. + * + */ +public class AuthorizationProxy implements InvocationHandler +{ + private static final Logger logger = LoggerFactory.getLogger(AuthorizationProxy.class); + + /* + A whitelist of permitted methods on the MBeanServer interface which *do not* take an ObjectName + as their first argument. These methods can be thought of as relating to the MBeanServer itself, + rather than to the MBeans it manages. All of the whitelisted methods are essentially descriptive, + hence they require the Subject to have the DESCRIBE permission on the root JMX resource. + */ + private static final Set<String> MBEAN_SERVER_METHOD_WHITELIST = ImmutableSet.of("getDefaultDomain", + "getDomains", + "getMBeanCount", + "hashCode", + "queryMBeans", + "queryNames", + "toString"); + + /* + A blacklist of method names which are never permitted to be executed by a remote user, + regardless of privileges they may be granted. + */ + private static final Set<String> METHOD_BLACKLIST = ImmutableSet.of("createMBean", + "deserialize", + "getClassLoader", + "getClassLoaderFor", + "instantiate", + "registerMBean", + "unregisterMBean"); + + private static final JMXPermissionsCache permissionsCache = new JMXPermissionsCache(); + private MBeanServer mbs; + + /* + Used to check whether the Role associated with the authenticated Subject has superuser + status. By default, just delegates to Roles::hasSuperuserStatus, but can be overridden for testing. + */ + protected Function<RoleResource, Boolean> isSuperuser = Roles::hasSuperuserStatus; + + /* + Used to retrieve the set of all permissions granted to a given role. By default, this fetches + the permissions from the local cache, which in turn loads them from the configured IAuthorizer + but can be overridden for testing. + */ + protected Function<RoleResource, Set<PermissionDetails>> getPermissions = permissionsCache::get; + + /* + Used to decide whether authorization is enabled or not, usually this depends on the configured + IAuthorizer, but can be overridden for testing. + */ + protected Supplier<Boolean> isAuthzRequired = () -> DatabaseDescriptor.getAuthorizer().requireAuthorization(); + + /* + Used to find matching MBeans when the invocation target is a pattern type ObjectName. + Defaults to querying the MBeanServer but can be overridden for testing. See checkPattern for usage. + */ + protected Function<ObjectName, Set<ObjectName>> queryNames = (name) -> mbs.queryNames(name, null); + + /* + Used to determine whether auth setup has completed so we know whether the expect the IAuthorizer + to be ready. Can be overridden for testing. + */ + protected Supplier<Boolean> isAuthSetupComplete = () -> StorageService.instance.isAuthSetupComplete(); + + @Override + public Object invoke(Object proxy, Method method, Object[] args) + throws Throwable + { + String methodName = method.getName(); + + if ("getMBeanServer".equals(methodName)) + throw new SecurityException("Access denied"); + + // Retrieve Subject from current AccessControlContext + AccessControlContext acc = AccessController.getContext(); + Subject subject = Subject.getSubject(acc); + + // Allow setMBeanServer iff performed on behalf of the connector server itself + if (("setMBeanServer").equals(methodName)) + { + if (subject != null) + throw new SecurityException("Access denied"); + + if (args[0] == null) + throw new IllegalArgumentException("Null MBeanServer"); + + if (mbs != null) + throw new IllegalArgumentException("MBeanServer already initialized"); + + mbs = (MBeanServer) args[0]; + return null; + } + + if (authorize(subject, methodName, args)) + return invoke(method, args); + + throw new SecurityException("Access Denied"); + } + + /** + * Performs the actual authorization of an identified subject to execute a remote method invocation. + * @param subject The principal making the execution request. A null value represents a local invocation + * from the JMX connector itself + * @param methodName Name of the method being invoked + * @param args Array containing invocation argument. If the first element is an ObjectName instance, for + * authz purposes we consider this an invocation of an MBean method, otherwise it is treated + * as an invocation of a method on the MBeanServer. + */ + @VisibleForTesting + boolean authorize(Subject subject, String methodName, Object[] args) + { + logger.trace("Authorizing JMX method invocation {} for {}", + methodName, + subject == null ? "" :subject.toString().replaceAll("\\n", " ")); + + if (!isAuthSetupComplete.get()) + { + logger.trace("Auth setup is not complete, refusing access"); + return false; + } + + // Permissive authorization is enabled + if (!isAuthzRequired.get()) + return true; + + // Allow operations performed locally on behalf of the connector server itself + if (subject == null) + return true; + + // Restrict access to certain methods by any remote user + if (METHOD_BLACKLIST.contains(methodName)) + { + logger.trace("Access denied to blacklisted method {}", methodName); + return false; + } + + // Reject if the user has not authenticated + Set<Principal> principals = subject.getPrincipals(); + if (principals == null || principals.isEmpty()) + return false; + + // Currently, we assume that the first Principal returned from the Subject + // is the one to use for authorization. It would be good to make this more + // robust, but we have no control over which Principals a given LoginModule + // might choose to associate with the Subject following successful authentication + RoleResource userResource = RoleResource.role(principals.iterator().next().getName()); + // A role with superuser status can do anything + if (isSuperuser.apply(userResource)) + return true; + + // The method being invoked may be a method on an MBean, or it could belong + // to the MBeanServer itself + if (args != null && args[0] instanceof ObjectName) + return authorizeMBeanMethod(userResource, methodName, args); + else + return authorizeMBeanServerMethod(userResource, methodName); + } + + /** + * Authorize execution of a method on the MBeanServer which does not take an MBean ObjectName + * as its first argument. The whitelisted methods that match this criteria are generally + * descriptive methods concerned with the MBeanServer itself, rather than with any particular + * set of MBeans managed by the server and so we check the DESCRIBE permission on the root + * JMXResource (representing the MBeanServer) + * + * @param subject + * @param methodName + * @return the result of the method invocation, if authorized + * @throws Throwable + * @throws SecurityException if authorization fails + */ + private boolean authorizeMBeanServerMethod(RoleResource subject, String methodName) + { + logger.trace("JMX invocation of {} on MBeanServer requires permission {}", methodName, Permission.DESCRIBE); + return (MBEAN_SERVER_METHOD_WHITELIST.contains(methodName) && + hasPermission(subject, Permission.DESCRIBE, JMXResource.root())); + } + + /** + * Authorize execution of a method on an MBean (or set of MBeans) which may be + * managed by the MBeanServer. Note that this also includes the queryMBeans and queryNames + * methods of MBeanServer as those both take an ObjectName (possibly a pattern containing + * wildcards) as their first argument. They both of those methods also accept null arguments, + * in which case they will be handled by authorizedMBeanServerMethod + * + * @param role + * @param methodName + * @param args + * @return the result of the method invocation, if authorized + * @throws Throwable + * @throws SecurityException if authorization fails + */ + private boolean authorizeMBeanMethod(RoleResource role, String methodName, Object[] args) + { + ObjectName targetBean = (ObjectName)args[0]; + + // work out which permission we need to execute the method being called on the mbean + Permission requiredPermission = getRequiredPermission(methodName); + if (null == requiredPermission) + return false; + + logger.trace("JMX invocation of {} on {} requires permission {}", methodName, targetBean, requiredPermission); + + // find any JMXResources upon which the authenticated subject has been granted the + // reqired permission. We'll do ObjectName-specific filtering & matching of resources later + Set<JMXResource> permittedResources = getPermittedResources(role, requiredPermission); + + if (permittedResources.isEmpty()) + return false; + + // finally, check the JMXResource from the grants to see if we have either + // an exact match or a wildcard match for the target resource, whichever is + // applicable + return targetBean.isPattern() + ? checkPattern(targetBean, permittedResources) + : checkExact(targetBean, permittedResources); + } + + /** + * Get any grants of the required permission for the authenticated subject, regardless + * of the resource the permission applies to as we'll do the filtering & matching in + * the calling method + * @param subject + * @param required + * @return the set of JMXResources upon which the subject has been granted the required permission + */ + private Set<JMXResource> getPermittedResources(RoleResource subject, Permission required) + { + return getPermissions.apply(subject) + .stream() + .filter(details -> details.permission == required) + .map(details -> (JMXResource)details.resource) + .collect(Collectors.toSet()); + } + + /** + * Check whether a required permission has been granted to the authenticated subject on a specific resource + * @param subject + * @param permission + * @param resource + * @return true if the Subject has been granted the required permission on the specified resource; false otherwise + */ + private boolean hasPermission(RoleResource subject, Permission permission, JMXResource resource) + { + return getPermissions.apply(subject) + .stream() + .anyMatch(details -> details.permission == permission && details.resource.equals(resource)); + } + + /** + * Given a set of JMXResources upon which the Subject has been granted a particular permission, + * check whether any match the pattern-type ObjectName representing the target of the method + * invocation. At this point, we are sure that whatever the required permission, the Subject + * has definitely been granted it against this set of JMXResources. The job of this method is + * only to verify that the target of the invocation is covered by the members of the set. + * + * @param target + * @param permittedResources + * @return true if all registered beans which match the target can also be matched by the + * JMXResources the subject has been granted permissions on; false otherwise + */ + private boolean checkPattern(ObjectName target, Set<JMXResource> permittedResources) + { + // if the required permission was granted on the root JMX resource, then we're done + if (permittedResources.contains(JMXResource.root())) + return true; + + // Get the full set of beans which match the target pattern + Set<ObjectName> targetNames = queryNames.apply(target); + + // Iterate over the resources the permission has been granted on. Some of these may + // be patterns, so query the server to retrieve the full list of matching names and + // remove those from the target set. Once the target set is empty (i.e. all required + // matches have been satisfied), the requirement is met. + // If there are still unsatisfied targets after all the JMXResources have been processed, + // there are insufficient grants to permit the operation. + for (JMXResource resource : permittedResources) + { + try + { + Set<ObjectName> matchingNames = queryNames.apply(ObjectName.getInstance(resource.getObjectName())); + targetNames.removeAll(matchingNames); + if (targetNames.isEmpty()) + return true; + } + catch (MalformedObjectNameException e) + { + logger.warn("Permissions for JMX resource contains invalid ObjectName {}", resource.getObjectName()); + } + } + + logger.trace("Subject does not have sufficient permissions on all MBeans matching the target pattern {}", target); + return false; + } + + /** + * Given a set of JMXResources upon which the Subject has been granted a particular permission, + * check whether any match the ObjectName representing the target of the method invocation. + * At this point, we are sure that whatever the required permission, the Subject has definitely + * been granted it against this set of JMXResources. The job of this method is only to verify + * that the target of the invocation is matched by a member of the set. + * + * @param target + * @param permittedResources + * @return true if at least one of the permitted resources matches the target; false otherwise + */ + private boolean checkExact(ObjectName target, Set<JMXResource> permittedResources) + { + // if the required permission was granted on the root JMX resource, then we're done + if (permittedResources.contains(JMXResource.root())) + return true; + + for (JMXResource resource : permittedResources) + { + try + { + if (ObjectName.getInstance(resource.getObjectName()).apply(target)) + return true; + } + catch (MalformedObjectNameException e) + { + logger.warn("Permissions for JMX resource contains invalid ObjectName {}", resource.getObjectName()); + } + } + + logger.trace("Subject does not have sufficient permissions on target MBean {}", target); + return false; + } + + /** + * Mapping between method names and the permission required to invoke them. Note, these + * names refer to methods on MBean instances invoked via the MBeanServer. + * @param methodName + * @return + */ + private static Permission getRequiredPermission(String methodName) + { + switch (methodName) + { + case "getAttribute": + case "getAttributes": + return Permission.SELECT; + case "setAttribute": + case "setAttributes": + return Permission.MODIFY; + case "invoke": + return Permission.EXECUTE; + case "getInstanceOf": + case "getMBeanInfo": + case "hashCode": + case "isInstanceOf": + case "isRegistered": + case "queryMBeans": + case "queryNames": + return Permission.DESCRIBE; + default: + logger.debug("Access denied, method name {} does not map to any defined permission", methodName); + return null; + } + } + + /** + * Invoke a method on the MBeanServer instance. This is called when authorization is not required (because + * AllowAllAuthorizer is configured, or because the invocation is being performed by the JMXConnector + * itself rather than by a connected client), and also when a call from an authenticated subject + * has been successfully authorized + * + * @param method + * @param args + * @return + * @throws Throwable + */ + private Object invoke(Method method, Object[] args) throws Throwable + { + try + { + return method.invoke(mbs, args); + } + catch (InvocationTargetException e) //Catch any exception that might have been thrown by the mbeans + { + Throwable t = e.getCause(); //Throw the exception that nodetool etc expects + throw t; + } + } + + /** + * Query the configured IAuthorizer for the set of all permissions granted on JMXResources to a specific subject + * @param subject + * @return All permissions granted to the specfied subject (including those transitively inherited from + * any roles the subject has been granted), filtered to include only permissions granted on + * JMXResources + */ + private static Set<PermissionDetails> loadPermissions(RoleResource subject) + { + // get all permissions for the specified subject. We'll cache them as it's likely + // we'll receive multiple lookups for the same subject (but for different resources + // and permissions) in quick succession + return DatabaseDescriptor.getAuthorizer().list(AuthenticatedUser.SYSTEM_USER, Permission.ALL, null, subject) + .stream() + .filter(details -> details.resource instanceof JMXResource) + .collect(Collectors.toSet()); + } + + private static final class JMXPermissionsCache extends AuthCache<RoleResource, Set<PermissionDetails>> + { + protected JMXPermissionsCache() + { + super("JMXPermissionsCache", + DatabaseDescriptor::setPermissionsValidity, + DatabaseDescriptor::getPermissionsValidity, + DatabaseDescriptor::setPermissionsUpdateInterval, + DatabaseDescriptor::getPermissionsUpdateInterval, + DatabaseDescriptor::setPermissionsCacheMaxEntries, + DatabaseDescriptor::getPermissionsCacheMaxEntries, + AuthorizationProxy::loadPermissions, + () -> true); + } - - public Set<PermissionDetails> get(RoleResource roleResource) - { - try - { - return super.get(roleResource); - } - catch (Exception e) - { - // because the outer class uses this method as Function<RoleResource, Set<PermissionDetails>>, - // which can be overridden for testing, it cannot throw checked exceptions. So here we simply - // use guava's propagation helper. - throw Throwables.propagate(e); - } - } + } +} diff --cc test/unit/org/apache/cassandra/auth/AuthCacheTest.java index 0000000,0000000..0030603 new file mode 100644 --- /dev/null +++ b/test/unit/org/apache/cassandra/auth/AuthCacheTest.java @@@ -1,0 -1,0 +1,164 @@@ ++/* ++ * Licensed to the Apache Software Foundation (ASF) under one ++ * or more contributor license agreements. See the NOTICE file ++ * distributed with this work for additional information ++ * regarding copyright ownership. The ASF licenses this file ++ * to you under the Apache License, Version 2.0 (the ++ * "License"); you may not use this file except in compliance ++ * with the License. You may obtain a copy of the License at ++ * ++ * http://www.apache.org/licenses/LICENSE-2.0 ++ * ++ * Unless required by applicable law or agreed to in writing, software ++ * distributed under the License is distributed on an "AS IS" BASIS, ++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. ++ * See the License for the specific language governing permissions and ++ * limitations under the License. ++ */ ++package org.apache.cassandra.auth; ++ ++import java.util.function.Consumer; ++import java.util.function.Function; ++import java.util.function.Supplier; ++ ++import org.junit.Test; ++ ++import org.apache.cassandra.db.ConsistencyLevel; ++import org.apache.cassandra.exceptions.UnavailableException; ++ ++import static org.junit.Assert.assertEquals; ++ ++public class AuthCacheTest ++{ ++ private int loadCounter = 0; ++ private int validity = 2000; ++ private boolean isCacheEnabled = true; ++ ++ @Test ++ public void testCacheLoaderIsCalledOnFirst() ++ { ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(1, loadCounter); ++ } ++ ++ @Test ++ public void testCacheLoaderIsNotCalledOnSecond() ++ { ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ authCache.get("10"); ++ assertEquals(1, loadCounter); ++ ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(1, loadCounter); ++ } ++ ++ @Test ++ public void testCacheLoaderIsAlwaysCalledWhenDisabled() ++ { ++ isCacheEnabled = false; ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ ++ authCache.get("10"); ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(2, loadCounter); ++ } ++ ++ @Test ++ public void testCacheLoaderIsAlwaysCalledWhenValidityIsZero() ++ { ++ setValidity(0); ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ ++ authCache.get("10"); ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(2, loadCounter); ++ } ++ ++ @Test ++ public void testCacheLoaderIsCalledAfterFullInvalidate() ++ { ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ authCache.get("10"); ++ ++ authCache.invalidate(); ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(2, loadCounter); ++ } ++ ++ @Test ++ public void testCacheLoaderIsCalledAfterInvalidateKey() ++ { ++ TestCache<String, Integer> authCache = new TestCache<>(this::countingLoader, this::setValidity, () -> validity, () -> isCacheEnabled); ++ authCache.get("10"); ++ ++ authCache.invalidate("10"); ++ int result = authCache.get("10"); ++ ++ assertEquals(10, result); ++ assertEquals(2, loadCounter); ++ } ++ ++ @Test(expected = UnavailableException.class) ++ public void testCassandraExceptionPassThroughWhenCacheEnabled() ++ { ++ TestCache<String, Integer> cache = new TestCache<>(s -> { ++ throw new UnavailableException(ConsistencyLevel.QUORUM, 3, 1); ++ }, this::setValidity, () -> validity, () -> isCacheEnabled); ++ ++ cache.get("expect-exception"); ++ } ++ ++ @Test(expected = UnavailableException.class) ++ public void testCassandraExceptionPassThroughWhenCacheDisable() ++ { ++ isCacheEnabled = false; ++ TestCache<String, Integer> cache = new TestCache<>(s -> { ++ throw new UnavailableException(ConsistencyLevel.QUORUM, 3, 1); ++ }, this::setValidity, () -> validity, () -> isCacheEnabled); ++ ++ cache.get("expect-exception"); ++ } ++ ++ private void setValidity(int validity) ++ { ++ this.validity = validity; ++ } ++ ++ private Integer countingLoader(String s) ++ { ++ loadCounter++; ++ return Integer.parseInt(s); ++ } ++ ++ private static class TestCache<K, V> extends AuthCache<K, V> ++ { ++ private static int nameCounter = 0; // Allow us to create many instances of cache with same name prefix ++ ++ TestCache(Function<K, V> loadFunction, Consumer<Integer> setValidityDelegate, Supplier<Integer> getValidityDelegate, Supplier<Boolean> cacheEnabledDelegate) ++ { ++ super("TestCache" + nameCounter++, ++ setValidityDelegate, ++ getValidityDelegate, ++ (updateInterval) -> { ++ }, ++ () -> 1000, ++ (maxEntries) -> { ++ }, ++ () -> 10, ++ loadFunction, ++ cacheEnabledDelegate); ++ } ++ } ++} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
