From 91b86487fe5389765711b3266097fb3faa84e5dd Mon Sep 17 00:00:00 2001 From: Josh McKenzie Date: Thu, 21 Jul 2022 14:59:36 -0400 Subject: [PATCH] Add guardrail to disallow DROP KEYSPACE commands for non superuser accounts Patch by Josh McKenzie; reviewed by Aleksey Yeschenko for CASSANDRA-17767 --- CHANGES.txt | 1 + NEWS.txt | 1 + conf/cassandra.yaml | 3 + .../org/apache/cassandra/config/Config.java | 1 + .../cassandra/config/GuardrailsOptions.java | 14 ++++ .../schema/DropKeyspaceStatement.java | 3 + .../cassandra/db/guardrails/Guardrails.java | 23 ++++++ .../db/guardrails/GuardrailsConfig.java | 7 ++ .../db/guardrails/GuardrailsMBean.java | 12 +++ .../guardrails/GuardrailDropKeyspaceTest.java | 79 +++++++++++++++++++ 10 files changed, 144 insertions(+) create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailDropKeyspaceTest.java diff --git a/CHANGES.txt b/CHANGES.txt index b75eddd30e..49cb5cc679 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.2 + * Add guardrail to disallow DROP KEYSPACE commands (CASSANDRA-17767) * Remove ephemeral snapshot marker file and introduce a flag to SnapshotManifest (CASSANDRA-16911) * Add a virtual table that exposes currently running queries (CASSANDRA-15241) * Allow sstableloader to specify table without relying on path (CASSANDRA-16584) diff --git a/NEWS.txt b/NEWS.txt index 9361b29a5e..87946a9b3c 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -70,6 +70,7 @@ New features - Whether ALTER TABLE commands are allowed to mutate columns - Whether SimpleStrategy is allowed on keyspace creation or alteration - Maximum replication factor + - Whether DROP KEYSPACE commands are allowed. - It is possible to list ephemeral snapshots by nodetool listsnaphots command when flag "-e" is specified. Upgrading diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index a4e5730741..b193a2a699 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1724,6 +1724,9 @@ drop_compact_storage_enabled: false # Guardrail to allow/disallow TRUNCATE and DROP TABLE statements # drop_truncate_table_enabled: true # +# Guardrail to allow/disallow DROP KEYSPACE statements +# drop_keyspace_enabled: true +# # Guardrail to warn or fail when using a page size greater than threshold. # The two thresholds default to -1 to disable. # page_size_warn_threshold: -1 diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index fb407ee202..dc752ffaa7 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -831,6 +831,7 @@ public class Config public volatile boolean alter_table_enabled = true; public volatile boolean group_by_enabled = true; public volatile boolean drop_truncate_table_enabled = true; + public volatile boolean drop_keyspace_enabled = true; public volatile boolean secondary_indexes_enabled = true; public volatile boolean uncompressed_tables_enabled = true; public volatile boolean compact_tables_enabled = true; diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 98d14a1d32..e84e0e2a9f 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -344,6 +344,20 @@ public class GuardrailsOptions implements GuardrailsConfig x -> config.drop_truncate_table_enabled = x); } + @Override + public boolean getDropKeyspaceEnabled() + { + return config.drop_keyspace_enabled; + } + + public void setDropKeyspaceEnabled(boolean enabled) + { + updatePropertyWithLogging("drop_keyspace_enabled", + enabled, + () -> config.drop_keyspace_enabled, + x -> config.drop_keyspace_enabled = x); + } + @Override public boolean getSecondaryIndexesEnabled() { diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/DropKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/DropKeyspaceStatement.java index f2bd30b249..47e514a527 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/DropKeyspaceStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/DropKeyspaceStatement.java @@ -21,6 +21,7 @@ import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; import org.apache.cassandra.auth.Permission; import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.schema.Keyspaces; import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff; import org.apache.cassandra.service.ClientState; @@ -39,6 +40,8 @@ public final class DropKeyspaceStatement extends AlterSchemaStatement public Keyspaces apply(Keyspaces schema) { + Guardrails.dropKeyspaceEnabled.ensureEnabled(state); + if (schema.containsKeyspace(keyspaceName)) return schema.without(keyspaceName); diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 36bb3d446f..1381655955 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -153,11 +153,22 @@ public final class Guardrails implements GuardrailsMBean state -> CONFIG_PROVIDER.getOrCreate(state).getAlterTableEnabled(), "User access to ALTER TABLE statement for column mutation"); + /** + * Guardrail disabling DROP / TRUNCATE TABLE behavior + */ public static final EnableFlag dropTruncateTableEnabled = new EnableFlag("drop_truncate_table_enabled", state -> CONFIG_PROVIDER.getOrCreate(state).getDropTruncateTableEnabled(), "DROP and TRUNCATE TABLE functionality"); + /** + * Guardrail disabling DROP KEYSPACE behavior + */ + public static final EnableFlag dropKeyspaceEnabled = + new EnableFlag("drop_keyspace_enabled", + state -> CONFIG_PROVIDER.getOrCreate(state).getDropKeyspaceEnabled(), + "DROP KEYSPACE functionality"); + /** * Guardrail disabling user's ability to turn off compression */ @@ -648,6 +659,18 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setDropTruncateTableEnabled(enabled); } + @Override + public boolean getDropKeyspaceEnabled() + { + return DEFAULT_CONFIG.getDropKeyspaceEnabled(); + } + + @Override + public void setDropKeyspaceEnabled(boolean enabled) + { + DEFAULT_CONFIG.setDropKeyspaceEnabled(enabled); + } + @Override public int getPageSizeWarnThreshold() { diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index c7067b53e0..d21b899241 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -167,6 +167,13 @@ public interface GuardrailsConfig */ boolean getDropTruncateTableEnabled(); + /** + * Returns whether DROP on keyspaces is allowed + * + * @return {@code true} if allowed, {@code false} otherwise. + */ + boolean getDropKeyspaceEnabled(); + /** * @return The threshold to warn when page size exceeds given size. */ diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index dc3fb48e22..e410d5c16f 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -304,6 +304,18 @@ public interface GuardrailsMBean */ void setDropTruncateTableEnabled(boolean enabled); + /** + * Returns whether users can DROP a keyspace + * + * @return {@code true} if allowed, {@code false} otherwise. + */ + boolean getDropKeyspaceEnabled(); + + /** + * Sets whether users can DROP a keyspace + */ + void setDropKeyspaceEnabled(boolean enabled); + /** * @return The threshold to warn when requested page size greater than threshold. * -1 means disabled. diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailDropKeyspaceTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailDropKeyspaceTest.java new file mode 100644 index 0000000000..de447259fe --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailDropKeyspaceTest.java @@ -0,0 +1,79 @@ +/* + * 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.db.guardrails; + +import org.junit.After; +import org.junit.Test; + +public class GuardrailDropKeyspaceTest extends GuardrailTester +{ + private String keyspaceQuery = "CREATE KEYSPACE dkdt WITH replication = {'class': 'SimpleStrategy', 'replication_factor' : 1}"; + + private void setGuardrail(boolean enabled) + { + Guardrails.instance.setDropKeyspaceEnabled(enabled); + } + + public GuardrailDropKeyspaceTest() + { + super(Guardrails.dropKeyspaceEnabled); + } + + @After + public void afterTest() throws Throwable + { + setGuardrail(true); + execute("DROP KEYSPACE IF EXISTS dkdt"); + } + + @Test + public void testCanDropWhileFeatureEnabled() throws Throwable + { + setGuardrail(true); + createKeyspace(keyspaceQuery); + execute("DROP KEYSPACE dkdt"); + } + + @Test + public void testCannotDropWhileFeatureDisabled() throws Throwable + { + setGuardrail(false); + createKeyspace(keyspaceQuery); + assertFails("DROP KEYSPACE dkdt", "DROP KEYSPACE functionality is not allowed"); + } + + @Test + public void testIfExistsDoesNotBypassCheck() throws Throwable + { + setGuardrail(false); + createKeyspace(keyspaceQuery); + assertFails("DROP KEYSPACE IF EXISTS dkdt", "DROP KEYSPACE functionality is not allowed"); + } + + @Test + public void testToggle() throws Throwable + { + setGuardrail(false); + createKeyspace(keyspaceQuery); + assertFails("DROP KEYSPACE IF EXISTS dkdt", "DROP KEYSPACE functionality is not allowed"); + + setGuardrail(true); + execute("DROP KEYSPACE dkdt"); + } +}