Merge branch 'cassandra-4.0' into cassandra-4.1

This commit is contained in:
Sam Tunnicliffe 2025-01-17 09:07:33 +00:00
commit d14d16926e
9 changed files with 247 additions and 19 deletions

View File

@ -19,6 +19,9 @@ Merged from 4.0:
* Fix text containing "/*" being interpreted as multiline comment in cqlsh (CASSANDRA-17667)
* Fix indexing of a frozen collection that is the clustering key and reversed (CASSANDRA-19889)
* Emit error when altering a table with non-frozen UDTs with nested non-frozen collections the same way as done upon table creation (CASSANDRA-19925)
Merged from 3.0
* Tighten up permissions on system keyspaces (CASSANDRA-20090)
* Fix incorrect column identifier bytes problem when renaming a column (CASSANDRA-18956)
4.1.7

View File

@ -66,4 +66,11 @@ public enum Permission
public static final Set<Permission> ALL =
Sets.immutableEnumSet(EnumSet.range(Permission.CREATE, Permission.EXECUTE));
public static final Set<Permission> NONE = ImmutableSet.of();
/**
* Set of Permissions which may never be granted on any system keyspace, or table in a system keyspace, to any role.
* (Only SELECT, DESCRIBE and ALTER may ever be granted).
*/
public static final Set<Permission> INVALID_FOR_SYSTEM_KEYSPACES =
Sets.immutableEnumSet(EnumSet.complementOf(EnumSet.of(Permission.SELECT, Permission.DESCRIBE, Permission.ALTER)));
}

View File

@ -19,6 +19,7 @@ package org.apache.cassandra.auth;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
import org.apache.cassandra.utils.Hex;
@ -27,18 +28,33 @@ public final class Resources
/**
* Construct a chain of resource parents starting with the resource and ending with the root.
*
* @param resource The staring point.
* @param resource The starting point.
* @return list of resource in the chain form start to the root.
*/
public static List<? extends IResource> chain(IResource resource)
{
List<IResource> chain = new ArrayList<IResource>();
return chain(resource, (r) -> true);
}
/**
* Construct a chain of resource parents starting with the resource and ending with the root. Only resources which
* satisfy the supplied predicate will be included.
*
* @param resource The starting point.
* @param filter can be used to omit specific resources from the chain
* @return list of resource in the chain form start to the root.
*/
public static List<? extends IResource> chain(IResource resource, Predicate<IResource> filter)
{
List<IResource> chain = new ArrayList<>(4);
while (true)
{
chain.add(resource);
if (!resource.hasParent())
break;
resource = resource.getParent();
if (filter.test(resource))
chain.add(resource);
if (!resource.hasParent())
break;
resource = resource.getParent();
}
return chain;
}

View File

