diff --git a/CHANGES.txt b/CHANGES.txt index c65b3e80708e..787b6f83aa8d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Schema annotations escape validation on CREATE and ALTER DDL statements (CASSANDRA-21046) * Calculate once and cache the result of ModificationStatement#requiresRead as a perf optimization (CASSANDRA-21040) * Update system schema tables with new distributed keyspace on upgrade (CASSANDRA-20872) * Fix issue when running cms reconfiguration with paxos repair disabled (CASSANDRA-20869) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java index 9651c92a49bb..4bc552ec4a10 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java @@ -234,6 +234,9 @@ public Keyspaces apply(ClusterMetadata metadata) public void validate(ClientState state) { super.validate(state); + // validate attributes to avoid silently accepting following statements + // create table ... like ... with security_label='xxx'; + attrs.validate(); // If a memtable configuration is specified, validate it against config if (attrs.hasOption(TableParams.Option.MEMTABLE)) diff --git a/src/java/org/apache/cassandra/schema/KeyspaceParams.java b/src/java/org/apache/cassandra/schema/KeyspaceParams.java index 35b68bbf3f66..128d2f88ff6e 100644 --- a/src/java/org/apache/cassandra/schema/KeyspaceParams.java +++ b/src/java/org/apache/cassandra/schema/KeyspaceParams.java @@ -59,9 +59,7 @@ public enum Option { DURABLE_WRITES, REPLICATION, - FAST_PATH, - COMMENT, - SECURITY_LABEL; + FAST_PATH; @Override public String toString() @@ -181,8 +179,8 @@ public String toString() .add(Option.DURABLE_WRITES.toString(), durableWrites) .add(Option.REPLICATION.toString(), replication) .add(Option.FAST_PATH.toString(), fastPath.toString()) - .add(Option.COMMENT.toString(), comment) - .add(Option.SECURITY_LABEL.toString(), securityLabel) + .add("COMMENT", comment) + .add("SECURITY_LABEL", securityLabel) .toString(); } diff --git a/src/java/org/apache/cassandra/schema/TableParams.java b/src/java/org/apache/cassandra/schema/TableParams.java index 7f27e47b5aa8..e5f51859a43b 100644 --- a/src/java/org/apache/cassandra/schema/TableParams.java +++ b/src/java/org/apache/cassandra/schema/TableParams.java @@ -70,7 +70,6 @@ import static org.apache.cassandra.schema.TableParams.Option.MIN_INDEX_INTERVAL; import static org.apache.cassandra.schema.TableParams.Option.PENDING_DROP; import static org.apache.cassandra.schema.TableParams.Option.READ_REPAIR; -import static org.apache.cassandra.schema.TableParams.Option.SECURITY_LABEL; import static org.apache.cassandra.schema.TableParams.Option.SPECULATIVE_RETRY; import static org.apache.cassandra.utils.LocalizeString.toLowerCaseLocalized; @@ -102,8 +101,7 @@ public enum Option TRANSACTIONAL_MODE, TRANSACTIONAL_MIGRATION_FROM, PENDING_DROP, - AUTO_REPAIR, - SECURITY_LABEL; + AUTO_REPAIR; @Override public String toString() @@ -342,7 +340,7 @@ public String toString() { return MoreObjects.toStringHelper(this) .add(COMMENT.toString(), comment) - .add(SECURITY_LABEL.toString(), securityLabel) + .add("SECURITY_LABEL", securityLabel) .add(ADDITIONAL_WRITE_POLICY.toString(), additionalWritePolicy) .add(ALLOW_AUTO_SNAPSHOT.toString(), allowAutoSnapshot) .add(BLOOM_FILTER_FP_CHANCE.toString(), bloomFilterFpChance) diff --git a/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CommentAndSecurityLabelTest.java b/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CommentAndSecurityLabelTest.java index fe404afa9dcd..1c25af2390a0 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CommentAndSecurityLabelTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CommentAndSecurityLabelTest.java @@ -373,6 +373,79 @@ public void testEmptyStringRejection() assertInvalidMessage("Cannot set security label to empty string", buildSecurityLabelStatement(ObjectType.FIELD, fieldRef, "")); } + @Test + public void testCommentAndSecurityLabelNotAllowedInCreateKeyspace() + { + // Test that comment property is rejected in CREATE KEYSPACE WITH clause + String createKsWithComment = "CREATE KEYSPACE ks_test WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'} AND comment = 'test comment'"; + assertInvalidMessage("Unknown property 'comment'", createKsWithComment); + + // Test that security_label property is rejected in CREATE KEYSPACE WITH clause + String createKsWithLabel = "CREATE KEYSPACE ks_test2 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'} AND security_label = 'TEST_LABEL'"; + assertInvalidMessage("Unknown property 'security_label'", createKsWithLabel); + } + + @Test + public void testCommentAndSecurityLabelNotAllowedInAlterKeyspace() + { + createKeyspaceWithName("ks_alter_test"); + + // Test that comment property is rejected in ALTER KEYSPACE WITH clause + String alterKsWithComment = "ALTER KEYSPACE ks_alter_test WITH comment = 'test comment'"; + assertInvalidMessage("Unknown property 'comment'", alterKsWithComment); + + // Test that security_label property is rejected in ALTER KEYSPACE WITH clause + String alterKsWithLabel = "ALTER KEYSPACE ks_alter_test WITH security_label = 'TEST_LABEL'"; + assertInvalidMessage("Unknown property 'security_label'", alterKsWithLabel); + } + + @Test + public void testSecurityLabelNotAllowedInCreateTable() + { + createKeyspaceWithName("ks_table_test"); + + // Test that security_label property is rejected in CREATE TABLE WITH clause + String createTableWithLabel = "CREATE TABLE ks_table_test.t1 (id int PRIMARY KEY, name text) WITH security_label = 'TEST_LABEL'"; + assertInvalidMessage("Unknown property 'security_label'", createTableWithLabel); + + // Verify that comment IS allowed in CREATE TABLE for backward compatibility + String createTableWithComment = "CREATE TABLE ks_table_test.t2 (id int PRIMARY KEY, name text) WITH comment = 'test comment'"; + execute(createTableWithComment); + assertComment(ObjectType.TABLE, "ks_table_test", "ks_table_test.t2", "test comment"); + } + + @Test + public void testSecurityLabelNotAllowedInAlterTable() + { + createKeyspaceWithName("ks_alter_table_test"); + createTableWithName("ks_alter_table_test", "t1"); + + // Test that security_label property is rejected in ALTER TABLE WITH clause + String alterTableWithLabel = "ALTER TABLE ks_alter_table_test.t1 WITH security_label = 'TEST_LABEL'"; + assertInvalidMessage("Unknown property 'security_label'", alterTableWithLabel); + + // Verify that comment IS allowed in ALTER TABLE for backward compatibility + String alterTableWithComment = "ALTER TABLE ks_alter_table_test.t1 WITH comment = 'test comment'"; + execute(alterTableWithComment); + assertComment(ObjectType.TABLE, "ks_alter_table_test", "ks_alter_table_test.t1", "test comment"); + } + + @Test + public void testSecurityLabelNotAllowedInCreateTableLike() + { + createKeyspaceWithName("ks_like_test"); + createTableWithName("ks_like_test", "source_table"); + + // Test that security_label property is rejected in CREATE TABLE ... LIKE ... WITH clause + String createTableLikeWithLabel = "CREATE TABLE ks_like_test.target_table LIKE ks_like_test.source_table WITH security_label = 'TEST_LABEL'"; + assertInvalidMessage("Unknown property 'security_label'", createTableLikeWithLabel); + + // Verify that comment IS allowed in CREATE TABLE ... LIKE for backward compatibility + String createTableLikeWithComment = "CREATE TABLE ks_like_test.target_table2 LIKE ks_like_test.source_table WITH comment = 'test comment'"; + execute(createTableLikeWithComment); + assertComment(ObjectType.TABLE, "ks_like_test", "ks_like_test.target_table2", "test comment"); + } + // Helper methods for setting comments and security labels private void setComment(ObjectType type, String objectName, String comment) {