From f444c4028680c78b6167161833d6564c3557618f Mon Sep 17 00:00:00 2001 From: Savni Nagarkar Date: Thu, 17 Feb 2022 13:29:58 -0600 Subject: [PATCH] Add guardrail to disallow querying with ALLOW FILTERING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Savni Nagarkar; reviewed by Andres de la Peña, David Capwell and Josh McKenzie for CASSANDRA-17370 --- CHANGES.txt | 1 + NEWS.txt | 1 + conf/cassandra.yaml | 2 + .../org/apache/cassandra/config/Config.java | 1 + .../cassandra/config/GuardrailsOptions.java | 14 +++ .../cql3/statements/SelectStatement.java | 4 +- .../cassandra/db/guardrails/Guardrails.java | 20 ++++ .../db/guardrails/GuardrailsConfig.java | 7 ++ .../db/guardrails/GuardrailsMBean.java | 14 +++ .../GuardrailAllowFilteringTest.java | 94 +++++++++++++++++++ 10 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 9410f075bc..4ae12a7c97 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add guardrail to disallow querying with ALLOW FILTERING (CASSANDRA-17370) * Enhance SnakeYAML properties to be reusable outside of YAML parsing, support camel case conversion to snake case, and add support to ignore properties (CASSANDRA-17166) * nodetool compact should support using a key string to find the range to avoid operators having to manually do this (CASSANDRA-17537) * Add guardrail for data disk usage (CASSANDRA-17150) diff --git a/NEWS.txt b/NEWS.txt index 72c95e00c4..97bfd331c4 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -84,6 +84,7 @@ New features - Whether GROUP BY queries are allowed. - Whether the creation of secondary indexes is allowed. - Whether the creation of uncompressed tables is allowed. + - Whether querying with ALLOW FILTERING is allowed. - Add support for the use of pure monotonic functions on the last attribute of the GROUP BY clause. - Add floor functions that can be use to group by time range. - Support for native transport rate limiting via native_transport_rate_limiting_enabled and diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index b28f4388f5..f6f0a3ccad 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1664,6 +1664,8 @@ drop_compact_storage_enabled: false # The two thresholds default to -1 to disable. # items_per_collection_warn_threshold: -1 # items_per_collection_fail_threshold: -1 +# Guardrail to allow/disallow querying with ALLOW FILTERING. Defaults to true. +# allow_filtering_enabled: true # Guardrail to warn or fail when creating a user-defined-type with more fields in than threshold. # Default -1 to disable. # fields_per_udt_warn_threshold: -1 diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 2f339418b1..a3d6348fa1 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -812,6 +812,7 @@ public class Config public volatile boolean uncompressed_tables_enabled = true; public volatile boolean compact_tables_enabled = true; public volatile boolean read_before_write_list_operations_enabled = true; + public volatile boolean allow_filtering_enabled = true; public volatile DataStorageSpec collection_size_warn_threshold = null; public volatile DataStorageSpec collection_size_fail_threshold = null; public volatile int items_per_collection_warn_threshold = -1; diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 160906353e..d32f3b1bc6 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -385,6 +385,20 @@ public class GuardrailsOptions implements GuardrailsConfig x -> config.read_before_write_list_operations_enabled = x); } + @Override + public boolean getAllowFilteringEnabled() + { + return config.allow_filtering_enabled; + } + + public void setAllowFilteringEnabled(boolean enabled) + { + updatePropertyWithLogging("allow_filtering_enabled", + enabled, + () -> config.allow_filtering_enabled, + x -> config.allow_filtering_enabled = x); + } + @Override public int getInSelectCartesianProductWarnThreshold() { diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index b27266ce7e..030b4cd785 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -35,6 +35,7 @@ import org.apache.cassandra.auth.Permission; import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.Schema; +import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.schema.TableMetadataRef; import org.apache.cassandra.cql3.*; @@ -238,7 +239,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement public void validate(ClientState state) throws InvalidRequestException { - // Nothing to do, all validation has been done by RawStatement.prepare() + if (parameters.allowFiltering && !SchemaConstants.isSystemKeyspace(table.keyspace)) + Guardrails.allowFilteringEnabled.ensureEnabled(state); } public ResultMessage.Rows execute(QueryState state, QueryOptions options, long queryStartNanoTime) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index d7ec81e83b..8451aff8f3 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -197,6 +197,14 @@ public final class Guardrails implements GuardrailsMBean state -> !CONFIG_PROVIDER.getOrCreate(state).getReadBeforeWriteListOperationsEnabled(), "List operation requiring read before write"); + /** + * Guardrail disabling ALLOW FILTERING statement within a query + */ + public static final DisableFlag allowFilteringEnabled = + new DisableFlag("allow_filtering", + state -> !CONFIG_PROVIDER.getOrCreate(state).getAllowFilteringEnabled(), + "Querying with ALLOW FILTERING"); + /** * Guardrail on the number of restrictions created by a cartesian product of a CQL's {@code IN} query. */ @@ -513,6 +521,18 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setUserTimestampsEnabled(enabled); } + @Override + public boolean getAllowFilteringEnabled() + { + return DEFAULT_CONFIG.getAllowFilteringEnabled(); + } + + @Override + public void setAllowFilteringEnabled(boolean enabled) + { + DEFAULT_CONFIG.setAllowFilteringEnabled(enabled); + } + @Override public boolean getUncompressedTablesEnabled() { diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 562509b7dd..6dbc0bb91e 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -170,6 +170,13 @@ public interface GuardrailsConfig */ boolean getReadBeforeWriteListOperationsEnabled(); + /** + * Returns whether ALLOW FILTERING property is allowed. + * + * @return {@code true} if ALLOW FILTERING is allowed, {@code false} otherwise. + */ + boolean getAllowFilteringEnabled(); + /** * @return The threshold to warn when an IN query creates a cartesian product with a size exceeding threshold. * -1 means disabled. diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 215b953b89..3d94410f4e 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -208,6 +208,20 @@ public interface GuardrailsMBean */ void setUserTimestampsEnabled(boolean enabled); + /** + * Returns whether ALLOW FILTERING property is allowed. + * + * @return {@code true} if ALLOW FILTERING is allowed, {@code false} otherwise. + */ + boolean getAllowFilteringEnabled(); + + /** + * Sets whether ALLOW FILTERING is allowed. + * + * @param enabled {@code true} if ALLOW FILTERING is allowed, {@code false} otherwise. + */ + void setAllowFilteringEnabled(boolean enabled); + /** * Returns whether users can disable compression on tables * diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java new file mode 100644 index 0000000000..c46498c01f --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java @@ -0,0 +1,94 @@ +/* + * 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.Before; +import org.junit.Test; + +import org.apache.cassandra.schema.SchemaConstants; +import org.apache.cassandra.schema.SchemaKeyspaceTables; + +public class GuardrailAllowFilteringTest extends GuardrailTester +{ + private boolean enableState; + + @Before + public void setupTest() + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, a int, b int)"); + enableState = getGuardrial(); + } + + @After + public void teardownTest() + { + setGuardrail(enableState); + } + + private void setGuardrail(boolean allowFilteringEnabled) + { + guardrails().setAllowFilteringEnabled(allowFilteringEnabled); + } + + private boolean getGuardrial() + { + return guardrails().getAllowFilteringEnabled(); + } + + @Test + public void testAllowFilteringDisabled() throws Throwable + { + setGuardrail(false); + assertFails("SELECT * FROM %s WHERE a = 5 ALLOW FILTERING", "Querying with ALLOW FILTERING is not allowed"); + } + + @Test + public void testAllowFilteringDisabedNotUsed() throws Throwable + { + setGuardrail(false); + execute("INSERT INTO %s (k, a, b) VALUES (1, 1, 1)"); + assertValid("SELECT * FROM %s"); + } + + @Test + public void testAllowFilteringEnabled() throws Throwable + { + setGuardrail(true); + execute("INSERT INTO %s (k, a, b) VALUES (1, 1, 1)"); + assertValid("SELECT * FROM %s WHERE a = 5 ALLOW FILTERING"); + } + + @Test + public void testExcludedUsers() throws Throwable + { + setGuardrail(false); + testExcludedUsers(() -> "SELECT * FROM %s WHERE a = 5 ALLOW FILTERING"); + } + + @Test + public void testSystemTable() throws Throwable + { + setGuardrail(false); + assertValid(String.format("SELECT * FROM %s.%s WHERE table_name = '%s' ALLOW FILTERING", + SchemaConstants.SCHEMA_KEYSPACE_NAME, + SchemaKeyspaceTables.TABLES, + currentTable())); + } +}