This is an automated email from the ASF dual-hosted git repository.
samt pushed a commit to branch cassandra-4.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.0 by this push:
new 066c489d76 Tighten up permissions on system keyspaces
066c489d76 is described below
commit 066c489d764d54fc574d77c9006eb9d6399d4e93
Author: Sam Tunnicliffe <[email protected]>
AuthorDate: Thu Feb 6 09:10:51 2025 +0000
Tighten up permissions on system keyspaces
* Restrict which permissions can be granted on system keyspaces
* Ensure that GRANT... ON ALL KEYSPACES excludes system keyspaces
* Add system_traces to the always readable set
Patch by Sam Tunnicliffe and Francisco Guerrero; reviewed by Sam
Tunnicliffe and Francisco Guerrero for CASSANDRA-20090
Note: this is a re-application of the commit to cassandra-4.0 only,
to fix a bad merge the first time around.
Co-authored-by: Francisco Guerrero <[email protected]>
---
CHANGES.txt | 1 +
src/java/org/apache/cassandra/auth/Permission.java | 7 +
src/java/org/apache/cassandra/auth/Resources.java | 28 +-
.../cql3/statements/GrantPermissionsStatement.java | 21 ++
.../apache/cassandra/schema/SchemaConstants.java | 10 +
.../org/apache/cassandra/service/ClientState.java | 61 +++-
.../apache/cassandra/auth/GrantAndRevokeTest.java | 359 +++++++++++++++++++++
test/unit/org/apache/cassandra/cql3/CQLTester.java | 2 +-
8 files changed, 470 insertions(+), 19 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 09a9bee44d..db7080bc16 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,6 @@
4.0.17
* Fix autocompletion for role names/user names (CASSANDRA-20175)
+ * Re-apply: Tighten up permission on system keyspaces (CASSANDRA-20040)
4.0.16
diff --git a/src/java/org/apache/cassandra/auth/Permission.java
b/src/java/org/apache/cassandra/auth/Permission.java
index d552280e64..11c7aeb05b 100644
--- a/src/java/org/apache/cassandra/auth/Permission.java
+++ b/src/java/org/apache/cassandra/auth/Permission.java
@@ -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)));
}
diff --git a/src/java/org/apache/cassandra/auth/Resources.java
b/src/java/org/apache/cassandra/auth/Resources.java
index 653cd46e32..2863710632 100644
--- a/src/java/org/apache/cassandra/auth/Resources.java
+++ b/src/java/org/apache/cassandra/auth/Resources.java
@@ -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;
}
diff --git
a/src/java/org/apache/cassandra/cql3/statements/GrantPermissionsStatement.java
b/src/java/org/apache/cassandra/cql3/statements/GrantPermissionsStatement.java
index 3db20e3841..104cb8a949 100644
---
a/src/java/org/apache/cassandra/cql3/statements/GrantPermissionsStatement.java
+++
b/src/java/org/apache/cassandra/cql3/statements/GrantPermissionsStatement.java
@@ -17,16 +17,20 @@
*/
package org.apache.cassandra.cql3.statements;
+import java.util.Collections;
import java.util.Set;
import org.apache.cassandra.audit.AuditLogContext;
import org.apache.cassandra.audit.AuditLogEntryType;
+import org.apache.cassandra.auth.DataResource;
import org.apache.cassandra.auth.IResource;
import org.apache.cassandra.auth.Permission;
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.transport.messages.ResultMessage;
@@ -37,6 +41,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 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
{
DatabaseDescriptor.getAuthorizer().grant(state.getUser(), permissions,
resource, grantee);
diff --git a/src/java/org/apache/cassandra/schema/SchemaConstants.java
b/src/java/org/apache/cassandra/schema/SchemaConstants.java
index 7b6b7de490..130227d844 100644
--- a/src/java/org/apache/cassandra/schema/SchemaConstants.java
+++ b/src/java/org/apache/cassandra/schema/SchemaConstants.java
@@ -108,4 +108,14 @@ public final class SchemaConstants
|| isReplicatedSystemKeyspace(keyspaceName)
|| isVirtualSystemKeyspace(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);
+ }
}
diff --git a/src/java/org/apache/cassandra/service/ClientState.java
b/src/java/org/apache/cassandra/service/ClientState.java
index f76e7e3a4f..adca727709 100644
--- a/src/java/org/apache/cassandra/service/ClientState.java
+++ b/src/java/org/apache/cassandra/service/ClientState.java
@@ -21,11 +21,12 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Arrays;
-import java.util.HashSet;
import java.util.Optional;
+import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
+import com.google.common.collect.ImmutableSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -47,6 +48,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;
@@ -58,29 +60,37 @@ 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
@@ -384,9 +394,12 @@ 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));
@@ -403,6 +416,24 @@ public class ClientState
if
(((FunctionResource)resource).getKeyspace().equals(SchemaConstants.SYSTEM_KEYSPACE_NAME))
return;
+ if (resource instanceof DataResource && !(user.isSuper() ||
user.isSystem()))
+ {
+ 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 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);
}
@@ -425,7 +456,13 @@ public class ClientState
private void ensurePermissionOnResourceChain(Permission perm, IResource
resource)
{
- for (IResource r : Resources.chain(resource))
+ ensurePermissionOnResourceChain(perm, Resources.chain(resource));
+ }
+
+ private void ensurePermissionOnResourceChain(Permission perm, List<?
extends IResource> resources)
+ {
+ IResource resource = resources.get(0);
+ for (IResource r : resources)
if (authorize(r).contains(perm))
return;
diff --git a/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java
b/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java
new file mode 100644
index 0000000000..4ce56721d7
--- /dev/null
+++ b/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java
@@ -0,0 +1,359 @@
+/*
+ * 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.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+import com.google.common.collect.Iterables;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.PlainTextAuthProvider;
+import com.datastax.driver.core.Session;
+import com.datastax.driver.core.exceptions.UnauthorizedException;
+import org.apache.cassandra.Util;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.schema.Schema;
+import org.apache.cassandra.schema.SchemaConstants;
+import org.apache.cassandra.schema.TableMetadata;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.SystemKeyspace;
+import org.apache.cassandra.utils.Pair;
+
+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;
+
+public class GrantAndRevokeTest extends CQLTester
+{
+ private static final String user = "user";
+ private static final String pass = "12345";
+ private static final Pair<String, String> USER = Pair.create(user, pass);
+ private static final Pair<String, String> SUPERUSER =
Pair.create("cassandra", "cassandra");
+ private static int counter = 0;
+
+ @BeforeClass
+ public static void setUpClass()
+ {
+ DatabaseDescriptor.setAuthenticator(new PasswordAuthenticator());
+ DatabaseDescriptor.setAuthorizer(new CassandraAuthorizer());
+ DatabaseDescriptor.setRoleManager(new CassandraRoleManager());
+ DatabaseDescriptor.setPermissionsValidity(0);
+ CQLTester.setUpClass();
+ requireNetworkWithoutDriver();
+ DatabaseDescriptor.getRoleManager().setup();
+ DatabaseDescriptor.getAuthenticator().setup();
+ DatabaseDescriptor.getAuthorizer().setup();
+ }
+
+ @After
+ public void tearDown() throws Throwable
+ {
+ session(SUPERUSER).execute("DROP ROLE " + user);
+ Roles.clearCache();
+ }
+
+ private Session session(Pair<String, String> credentials)
+ {
+ Cluster cluster = Cluster.builder()
+ .addContactPoints(nativeAddr)
+ .withClusterName("Test Cluster " + counter++)
+ .withPort(nativePort)
+ .withAuthProvider(new
PlainTextAuthProvider(credentials.left, credentials.right))
+ .build();
+
+ return cluster.connect();
+ }
+
+ @Test
+ public void testGrantedKeyspace() throws Throwable
+ {
+ Session superuser = session(SUPERUSER);
+ superuser.execute(String.format("CREATE ROLE %s WITH LOGIN = TRUE AND
password='%s'", user, pass));
+ superuser.execute("GRANT CREATE ON KEYSPACE " + KEYSPACE_PER_TEST + "
TO " + user);
+ String table = KEYSPACE_PER_TEST + '.' +
createTable(KEYSPACE_PER_TEST, "CREATE TABLE %s (pk int, ck int, val int, val_2
text, PRIMARY KEY (pk, ck))");
+ String index = KEYSPACE_PER_TEST + ".idx_01";
+ createIndex(KEYSPACE_PER_TEST, "CREATE INDEX idx_01 ON %s (val_2)");
+ String type = KEYSPACE_PER_TEST + '.' + createType(KEYSPACE_PER_TEST,
"CREATE TYPE %s (a int, b text)");
+ String mv = KEYSPACE_PER_TEST + ".ks_mv_01";
+ superuser.execute("CREATE MATERIALIZED VIEW " + mv + " AS SELECT *
FROM " + table + " WHERE val IS NOT NULL AND pk IS NOT NULL AND ck IS NOT NULL
PRIMARY KEY (val, pk, ck)");
+
+ Session nonsuperuser = session(USER);
+ // ALTER and DROP tables created by somebody else
+ // Spin assert for effective auth changes.
+ final String spinAssertTable = table;
+ Util.spinAssertEquals(false, () -> {
+ try
+ {
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + spinAssertTable + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "INSERT
INTO %s (pk, ck, val, val_2) VALUES (1, 1, 1, '1')"));
+ }
+ catch(Throwable e)
+ {
+ return true;
+ }
+ return false;
+ }, 10);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "UPDATE %s SET
val = 1 WHERE pk = 1 AND ck = 1"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "DELETE FROM %s
WHERE pk = 1 AND ck = 2"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no SELECT
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM
%s WHERE pk = 1 AND ck = 1"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no SELECT
permission on <table " + table + "> or any of its parents",
+ "SELECT * FROM " + mv + " WHERE val = 1 AND pk
= 1 AND ck = 1");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE
%s"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s
ADD val_3 int"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no DROP
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "DROP TABLE
%s"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <keyspace " + KEYSPACE_PER_TEST + "> or any of its parents",
+ "ALTER TYPE " + type + " ADD c bigint");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no DROP
permission on <keyspace " + KEYSPACE_PER_TEST + "> or any of its parents",
+ "DROP TYPE " + type);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ "DROP MATERIALIZED VIEW " + mv);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ "DROP INDEX " + index);
+
+ superuser.execute("GRANT ALTER ON KEYSPACE " + KEYSPACE_PER_TEST + "
TO " + user);
+ superuser.execute("GRANT DROP ON KEYSPACE " + KEYSPACE_PER_TEST + " TO
" + user);
+ superuser.execute("GRANT SELECT ON KEYSPACE " + KEYSPACE_PER_TEST + "
TO " + user);
+ superuser.execute("GRANT MODIFY ON KEYSPACE " + KEYSPACE_PER_TEST + "
TO " + user);
+ // Spin assert for effective auth changes.
+ Util.spinAssertEquals(false, () -> {
+ try
+ {
+ nonsuperuser.execute("ALTER KEYSPACE " + KEYSPACE_PER_TEST + "
WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}");
+ }
+ catch(Throwable e)
+ {
+ return true;
+ }
+ return false;
+ }, 10);
+
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "INSERT INTO %s
(pk, ck, val, val_2) VALUES (1, 1, 1, '1')"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "UPDATE %s SET val
= 1 WHERE pk = 1 AND ck = 1"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "DELETE FROM %s
WHERE pk = 1 AND ck = 2"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s
WHERE pk = 1 AND ck = 1"));
+ nonsuperuser.execute("SELECT * FROM " + mv + " WHERE val = 1 AND pk =
1");
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE
%s"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s
ADD val_3 int"));
+ nonsuperuser.execute("DROP MATERIALIZED VIEW " + mv);
+ nonsuperuser.execute("DROP INDEX " + index);
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "DROP TABLE %s"));
+ nonsuperuser.execute("ALTER TYPE " + type + " ADD c bigint");
+ nonsuperuser.execute("DROP TYPE " + type);
+
+ // calling creatTableName to create a new table name that will be used
by the formatQuery
+ table = createTableName();
+ type = KEYSPACE_PER_TEST + "." + createTypeName();
+ mv = KEYSPACE_PER_TEST + ".ks_mv_02";
+ nonsuperuser.execute("CREATE TYPE " + type + " (a int, b text)");
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "CREATE TABLE %s
(pk int, ck int, val int, val_2 text, PRIMARY KEY (pk, ck))"));
+ nonsuperuser.execute("CREATE MATERIALIZED VIEW " + mv + " AS SELECT *
FROM " + table + " WHERE val IS NOT NULL AND pk IS NOT NULL AND ck IS NOT NULL
PRIMARY KEY (val, pk, ck)");
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "INSERT INTO %s
(pk, ck, val, val_2) VALUES (1, 1, 1, '1')"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "UPDATE %s SET val
= 1 WHERE pk = 1 AND ck = 1"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "DELETE FROM %s
WHERE pk = 1 AND ck = 2"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s
WHERE pk = 1 AND ck = 1"));
+ nonsuperuser.execute("SELECT * FROM " + mv + " WHERE val = 1 AND pk =
1");
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE
%s"));
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s
ADD val_3 int"));
+ nonsuperuser.execute("DROP MATERIALIZED VIEW " + mv);
+ nonsuperuser.execute(formatQuery(KEYSPACE_PER_TEST, "DROP TABLE %s"));
+ nonsuperuser.execute("ALTER TYPE " + type + " ADD c bigint");
+ nonsuperuser.execute("DROP TYPE " + type);
+
+ superuser.execute("REVOKE ALTER ON KEYSPACE " + KEYSPACE_PER_TEST + "
FROM " + user);
+ superuser.execute("REVOKE DROP ON KEYSPACE " + KEYSPACE_PER_TEST + "
FROM " + user);
+ superuser.execute("REVOKE SELECT ON KEYSPACE " + KEYSPACE_PER_TEST + "
FROM " + user);
+ superuser.execute("REVOKE MODIFY ON KEYSPACE " + KEYSPACE_PER_TEST + "
FROM " + user);
+
+ table = KEYSPACE_PER_TEST + "." + createTable(KEYSPACE_PER_TEST,
"CREATE TABLE %s (pk int, ck int, val int, val_2 text, PRIMARY KEY (pk, ck))");
+ type = KEYSPACE_PER_TEST + "." + createType(KEYSPACE_PER_TEST, "CREATE
TYPE %s (a int, b text)");
+ index = KEYSPACE_PER_TEST + ".idx_02";
+ createIndex(KEYSPACE_PER_TEST, "CREATE INDEX idx_02 ON %s (val_2)");
+ mv = KEYSPACE_PER_TEST + ".ks_mv_03";
+ superuser.execute("CREATE MATERIALIZED VIEW " + mv + " AS SELECT *
FROM " + table + " WHERE val IS NOT NULL AND pk IS NOT NULL AND ck IS NOT NULL
PRIMARY KEY (val, pk, ck)");
+
+ // Spin assert for effective auth changes.
+ final String spinAssertTable2 = table;
+ Util.spinAssertEquals(false, () -> {
+ try
+ {
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + spinAssertTable2 + "> or any of its parents",
+ "INSERT INTO " + spinAssertTable2 + "
(pk, ck, val, val_2) VALUES (1, 1, 1, '1')");
+ }
+ catch(Throwable e)
+ {
+ return true;
+ }
+ return false;
+ }, 10);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ "UPDATE " + table + " SET val = 1 WHERE pk = 1
AND ck = 1");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ "DELETE FROM " + table + " WHERE pk = 1 AND ck
= 2");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no SELECT
permission on <table " + table + "> or any of its parents",
+ "SELECT * FROM " + table + " WHERE pk = 1 AND
ck = 1");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no SELECT
permission on <table " + table + "> or any of its parents",
+ "SELECT * FROM " + mv + " WHERE val = 1 AND pk
= 1 AND ck = 1");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no MODIFY
permission on <table " + table + "> or any of its parents",
+ "TRUNCATE TABLE " + table);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE "
+ table + " ADD val_3 int"));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no DROP
permission on <table " + table + "> or any of its parents",
+ formatQuery(KEYSPACE_PER_TEST, "DROP TABLE " +
table));
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <keyspace " + KEYSPACE_PER_TEST + "> or any of its parents",
+ "ALTER TYPE " + type + " ADD c bigint");
+ assertUnauthorizedQuery(nonsuperuser, "User user has no DROP
permission on <keyspace " + KEYSPACE_PER_TEST + "> or any of its parents",
+ "DROP TYPE " + type);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ "DROP MATERIALIZED VIEW " + mv);
+ assertUnauthorizedQuery(nonsuperuser, "User user has no ALTER
permission on <table " + table + "> or any of its parents",
+ "DROP INDEX " + index);
+ }
+
+
+ @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.
+ Session superuser = session(SUPERUSER);
+ superuser.execute("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(superuser, responseMsg, format("GRANT ALL
PERMISSIONS ON KEYSPACE %s TO %s", keyspace, user));
+ DataResource keyspaceResource = DataResource.keyspace(keyspace);
+ for (Permission p : keyspaceResource.applicablePermissions())
+ maybeRejectGrant(superuser, p, responseMsg, format("GRANT %s
ON KEYSPACE %s TO %s", p.name(), keyspace, user));
+
+ for (TableMetadata table :
Schema.instance.getKeyspaceMetadata(keyspace).tables)
+ {
+ DataResource tableResource = DataResource.table(keyspace,
table.name);
+ assertUnauthorizedQuery(superuser, responseMsg, format("GRANT
ALL PERMISSIONS ON %s.\"%s\" TO %s", table.keyspace, table.name, user));
+ for (Permission p : tableResource.applicablePermissions())
+ maybeRejectGrant(superuser, p, responseMsg, format("GRANT
%s ON %s.\"%s\" TO %s", p.name(), table.keyspace, table.name, 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.
+ Session superuser = session(SUPERUSER);
+ superuser.execute(String.format("CREATE ROLE %s WITH LOGIN = TRUE AND
password='%s'", user, pass));
+ superuser.execute(String.format("ALTER KEYSPACE %s WITH replication =
{'class': 'SimpleStrategy', 'replication_factor': '1'}",
SchemaConstants.TRACE_KEYSPACE_NAME));
+ superuser.execute("CREATE KEYSPACE user_keyspace WITH replication =
{'class': 'SimpleStrategy', 'replication_factor': '1'}");
+ superuser.execute("CREATE TABLE user_keyspace.t1 (k int PRIMARY KEY)");
+
+ Session nonsuperuser = session(USER);
+ assertUnauthorizedQuery(nonsuperuser, "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(nonsuperuser, "User user has no MODIFY
permission on <table system.local> or any of its parents",
+ "INSERT INTO system.local(key) VALUES
('invalid')");
+
+ superuser.execute(format("GRANT MODIFY ON ALL KEYSPACES TO %s", user));
+ // User now has write permission on non-system keyspaces only
+ nonsuperuser.execute("INSERT INTO user_keyspace.t1 (k) VALUES (0)");
+ assertUnauthorizedQuery(nonsuperuser, "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(nonsuperuser, false);
+ superuser.execute(format("GRANT ALL PERMISSIONS ON ALL KEYSPACES TO
%s", user));
+ maybeReadSystemTables(nonsuperuser, false);
+
+ // A superuser can still read system tables
+ maybeReadSystemTables(superuser, true);
+ // and also write to them, though this is still strongly discouraged
+ superuser.execute("INSERT INTO system.peers(peer, data_center) VALUES
('127.0.100.100', 'invalid_dc')");
+
+ }
+
+ private void maybeReadSystemTables(Session session, boolean isSuper)
throws Throwable
+ {
+ Set<String> readableKeyspaces = new
HashSet<>(Arrays.asList(SchemaConstants.SCHEMA_KEYSPACE_NAME,
SchemaConstants.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 (isSuper || (readableKeyspaces.contains(keyspace) ||
(keyspace.equals(SchemaConstants.SYSTEM_KEYSPACE_NAME) &&
readableSystemTables.contains(table.name))))
+ {
+ session.execute(format("SELECT * FROM %s.\"%s\" LIMIT 1",
table.keyspace, table.name));
+ }
+ else
+ {
+ assertUnauthorizedQuery(session, format("User %s has no
SELECT permission on %s or any of its parents", user,
DataResource.table(table.keyspace, table.name)),
+ format("SELECT * FROM %s.\"%s\"
LIMIT 1", table.keyspace, table.name));
+ }
+ }
+ }
+ }
+
+ private void maybeRejectGrant(Session session, Permission p, String
errorResponse, String grant) throws Throwable
+ {
+ if (Permission.INVALID_FOR_SYSTEM_KEYSPACES.contains(p))
+ assertUnauthorizedQuery(session, errorResponse, grant);
+ else
+ session.execute(grant);
+ }
+
+ private void assertUnauthorizedQuery(Session session, String errorMessage,
String query) throws Throwable
+ {
+ try
+ {
+ session.execute(query);
+ Assert.fail("Query should be invalid but no error was thrown.
Query is: " + query);
+ }
+ catch (Exception e)
+ {
+ if (!UnauthorizedException.class.isAssignableFrom(e.getClass()))
+ {
+ Assert.fail("Query should be invalid but wrong error was
thrown. " +
+ "Expected: " +
UnauthorizedException.class.getName() + ", got: " + e.getClass().getName() + ".
" +
+ "Query is: " + query);
+ }
+ if (errorMessage != null)
+ {
+ assertMessageContains(errorMessage, e);
+ }
+ }
+ }
+}
diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java
b/test/unit/org/apache/cassandra/cql3/CQLTester.java
index 48fddec68a..d568d13e11 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -1561,7 +1561,7 @@ public abstract class CQLTester
* @param text the text that the exception message must contains
* @param e the exception to check
*/
- private static void assertMessageContains(String text, Exception e)
+ protected static void assertMessageContains(String text, Exception e)
{
Assert.assertTrue("Expected error message to contain '" + text + "',
but got '" + e.getMessage() + "'",
e.getMessage().contains(text));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
