From 85248da628770d9d93fdd2cbd1eedd55b3ddc206 Mon Sep 17 00:00:00 2001 From: Bereng Date: Thu, 28 Oct 2021 09:44:32 +0200 Subject: [PATCH] PasswordObfuscator should not assume PASSWORD is the last item in the WITH clause patch by Berenguer Blasi; reviewed by Benjamin Lerer, Ekaterina Dimitrova for CASSANDRA-16801 --- CHANGES.txt | 1 + NEWS.txt | 9 +- .../cassandra/pages/new/auditlogging.adoc | 13 +- .../cassandra/audit/AuditLogManager.java | 33 +++- .../cassandra/cql3/PasswordObfuscator.java | 41 ++++- .../apache/cassandra/cql3/QueryEvents.java | 42 +++-- .../cql3/statements/AlterRoleStatement.java | 7 + .../statements/AuthenticationStatement.java | 5 + .../cql3/statements/CreateRoleStatement.java | 7 + .../cassandra/audit/AuditLoggerAuthTest.java | 71 ++++++-- .../cql3/PasswordObfuscatorTest.java | 154 +++++++++++++----- 11 files changed, 295 insertions(+), 88 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 2bb5f301a5..cf4c20644e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -101,6 +101,7 @@ Merged from 3.11: Merged from 3.0: 4.0-rc2 + * Improved password obfuscation (CASSANDRA-16801) * Avoid memoizing the wrong min cluster version during upgrades (CASSANDRA-16759) * Obfuscate passwords in statements in QueryEvents (CASSANDRA-16669) * Fix queries on empty partitions with static data (CASSANDRA-16686) diff --git a/NEWS.txt b/NEWS.txt index 649f3fda46..749b5317fc 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -37,6 +37,11 @@ using the provided 'sstableupgrade' tool. 4.0.2 ===== +New features +------------ + - Full support for Java 11, it is not experimental anymore. + - DCL statements in audit logs will now obscure only the password if they don't fail to parse. + Upgrading --------- - Before you upgrade, if you are using `cassandra.auth_bcrypt_gensalt_log2_rounds` property, @@ -49,10 +54,6 @@ Upgrading currently both old and new names should work. Cassandra 4.0.0 and Cassandra 4.0.1 work ONLY with the new names (They weren't updated in cassandra.yaml though). -New Features ------------- - - Full support for Java 11, it is not experimental anymore. - 4.0 === diff --git a/doc/modules/cassandra/pages/new/auditlogging.adoc b/doc/modules/cassandra/pages/new/auditlogging.adoc index e81776a097..a47992192f 100644 --- a/doc/modules/cassandra/pages/new/auditlogging.adoc +++ b/doc/modules/cassandra/pages/new/auditlogging.adoc @@ -32,6 +32,9 @@ The audit log does not contain: * configuration changes made in `cassandra.yaml` file * `nodetool` commands +* Passwords mentioned as part of DCL statements: Passwords will be obfuscated as \*\*\*\*\*\*\*. + ** Statements that fail to parse will have everything after the appearance of the word password obfuscated as \*\*\*\*\*\*\*. + ** Statements with a mistyped word 'password' will be logged without obfuscation. Please make sure to use a different password on retries. The audit log is a series of log entries. An audit log entry contains: @@ -455,7 +458,15 @@ WITH replication = {'class': 'SimpleStrategy', 'replication_factor' : 1}; Type: AuditLog LogMessage: user:anonymous|host:10.0.2.238:7000|source:/127.0.0.1|port:46264|timestamp:1564714870678|type :USE_KEYSPACE|category:OTHER|ks:auditlogkeyspace|operation:USE auditlogkeyspace; -[ec2-user@ip-10-0-2-238 hourly]$ + +Password obfuscation examples: +LogMessage: user:cassandra|host:localhost/127.0.0.1:7000|source:/127.0.0.1|port:65282|timestamp:1622630496708|type:CREATE_ROLE|category:DCL|operation:CREATE ROLE role1 WITH PASSWORD = '*******'; +Type: audit +LogMessage: user:cassandra|host:localhost/127.0.0.1:7000|source:/127.0.0.1|port:65282|timestamp:1622630634552|type:ALTER_ROLE|category:DCL|operation:ATLER ROLE role1 WITH PASSWORD = '*******'; +Type: audit +LogMessage: user:cassandra|host:localhost/127.0.0.1:7000|source:/127.0.0.1|port:65282|timestamp:1622630698686|type:CREATE_ROLE|category:DCL|operation:CREATE USER user1 WITH PASSWORD '*******'; +Type: audit +LogMessage: user:cassandra|host:localhost/127.0.0.1:7000|source:/127.0.0.1|port:65282|timestamp:1622630747344|type:ALTER_ROLE|category:DCL|operation:ALTER USER user1 WITH PASSWORD '*******'; ---- == Diagnostic events for user audit logging diff --git a/src/java/org/apache/cassandra/audit/AuditLogManager.java b/src/java/org/apache/cassandra/audit/AuditLogManager.java index 3168b3b4da..88e025109b 100644 --- a/src/java/org/apache/cassandra/audit/AuditLogManager.java +++ b/src/java/org/apache/cassandra/audit/AuditLogManager.java @@ -22,12 +22,13 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.UUID; import javax.annotation.Nullable; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,12 +36,14 @@ import org.apache.cassandra.auth.AuthEvents; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.ParameterizedClass; import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.PasswordObfuscator; import org.apache.cassandra.cql3.QueryEvents; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.statements.BatchStatement; import org.apache.cassandra.exceptions.AuthenticationException; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.exceptions.PreparedQueryNotFoundException; +import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.exceptions.UnauthorizedException; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.transport.Message; @@ -119,6 +122,11 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene } private void log(AuditLogEntry logEntry, Exception e) + { + log(logEntry, e, null); + } + + private void log(AuditLogEntry logEntry, Exception e, List queries) { AuditLogEntry.Builder builder = new AuditLogEntry.Builder(logEntry); @@ -135,7 +143,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene builder.setType(AuditLogEntryType.REQUEST_FAILURE); } - builder.appendToOperation(QueryEvents.instance.getObfuscator().obfuscate(e.getMessage())); + builder.appendToOperation(obfuscatePasswordInformation(e, queries)); log(builder.build()); } @@ -206,7 +214,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene AuditLogEntry entry = new AuditLogEntry.Builder(state).setOperation(query) .setOptions(options) .build(); - log(entry, cause); + log(entry, cause, query == null ? null : ImmutableList.of(query)); } public void executeSuccess(CQLStatement statement, String query, QueryOptions options, QueryState state, long queryTime, Message.Response response) @@ -240,7 +248,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene .build(); } if (entry != null) - log(entry, cause); + log(entry, cause, query == null ? null : ImmutableList.of(query)); } public void batchSuccess(BatchStatement.Type batchType, List statements, List queries, List> values, QueryOptions options, QueryState state, long queryTime, Message.Response response) @@ -259,7 +267,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene .setOptions(options) .setType(AuditLogEntryType.BATCH) .build(); - log(entry, cause); + log(entry, cause, queries); } private static List buildEntriesForBatch(List statements, List queries, QueryState state, QueryOptions options, long queryStartTimeMillis) @@ -328,4 +336,19 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene .build(); log(entry, cause); } + + private String obfuscatePasswordInformation(Exception e, List queries) + { + // A syntax error may reveal the password in the form of 'line 1:33 mismatched input 'secret_password'' + if (e instanceof SyntaxException && queries != null && !queries.isEmpty()) + { + for (String query : queries) + { + if (query.toLowerCase().contains(PasswordObfuscator.PASSWORD_TOKEN)) + return "Syntax Exception. Obscured for security reasons."; + } + } + + return PasswordObfuscator.obfuscate(e.getMessage()); + } } diff --git a/src/java/org/apache/cassandra/cql3/PasswordObfuscator.java b/src/java/org/apache/cassandra/cql3/PasswordObfuscator.java index 97ae2e466c..89962f90d7 100644 --- a/src/java/org/apache/cassandra/cql3/PasswordObfuscator.java +++ b/src/java/org/apache/cassandra/cql3/PasswordObfuscator.java @@ -18,15 +18,26 @@ package org.apache.cassandra.cql3; +import com.google.common.base.Optional; + +import org.apache.cassandra.auth.PasswordAuthenticator; +import org.apache.cassandra.auth.RoleOptions; + /** * Obfuscates passwords in a given string */ public class PasswordObfuscator { - public static final String OBFUSCATION_TOKEN = " *******"; - private static final String PASSWORD_TOKEN = "password"; + public static final String OBFUSCATION_TOKEN = "*******"; + public static final String PASSWORD_TOKEN = PasswordAuthenticator.PASSWORD_KEY.toLowerCase(); - public String obfuscate(String sourceString) + /** + * Obfuscates everything after the first appearance password token + * + * @param sourceString The query to obfuscate + * @return The obfuscated query + */ + public static String obfuscate(String sourceString) { if (null == sourceString) return null; @@ -35,6 +46,26 @@ public class PasswordObfuscator if (passwordTokenStartIndex < 0) return sourceString; - return sourceString.substring(0, passwordTokenStartIndex + PASSWORD_TOKEN.length()) + OBFUSCATION_TOKEN; + return sourceString.substring(0, passwordTokenStartIndex + PASSWORD_TOKEN.length()) + " " + OBFUSCATION_TOKEN; } -} \ No newline at end of file + + /** + * Obfuscates the password in a query + * + * @param query The query whose password to obfuscate + * @param opts The options containing the password to obfuscate + * @return The query with obfuscated password + */ + public static String obfuscate(String query, RoleOptions opts) + { + if (opts == null || query == null || query.isEmpty()) + return query; + + Optional pass = opts.getPassword(); + if (!pass.isPresent() || pass.get().isEmpty()) + return query; + + // match new line, case insensitive (?si), and PASSWORD_TOKEN up to the actual password greedy. Group that and replace the password + return query.replaceAll("((?si)"+ PASSWORD_TOKEN + ".+?)" + pass.get(), "$1" + PasswordObfuscator.OBFUSCATION_TOKEN); + } +} diff --git a/src/java/org/apache/cassandra/cql3/QueryEvents.java b/src/java/org/apache/cassandra/cql3/QueryEvents.java index ffec3ffb4b..52414ebb74 100644 --- a/src/java/org/apache/cassandra/cql3/QueryEvents.java +++ b/src/java/org/apache/cassandra/cql3/QueryEvents.java @@ -48,20 +48,12 @@ public class QueryEvents private final Set listeners = new CopyOnWriteArraySet<>(); - private final PasswordObfuscator passwordObfuscator = new PasswordObfuscator(); - @VisibleForTesting public int listenerCount() { return listeners.size(); } - @VisibleForTesting - public PasswordObfuscator getObfuscator() - { - return passwordObfuscator; - } - public void registerListener(Listener listener) { listeners.add(listener); @@ -81,9 +73,9 @@ public class QueryEvents { try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(statement, query) : query; for (Listener listener : listeners) - listener.querySuccess(statement, possiblyObfuscatedQuery, options, state, queryTime, response); + listener.querySuccess(statement, maybeObfuscatedQuery, options, state, queryTime, response); } catch (Throwable t) { @@ -100,9 +92,9 @@ public class QueryEvents { try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(statement, query) : query; for (Listener listener : listeners) - listener.queryFailure(statement, possiblyObfuscatedQuery, options, state, cause); + listener.queryFailure(statement, maybeObfuscatedQuery, options, state, cause); } catch (Throwable t) { @@ -120,9 +112,9 @@ public class QueryEvents { try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(statement, query) : query; for (Listener listener : listeners) - listener.executeSuccess(statement, possiblyObfuscatedQuery, options, state, queryTime, response); + listener.executeSuccess(statement, maybeObfuscatedQuery, options, state, queryTime, response); } catch (Throwable t) { @@ -140,9 +132,9 @@ public class QueryEvents String query = prepared != null ? prepared.rawCQLStatement : null; try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(statement, query) : query; for (Listener listener : listeners) - listener.executeFailure(statement, possiblyObfuscatedQuery, options, state, cause); + listener.executeFailure(statement, maybeObfuscatedQuery, options, state, cause); } catch (Throwable t) { @@ -217,9 +209,9 @@ public class QueryEvents { try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(prepared.statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(prepared.statement, query) : query; for (Listener listener : listeners) - listener.prepareSuccess(prepared.statement, possiblyObfuscatedQuery, state, queryTime, response); + listener.prepareSuccess(prepared.statement, maybeObfuscatedQuery, state, queryTime, response); } catch (Throwable t) { @@ -239,9 +231,9 @@ public class QueryEvents { try { - final String possiblyObfuscatedQuery = listeners.size() > 0 ? possiblyObfuscateQuery(statement, query) : query; + final String maybeObfuscatedQuery = listeners.size() > 0 ? maybeObfuscatePassword(statement, query) : query; for (Listener listener : listeners) - listener.prepareFailure(statement, possiblyObfuscatedQuery, state, cause); + listener.prepareFailure(statement, maybeObfuscatedQuery, state, cause); } catch (Throwable t) { @@ -250,10 +242,16 @@ public class QueryEvents } } - private String possiblyObfuscateQuery(CQLStatement statement, String query) + private String maybeObfuscatePassword(CQLStatement statement, String query) { // Statement might be null as side-effect of failed parsing, originates from QueryMessage#execute - return null == statement || statement instanceof AuthenticationStatement ? passwordObfuscator.obfuscate(query) : query; + if (statement == null) + return PasswordObfuscator.obfuscate(query); + + if (statement instanceof AuthenticationStatement) + return ((AuthenticationStatement) statement).obfuscatePassword(query); + + return query; } public boolean hasListeners() diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java index 7a748e8aaf..2ffd050d66 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java @@ -22,6 +22,7 @@ import org.apache.cassandra.audit.AuditLogEntryType; import org.apache.cassandra.auth.*; import org.apache.cassandra.auth.IRoleManager.Option; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.cql3.PasswordObfuscator; import org.apache.cassandra.cql3.RoleName; import org.apache.cassandra.exceptions.*; import org.apache.cassandra.service.ClientState; @@ -117,4 +118,10 @@ public class AlterRoleStatement extends AuthenticationStatement { return new AuditLogContext(AuditLogEntryType.ALTER_ROLE); } + + @Override + public String obfuscatePassword(String query) + { + return PasswordObfuscator.obfuscate(query, opts); + } } diff --git a/src/java/org/apache/cassandra/cql3/statements/AuthenticationStatement.java b/src/java/org/apache/cassandra/cql3/statements/AuthenticationStatement.java index a8cbaa7197..db3aa99273 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AuthenticationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AuthenticationStatement.java @@ -63,5 +63,10 @@ public abstract class AuthenticationStatement extends CQLStatement.Raw implement state.getUser().getName())); } } + + public String obfuscatePassword(String query) + { + return query; + } } diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateRoleStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateRoleStatement.java index 8224c4bc90..b3333fc605 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateRoleStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateRoleStatement.java @@ -21,6 +21,7 @@ import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; import org.apache.cassandra.auth.*; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.cql3.PasswordObfuscator; import org.apache.cassandra.cql3.RoleName; import org.apache.cassandra.exceptions.*; import org.apache.cassandra.service.ClientState; @@ -125,4 +126,10 @@ public class CreateRoleStatement extends AuthenticationStatement { return new AuditLogContext(AuditLogEntryType.CREATE_ROLE); } + + @Override + public String obfuscatePassword(String query) + { + return PasswordObfuscator.obfuscate(query, opts); + } } diff --git a/test/unit/org/apache/cassandra/audit/AuditLoggerAuthTest.java b/test/unit/org/apache/cassandra/audit/AuditLoggerAuthTest.java index a873b6ed95..963a473e42 100644 --- a/test/unit/org/apache/cassandra/audit/AuditLoggerAuthTest.java +++ b/test/unit/org/apache/cassandra/audit/AuditLoggerAuthTest.java @@ -41,6 +41,7 @@ import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.PasswordObfuscator; import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.service.EmbeddedCassandraService; +import org.hamcrest.CoreMatchers; import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; @@ -132,13 +133,13 @@ public class AuditLoggerAuthTest { String createTestRoleCQL = String.format("CREATE ROLE %s WITH LOGIN = %s ANDSUPERUSER = %s AND PASSWORD", TEST_ROLE, true, false) + CASS_PW; - String createTestRoleCQLExpected = String.format("CREATE ROLE %s WITH LOGIN = %s ANDSUPERUSER = %s AND PASSWORD", + String createTestRoleCQLExpected = String.format("CREATE ROLE %s WITH LOGIN = %s ANDSUPERUSER = %s AND PASSWORD ", TEST_ROLE, true, false) + PasswordObfuscator.OBFUSCATION_TOKEN; executeWithCredentials(Arrays.asList(createTestRoleCQL), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.REQUEST_FAILURE, createTestRoleCQLExpected, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.REQUEST_FAILURE, createTestRoleCQLExpected, CASS_USER, TEST_PW); assertEquals(0, getInMemAuditLogger().size()); } @@ -150,7 +151,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.ALTER_ROLE, "ALTER ROLE " + TEST_ROLE + " WITH PASSWORD" + PasswordObfuscator.OBFUSCATION_TOKEN, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.ALTER_ROLE, "ALTER ROLE " + TEST_ROLE + " WITH PASSWORD = '" + PasswordObfuscator.OBFUSCATION_TOKEN + "'", CASS_USER, "foo_bar"); assertEquals(0, getInMemAuditLogger().size()); } @@ -162,7 +163,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.DROP_ROLE, cql, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.DROP_ROLE, cql, CASS_USER, ""); assertEquals(0, getInMemAuditLogger().size()); } @@ -173,7 +174,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.LIST_ROLES, cql, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.LIST_ROLES, cql, CASS_USER, ""); } @Test @@ -183,7 +184,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.LIST_PERMISSIONS, cql, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.LIST_PERMISSIONS, cql, CASS_USER, ""); } @Test @@ -194,7 +195,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.GRANT, cql, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.GRANT, cql, CASS_USER, ""); } @Test @@ -205,7 +206,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.REVOKE, cql, CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.REVOKE, cql, CASS_USER, ""); } @Test @@ -216,10 +217,49 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(cql), TEST_USER, TEST_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.UNAUTHORIZED_ATTEMPT, cql, TEST_USER); + assertLogEntry(logEntry, AuditLogEntryType.UNAUTHORIZED_ATTEMPT, cql, TEST_USER, ""); assertEquals(0, getInMemAuditLogger().size()); } + @Test + public void testCqlUSERCommandsAuditing() + { + //CREATE USER and ALTER USER are supported only for backwards compatibility. + + String user = TEST_ROLE + "user"; + String cql = "CREATE USER " + user + " WITH PASSWORD '" + TEST_PW + "'"; + executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); + assertTrue(getInMemAuditLogger().size() > 0); + AuditLogEntry logEntry = getInMemAuditLogger().poll(); + assertLogEntry(logEntry, + AuditLogEntryType.CREATE_ROLE, + "CREATE USER " + user + " WITH PASSWORD '" + PasswordObfuscator.OBFUSCATION_TOKEN + "'", + CASS_USER, + TEST_PW); + + cql = "ALTER USER " + user + " WITH PASSWORD '" + TEST_PW + "'"; + executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); + assertTrue(getInMemAuditLogger().size() > 0); + logEntry = getInMemAuditLogger().poll(); + assertLogEntry(logEntry, + AuditLogEntryType.ALTER_ROLE, + "ALTER USER " + user + " WITH PASSWORD '" + PasswordObfuscator.OBFUSCATION_TOKEN + "'", + CASS_USER, + TEST_PW); + + cql = "ALTER USER " + user + " WITH PASSWORD " + TEST_PW; + executeWithCredentials(Arrays.asList(cql), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); + assertTrue(getInMemAuditLogger().size() > 0); + logEntry = getInMemAuditLogger().poll(); + assertLogEntry(logEntry, + AuditLogEntryType.REQUEST_FAILURE, + "ALTER USER " + user + + " WITH PASSWORD " + PasswordObfuscator.OBFUSCATION_TOKEN + + "; Syntax Exception. Obscured for security reasons.", + CASS_USER, + TEST_PW); + } + /** * Helper methods */ @@ -274,7 +314,7 @@ public class AuditLoggerAuthTest return ((InMemoryAuditLogger) AuditLogManager.instance.getLogger()).inMemQueue; } - private static void assertLogEntry(AuditLogEntry logEntry, AuditLogEntryType type, String cql, String username) + private static void assertLogEntry(AuditLogEntry logEntry, AuditLogEntryType type, String cql, String username, String forbiddenPassword) { assertSource(logEntry, username); assertNotEquals(0, logEntry.getTimestamp()); @@ -282,6 +322,8 @@ public class AuditLoggerAuthTest if (null != cql && !cql.isEmpty()) { assertThat(logEntry.getOperation(), containsString(cql)); + if (!forbiddenPassword.isEmpty()) + assertThat(logEntry.getOperation(), CoreMatchers.not(containsString(forbiddenPassword))); } } @@ -295,8 +337,11 @@ public class AuditLoggerAuthTest private static String getCreateRoleCql(String role, boolean login, boolean superUser, boolean isPasswordObfuscated) { - String baseQueryString = String.format("CREATE ROLE IF NOT EXISTS %s WITH LOGIN = %s AND SUPERUSER = %s AND PASSWORD", role, login, superUser); - return isPasswordObfuscated ? baseQueryString + PasswordObfuscator.OBFUSCATION_TOKEN : baseQueryString + String.format(" = '%s'", TEST_PW); + return String.format("CREATE ROLE IF NOT EXISTS %s WITH PASSWORD = '%s' AND LOGIN = %s AND SUPERUSER = %s", + role, + isPasswordObfuscated ? PasswordObfuscator.OBFUSCATION_TOKEN : TEST_PW, + login, + superUser); } private static void createTestRole() @@ -305,7 +350,7 @@ public class AuditLoggerAuthTest executeWithCredentials(Arrays.asList(createTestRoleCQL), CASS_USER, CASS_PW, AuditLogEntryType.LOGIN_SUCCESS); assertTrue(getInMemAuditLogger().size() > 0); AuditLogEntry logEntry = getInMemAuditLogger().poll(); - assertLogEntry(logEntry, AuditLogEntryType.CREATE_ROLE, getCreateRoleCql(TEST_ROLE, true, false, true), CASS_USER); + assertLogEntry(logEntry, AuditLogEntryType.CREATE_ROLE, getCreateRoleCql(TEST_ROLE, true, false, true), CASS_USER, ""); assertEquals(0, getInMemAuditLogger().size()); } } \ No newline at end of file diff --git a/test/unit/org/apache/cassandra/cql3/PasswordObfuscatorTest.java b/test/unit/org/apache/cassandra/cql3/PasswordObfuscatorTest.java index 2308850408..09a366e49c 100644 --- a/test/unit/org/apache/cassandra/cql3/PasswordObfuscatorTest.java +++ b/test/unit/org/apache/cassandra/cql3/PasswordObfuscatorTest.java @@ -18,145 +18,223 @@ package org.apache.cassandra.cql3; +import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.auth.RoleOptions; + +import static org.apache.cassandra.cql3.PasswordObfuscator.*; import static java.lang.String.format; import static org.junit.Assert.assertEquals; public class PasswordObfuscatorTest { - private static final PasswordObfuscator obfuscator = new PasswordObfuscator(); + private static final RoleOptions opts = new RoleOptions(); + private static final String optsPassword = "testpassword"; + + @BeforeClass + public static void startup() + { + opts.setOption(org.apache.cassandra.auth.IRoleManager.Option.PASSWORD, "testpassword"); + } @Test public void testCreatRoleWithLoginPriorToPassword() { - assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '123'")); + assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '123'")); + + assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '%s'", OBFUSCATION_TOKEN), + obfuscate(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '%s'", optsPassword), opts)); } @Test public void testCreatRoleWithLoginAfterPassword() { - assertEquals(format("CREATE ROLE role1 WITH password%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE ROLE role1 WITH password = '123' AND LOGIN = true")); + assertEquals(format("CREATE ROLE role1 WITH password %s", OBFUSCATION_TOKEN), + obfuscate("CREATE ROLE role1 WITH password = '123' AND LOGIN = true")); + + assertEquals(format("CREATE ROLE role1 WITH password = '%s' AND LOGIN = true", OBFUSCATION_TOKEN), + obfuscate(format("CREATE ROLE role1 WITH password = '%s' AND LOGIN = true", optsPassword), opts)); } @Test public void testCreateRoleWithoutPassword() { - assertEquals("CREATE ROLE role1", obfuscator.obfuscate("CREATE ROLE role1")); + assertEquals("CREATE ROLE role1", obfuscate("CREATE ROLE role1")); + assertEquals("CREATE ROLE role1", obfuscate("CREATE ROLE role1", opts)); } @Test public void testCreateMultipleRoles() { - assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '123';" + - "CREATE ROLE role2 WITH LOGIN = true AND PASSWORD = '123'")); + assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '123';" + + "CREATE ROLE role2 WITH LOGIN = true AND PASSWORD = '123'")); + + assertEquals(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '%s';" + + "CREATE ROLE role2 WITH LOGIN = true AND PASSWORD = '%s'", OBFUSCATION_TOKEN, OBFUSCATION_TOKEN), + obfuscate(format("CREATE ROLE role1 WITH LOGIN = true AND PASSWORD = '%s';" + + "CREATE ROLE role2 WITH LOGIN = true AND PASSWORD = '%s'", optsPassword, optsPassword), + opts)); } @Test public void testAlterRoleWithPassword() { - assertEquals(format("ALTER ROLE role1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER ROLE role1 with PASSWORD = '123'")); + assertEquals(format("ALTER ROLE role1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("ALTER ROLE role1 with PASSWORD = '123'")); + + assertEquals(format("ALTER ROLE role1 with PASSWORD = '%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER ROLE role1 with PASSWORD = '%s'", optsPassword), opts)); } @Test public void testAlterRoleWithPasswordNoSpace() { - assertEquals(format("ALTER ROLE role1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER ROLE role1 with PASSWORD='123'")); + assertEquals(format("ALTER ROLE role1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("ALTER ROLE role1 with PASSWORD='123'")); + + assertEquals(format("ALTER ROLE role1 with PASSWORD='%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER ROLE role1 with PASSWORD='%s'", optsPassword), opts)); } @Test public void testAlterRoleWithPasswordNoImmediateSpace() { - assertEquals(format("ALTER ROLE role1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER ROLE role1 with PASSWORD= '123'")); + assertEquals(format("ALTER ROLE role1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("ALTER ROLE role1 with PASSWORD= '123'")); + + assertEquals(format("ALTER ROLE role1 with PASSWORD= '%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER ROLE role1 with PASSWORD= '%s'", optsPassword), opts)); } @Test public void testAlterRoleWithoutPassword() { - assertEquals("ALTER ROLE role1", obfuscator.obfuscate("ALTER ROLE role1")); + assertEquals("ALTER ROLE role1", obfuscate("ALTER ROLE role1")); + + assertEquals("ALTER ROLE role1", obfuscate("ALTER ROLE role1", opts)); } @Test public void testCreateUserWithPassword() { - assertEquals(format("CREATE USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE USER user1 with PASSWORD '123'")); + assertEquals(format("CREATE USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE USER user1 with PASSWORD '123'")); + + assertEquals(format("CREATE USER user1 with PASSWORD '%s'", OBFUSCATION_TOKEN), + obfuscate(format("CREATE USER user1 with PASSWORD '%s'", optsPassword), opts)); } @Test public void testCreateUserWithoutPassword() { - assertEquals("CREATE USER user1", obfuscator.obfuscate("CREATE USER user1")); + assertEquals("CREATE USER user1", obfuscate("CREATE USER user1")); + + assertEquals("CREATE USER user1", obfuscate("CREATE USER user1", opts)); } @Test public void testAlterUserWithPassword() { - assertEquals(format("ALTER USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER USER user1 with PASSWORD '123'")); + assertEquals(format("ALTER USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("ALTER USER user1 with PASSWORD '123'")); + + assertEquals(format("ALTER USER user1 with PASSWORD '%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER USER user1 with PASSWORD '%s'", optsPassword), opts)); } @Test public void testAlterUserWithPasswordMixedCase() { - assertEquals(format("ALTER USER user1 with paSSwoRd%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER USER user1 with paSSwoRd '123'")); + assertEquals(format("ALTER USER user1 with paSSwoRd %s", OBFUSCATION_TOKEN), + obfuscate("ALTER USER user1 with paSSwoRd '123'")); + + assertEquals(format("ALTER USER user1 with paSSwoRd '%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER USER user1 with paSSwoRd '%s'", optsPassword), opts)); } @Test public void testAlterUserWithPasswordWithNewLine() { - assertEquals(format("ALTER USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("ALTER USER user1 with PASSWORD\n'123'")); + assertEquals(format("ALTER USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("ALTER USER user1 with PASSWORD\n'123'")); + + assertEquals(format("ALTER USER user1 with PASSWORD\n'%s'", OBFUSCATION_TOKEN), + obfuscate(format("ALTER USER user1 with PASSWORD\n'%s'", optsPassword), opts)); } @Test public void testPasswordWithNewLinesObfuscation() { - assertEquals(String.format("CREATE USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE USER user1 with PASSWORD 'a\nb'")); + assertEquals(String.format("CREATE USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE USER user1 with PASSWORD 'a\nb'")); + + RoleOptions newLinePassOpts = new RoleOptions(); + newLinePassOpts.setOption(org.apache.cassandra.auth.IRoleManager.Option.PASSWORD, "test\npassword"); + assertEquals(String.format("CREATE USER user1 with PASSWORD '%s'", OBFUSCATION_TOKEN), + obfuscate(format("CREATE USER user1 with PASSWORD '%s'", "test\npassword"), newLinePassOpts)); } @Test public void testEmptyPasswordObfuscation() { - assertEquals(String.format("CREATE USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE USER user1 with PASSWORD ''")); + assertEquals(String.format("CREATE USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE USER user1 with PASSWORD ''")); + + RoleOptions emptyPassOpts = new RoleOptions(); + emptyPassOpts.setOption(org.apache.cassandra.auth.IRoleManager.Option.PASSWORD, ""); + assertEquals("CREATE USER user1 with PASSWORD ''", + obfuscate("CREATE USER user1 with PASSWORD ''", emptyPassOpts)); } @Test public void testPasswordWithSpaces() { - assertEquals(String.format("CREATE USER user1 with PASSWORD%s", PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("CREATE USER user1 with PASSWORD 'p a ss wor d'")); + assertEquals(String.format("CREATE USER user1 with PASSWORD %s", OBFUSCATION_TOKEN), + obfuscate("CREATE USER user1 with PASSWORD 'p a ss wor d'")); } @Test public void testSimpleBatch() { assertEquals(format("BEGIN BATCH \n" + - " CREATE ROLE alice1 WITH PASSWORD%s", - PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD %s", + OBFUSCATION_TOKEN), + obfuscate("BEGIN BATCH \n" + " CREATE ROLE alice1 WITH PASSWORD = 'alice123' and LOGIN = true; \n" + "APPLY BATCH;")); + + assertEquals(format("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD = '%s' and LOGIN = true; \n" + + "APPLY BATCH;", OBFUSCATION_TOKEN), + obfuscate(format("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD = '%s' and LOGIN = true; \n" + + "APPLY BATCH;", optsPassword), + opts)); } @Test public void testComplexBatch() { assertEquals(format("BEGIN BATCH \n" + - " CREATE ROLE alice1 WITH PASSWORD%s", - PasswordObfuscator.OBFUSCATION_TOKEN), - obfuscator.obfuscate("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD %s", + OBFUSCATION_TOKEN), + obfuscate("BEGIN BATCH \n" + " CREATE ROLE alice1 WITH PASSWORD = 'alice123' and LOGIN = true; \n" + " CREATE ROLE alice2 WITH PASSWORD = 'alice123' and LOGIN = true; \n" + "APPLY BATCH;")); + + assertEquals(format("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD = '%s' and LOGIN = true; \n" + + " CREATE ROLE alice2 WITH PASSWORD = '%s' and LOGIN = true; \n" + + "APPLY BATCH;" + , OBFUSCATION_TOKEN, OBFUSCATION_TOKEN), + obfuscate(format("BEGIN BATCH \n" + + " CREATE ROLE alice1 WITH PASSWORD = '%s' and LOGIN = true; \n" + + " CREATE ROLE alice2 WITH PASSWORD = '%s' and LOGIN = true; \n" + + "APPLY BATCH;", optsPassword, optsPassword), + opts)); } -} \ No newline at end of file +}