@ -17,11 +17,13 @@
*/
package org.apache.cassandra.cql3.statements;
import java.util.Collections;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.cassandra.audit.AuditLogContext;
import org.apache.cassandra.audit.AuditLogEntryType;
import org.apache.cassandra.auth.DataResource;
import org.apache.cassandra.auth.IAuthorizer;
import org.apache.cassandra.auth.IResource;
import org.apache.cassandra.auth.Permission;
@ -29,6 +31,8 @@ import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.RoleName;
import org.apache.cassandra.exceptions.RequestExecutionException;
import org.apache.cassandra.exceptions.RequestValidationException;
import org.apache.cassandra.exceptions.UnauthorizedException;
import org.apache.cassandra.schema.SchemaConstants;
import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.service.ClientWarn;
import org.apache.cassandra.transport.messages.ResultMessage;
@ -40,6 +44,23 @@ public class GrantPermissionsStatement extends PermissionsManagementStatement
super(permissions, resource, grantee);
}
public void validate(ClientState state) throws RequestValidationException
{
super.validate(state);
if (resource instanceof DataResource)
{
DataResource data = (DataResource) resource;
// Only a subset of permissions can be granted on non-virtual system keyspaces
if (!data.isRootLevel()
&& SchemaConstants.isNonVirtualSystemKeyspace(data.getKeyspace())
&& !Collections.disjoint(permissions, Permission.INVALID_FOR_SYSTEM_KEYSPACES))
{
throw new UnauthorizedException("Granting permissions on system keyspaces is strictly limited, " +
"this operation is not permitted");
}
}
}
public ResultMessage execute(ClientState state) throws RequestValidationException, RequestExecutionException
{
IAuthorizer authorizer = DatabaseDescriptor.getAuthorizer();

View File

@ -116,6 +116,16 @@ public final class SchemaConstants
|| isReplicatedSystemKeyspace(keyspaceName);
}
/**
* @return whether or not the keyspace is a non-virtual, system keyspace
*/
public static boolean isNonVirtualSystemKeyspace(String keyspaceName)
{
final String lowercaseKeyspaceName = keyspaceName.toLowerCase();
return LOCAL_SYSTEM_KEYSPACE_NAMES.contains(lowercaseKeyspaceName)
|| REPLICATED_SYSTEM_KEYSPACE_NAMES.contains(lowercaseKeyspaceName);
}
/**
* Returns the set of all system keyspaces
* @return all system keyspaces

View File

@ -21,7 +21,6 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@ -31,6 +30,7 @@ import java.util.concurrent.atomic.AtomicLong;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import org.slf4j.Logger;
@ -54,6 +54,7 @@ import org.apache.cassandra.dht.Datacenters;
import org.apache.cassandra.exceptions.AuthenticationException;
import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.exceptions.UnauthorizedException;
import org.apache.cassandra.tracing.TraceKeyspace;
import org.apache.cassandra.utils.FBUtilities;
import org.apache.cassandra.utils.JVMStabilityInspector;
import org.apache.cassandra.utils.MD5Digest;
@ -67,29 +68,39 @@ public class ClientState
{
private static final Logger logger = LoggerFactory.getLogger(ClientState.class);
private static final Set<IResource> READABLE_SYSTEM_RESOURCES = new HashSet<>();
private static final Set<IResource> PROTECTED_AUTH_RESOURCES = new HashSet<>();
public static final ImmutableSet<IResource> READABLE_SYSTEM_RESOURCES;
public static final ImmutableSet<IResource> PROTECTED_AUTH_RESOURCES;
static
{
// We want these system cfs to be always readable to authenticated users since many tools rely on them
// (nodetool, cqlsh, bulkloader, etc.)
for (String cf : Arrays.asList(SystemKeyspace.LOCAL, SystemKeyspace.LEGACY_PEERS, SystemKeyspace.PEERS_V2))
READABLE_SYSTEM_RESOURCES.add(DataResource.table(SchemaConstants.SYSTEM_KEYSPACE_NAME, cf));
ImmutableSet.Builder<IResource> readableBuilder = ImmutableSet.builder();
for (String cf : Arrays.asList(SystemKeyspace.LOCAL, SystemKeyspace.LEGACY_PEERS, SystemKeyspace.PEERS_V2,
SystemKeyspace.LEGACY_SIZE_ESTIMATES, SystemKeyspace.TABLE_ESTIMATES))
readableBuilder.add(DataResource.table(SchemaConstants.SYSTEM_KEYSPACE_NAME, cf));
// make all schema tables readable by default (required by the drivers)
SchemaKeyspaceTables.ALL.forEach(table -> READABLE_SYSTEM_RESOURCES.add(DataResource.table(SchemaConstants.SCHEMA_KEYSPACE_NAME, table)));
SchemaKeyspaceTables.ALL.forEach(table -> readableBuilder.add(DataResource.table(SchemaConstants.SCHEMA_KEYSPACE_NAME, table)));
// make system_traces readable by all or else tracing will require explicit grants
readableBuilder.add(DataResource.table(SchemaConstants.TRACE_KEYSPACE_NAME, TraceKeyspace.EVENTS));
readableBuilder.add(DataResource.table(SchemaConstants.TRACE_KEYSPACE_NAME, TraceKeyspace.SESSIONS));
// make all virtual schema tables readable by default as well
VirtualSchemaKeyspace.instance.tables().forEach(t -> READABLE_SYSTEM_RESOURCES.add(t.metadata().resource));
VirtualSchemaKeyspace.instance.tables().forEach(t -> readableBuilder.add(t.metadata().resource));
READABLE_SYSTEM_RESOURCES = readableBuilder.build();
ImmutableSet.Builder<IResource> protectedBuilder = ImmutableSet.builder();
// neither clients nor tools need authentication/authorization
if (DatabaseDescriptor.isDaemonInitialized())
{
PROTECTED_AUTH_RESOURCES.addAll(DatabaseDescriptor.getAuthenticator().protectedResources());
PROTECTED_AUTH_RESOURCES.addAll(DatabaseDescriptor.getAuthorizer().protectedResources());
PROTECTED_AUTH_RESOURCES.addAll(DatabaseDescriptor.getRoleManager().protectedResources());
protectedBuilder.addAll(DatabaseDescriptor.getAuthenticator().protectedResources());
protectedBuilder.addAll(DatabaseDescriptor.getAuthorizer().protectedResources());
protectedBuilder.addAll(DatabaseDescriptor.getRoleManager().protectedResources());
}
PROTECTED_AUTH_RESOURCES = protectedBuilder.build();
}
// Current user for the session
@ -425,12 +436,16 @@ public class ClientState
preventSystemKSSchemaModification(keyspace, resource, perm);
// Some system data is always readable
if ((perm == Permission.SELECT) && READABLE_SYSTEM_RESOURCES.contains(resource))
return;
// Modifications to any resource upon which the authenticator, authorizer or role manager depend should not be
// be performed by users
if (PROTECTED_AUTH_RESOURCES.contains(resource))
if ((perm == Permission.CREATE) || (perm == Permission.ALTER) || (perm == Permission.DROP))
throw new UnauthorizedException(String.format("%s schema is protected", resource));
ensurePermission(perm, resource);
}
@ -444,6 +459,24 @@ public class ClientState
if (((FunctionResource)resource).getKeyspace().equals(SchemaConstants.SYSTEM_KEYSPACE_NAME))
return;
if (resource instanceof DataResource && isOrdinaryUser())
{
DataResource dataResource = (DataResource)resource;
if (!dataResource.isRootLevel())
{
String keyspace = dataResource.getKeyspace();
// A user may have permissions granted on ALL KEYSPACES, but this should exclude system keyspaces. Any
// permission on those keyspaces or their tables must be granted to the user either explicitly or
// transitively. The set of grantable permissions for non-virtual system keyspaces is further limited,
// see the Permission enum for details.
if (SchemaConstants.isSystemKeyspace(keyspace))
{
ensurePermissionOnResourceChain(perm, Resources.chain(dataResource, IResource::hasParent));
return;
}
}
}
ensurePermissionOnResourceChain(perm, resource);
}
@ -466,7 +499,12 @@ public class ClientState
private void ensurePermissionOnResourceChain(Permission perm, IResource resource)
{
List<? extends IResource> resources = Resources.chain(resource);
ensurePermissionOnResourceChain(perm, Resources.chain(resource));
}
private void ensurePermissionOnResourceChain(Permission perm, List<? extends IResource> resources)
{
IResource resource = resources.get(0);
if (DatabaseDescriptor.getAuthFromRoot())
resources = Lists.reverse(resources);

View File

@ -17,6 +17,11 @@
*/
package org.apache.cassandra.auth;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import com.google.common.collect.Iterables;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;
@ -25,7 +30,19 @@ import com.datastax.driver.core.ResultSet;
import org.apache.cassandra.Util;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.CQLTester;
import org.apache.cassandra.db.ConsistencyLevel;
import org.apache.cassandra.db.SystemKeyspace;
import org.apache.cassandra.schema.Schema;
import org.apache.cassandra.schema.SchemaConstants;
import org.apache.cassandra.schema.TableMetadata;
import org.apache.cassandra.transport.ProtocolVersion;
import static java.lang.String.format;
import static org.apache.cassandra.schema.SchemaConstants.LOCAL_SYSTEM_KEYSPACE_NAMES;
import static org.apache.cassandra.schema.SchemaConstants.REPLICATED_SYSTEM_KEYSPACE_NAMES;
import static org.apache.cassandra.schema.SchemaConstants.SCHEMA_KEYSPACE_NAME;
import static org.apache.cassandra.schema.SchemaConstants.SYSTEM_KEYSPACE_NAME;
import static org.apache.cassandra.schema.SchemaConstants.TRACE_KEYSPACE_NAME;
import static org.junit.Assert.assertTrue;
public class GrantAndRevokeTest extends CQLTester
@ -202,7 +219,6 @@ public class GrantAndRevokeTest extends CQLTester
"DROP MATERIALIZED VIEW " + mv);
assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",
"DROP INDEX " + index);
}
@Test
@ -385,4 +401,111 @@ public class GrantAndRevokeTest extends CQLTester
res = executeNet("REVOKE SELECT, MODIFY ON KEYSPACE revoke_yeah FROM " + user);
assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted MODIFY on <keyspace revoke_yeah>");
}
@Test
public void testSpecificGrantsOnSystemKeyspaces() throws Throwable
{
// Granting specific permissions on system keyspaces should not be allowed if those permissions include any from
// the denylist Permission.INVALID_FOR_SYSTEM_KEYSPACES. By this definition, GRANT ALL on any system keyspace,
// or a table within one, should be rejected.
useSuperUser();
executeNet("CREATE ROLE '" + user + "'");
String responseMsg = "Granting permissions on system keyspaces is strictly limited, this operation is not permitted";
for (String keyspace : Iterables.concat(LOCAL_SYSTEM_KEYSPACE_NAMES, REPLICATED_SYSTEM_KEYSPACE_NAMES))
{
assertUnauthorizedQuery(responseMsg, format("GRANT ALL PERMISSIONS ON KEYSPACE %s TO %s", keyspace, user));
DataResource keyspaceResource = DataResource.keyspace(keyspace);
for (Permission p : keyspaceResource.applicablePermissions())
maybeRejectGrant(p, responseMsg, format("GRANT %s ON KEYSPACE %s TO %s", p.name(), keyspace, user));
assertUnauthorizedQuery(responseMsg, format("GRANT ALL PERMISSIONS ON ALL TABLES IN KEYSPACE %s TO %s", keyspace, user));
for (TableMetadata table : Schema.instance.getKeyspaceMetadata(keyspace).tables)
{
DataResource tableResource = DataResource.table(keyspace, table.name);
assertUnauthorizedQuery(responseMsg, format("GRANT ALL PERMISSIONS ON %s TO %s", table, user));
for (Permission p : tableResource.applicablePermissions())
maybeRejectGrant(p, responseMsg, format("GRANT %s ON %s TO %s", p.name(), table, user));
}
}
}
@Test
public void testGrantOnAllKeyspaces() throws Throwable
{
// Granting either specific or ALL permissions on ALL KEYSPACES is allowed, however these permissions are
// effective for non-system keyspaces only. If for any reason it is necessary to modify permissions on
// on a system keyspace, it must be done using keyspace specific grant statements.
useSuperUser();
executeNet(String.format("CREATE ROLE %s WITH LOGIN = TRUE AND password='%s'", user, pass));
executeNet(String.format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}", SchemaConstants.TRACE_KEYSPACE_NAME));
executeNet("CREATE KEYSPACE user_keyspace WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}");
executeNet("CREATE TABLE user_keyspace.t1 (k int PRIMARY KEY)");
useUser(user, pass);
assertUnauthorizedQuery("User user has no MODIFY permission on <table user_keyspace.t1> or any of its parents",
"INSERT INTO user_keyspace.t1 (k) VALUES (0)");
assertUnauthorizedQuery("User user has no MODIFY permission on <table system.local> or any of its parents",
"INSERT INTO system.local(key) VALUES ('invalid')");
useSuperUser();
executeNet(ProtocolVersion.CURRENT, format("GRANT MODIFY ON ALL KEYSPACES TO %s", user));
useUser(user, pass);
// User now has write permission on non-system keyspaces only
executeNet(ProtocolVersion.CURRENT, "INSERT INTO user_keyspace.t1 (k) VALUES (0)");
assertUnauthorizedQuery("User user has no MODIFY permission on <table system.local> or any of its parents",
"INSERT INTO system.local(key) VALUES ('invalid')");
// A non-superuser only has read access to a pre-defined set of system tables and all system_schema/traces
// tables and granting ALL permissions on ALL keyspaces also does not affect this.
maybeReadSystemTables(false);
useSuperUser();
executeNet(ProtocolVersion.CURRENT, format("GRANT ALL PERMISSIONS ON ALL KEYSPACES TO %s", user));
maybeReadSystemTables(false);
// A superuser can still read system tables
useSuperUser();
maybeReadSystemTables(true);
// and also write to them, though this is still strongly discouraged
executeNet(ProtocolVersion.CURRENT, "INSERT INTO system.peers_v2(peer, peer_port, data_center) VALUES ('127.0.100.100', 7012, 'invalid_dc')");
}
private void maybeReadSystemTables(boolean superuser) throws Throwable
{
if (superuser)
useSuperUser();
else
useUser(user, pass);
Set<String> readableKeyspaces = new HashSet<>(Arrays.asList(SCHEMA_KEYSPACE_NAME, TRACE_KEYSPACE_NAME));
Set<String> readableSystemTables = new HashSet<>(Arrays.asList(SystemKeyspace.LOCAL,
SystemKeyspace.PEERS_V2,
SystemKeyspace.LEGACY_PEERS,
SystemKeyspace.LEGACY_SIZE_ESTIMATES,
SystemKeyspace.TABLE_ESTIMATES));
for (String keyspace : Iterables.concat(LOCAL_SYSTEM_KEYSPACE_NAMES, REPLICATED_SYSTEM_KEYSPACE_NAMES))
{
for (TableMetadata table : Schema.instance.getKeyspaceMetadata(keyspace).tables)
{
if (superuser || (readableKeyspaces.contains(keyspace) || (keyspace.equals(SYSTEM_KEYSPACE_NAME) && readableSystemTables.contains(table.name))))
{
executeNet(ProtocolVersion.CURRENT, ConsistencyLevel.ONE, format("SELECT * FROM %s LIMIT 1", table));
}
else
{
assertUnauthorizedQuery(format("User %s has no SELECT permission on %s or any of its parents", user, table.resource),
format("SELECT * FROM %s LIMIT 1", table));
}
}
}
}
private void maybeRejectGrant(Permission p, String errorResponse, String grant) throws Throwable
{
if (Permission.INVALID_FOR_SYSTEM_KEYSPACES.contains(p))
assertUnauthorizedQuery(errorResponse, grant);
else
executeNet(ProtocolVersion.CURRENT, grant);
}
}

View File

@ -71,6 +71,7 @@ import org.apache.cassandra.concurrent.ScheduledExecutors;
import org.apache.cassandra.concurrent.Stage;
import org.apache.cassandra.config.DataStorageSpec;
import org.apache.cassandra.config.EncryptionOptions;
import org.apache.cassandra.db.ConsistencyLevel;
import org.apache.cassandra.db.virtual.VirtualKeyspaceRegistry;
import org.apache.cassandra.db.virtual.VirtualSchemaKeyspace;
import org.apache.cassandra.exceptions.InvalidRequestException;
@ -1258,6 +1259,13 @@ public abstract class CQLTester
return Schema.instance.getTableMetadata(KEYSPACE, currentTable());
}
protected com.datastax.driver.core.ResultSet executeNet(ProtocolVersion protocolVersion, ConsistencyLevel consistency, String query) throws Throwable
{
Statement statement = new SimpleStatement(formatQuery(query));
statement = statement.setConsistencyLevel(com.datastax.driver.core.ConsistencyLevel.valueOf(consistency.name()));
return sessionNet(protocolVersion).execute(statement);
}
protected com.datastax.driver.core.ResultSet executeNet(ProtocolVersion protocolVersion, String query, Object... values) throws Throwable
{
return sessionNet(protocolVersion).execute(formatQuery(query), values);

View File

@ -55,7 +55,9 @@ public class ClientStateTest
SchemaLoader.createKeyspace(SchemaConstants.AUTH_KEYSPACE_NAME,
KeyspaceParams.simple(1),
Iterables.toArray(AuthKeyspace.metadata().tables, TableMetadata.class));
DatabaseDescriptor.setRoleManager(new AuthTestUtils.LocalCassandraRoleManager());
DatabaseDescriptor.getRoleManager().setup();
Roles.init();
AuthCacheService.initializeAndRegisterCaches();
}