From 25291ff3fd99f92cdb0a7d5d2125442282d42ff8 Mon Sep 17 00:00:00 2001 From: Francisco Guerrero Date: Mon, 22 Jul 2024 10:56:21 -0700 Subject: [PATCH] Allow configuring log format for Audit Logs patch by Francisco Guerrero; reviewed by Stefan Miklosovic, Andy Tolbert for CASSANDRA-19792 --- CHANGES.txt | 1 + conf/cassandra.yaml | 6 ++ .../apache/cassandra/audit/AuditLogEntry.java | 39 +++++--- .../cassandra/audit/AuditLogManager.java | 2 + .../cassandra/audit/BinAuditLogger.java | 34 +++++-- .../cassandra/audit/FileAuditLogger.java | 13 ++- .../auth/MutualTlsAuthenticator.java | 2 +- .../cassandra/audit/AuditLogEntryTest.java | 92 +++++++++++++++++++ .../cassandra/audit/FileAuditLoggerTest.java | 75 +++++++++++++++ 9 files changed, 241 insertions(+), 23 deletions(-) create mode 100644 test/unit/org/apache/cassandra/audit/AuditLogEntryTest.java create mode 100644 test/unit/org/apache/cassandra/audit/FileAuditLoggerTest.java diff --git a/CHANGES.txt b/CHANGES.txt index d166688312..9abfadbfde 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Allow configuring log format for Audit Logs (CASSANDRA-19792) * Support for noboolean rpm (centos7 compatible) packages removed (CASSANDRA-19787) * Allow threads waiting for the metadata log follower to be interrupted (CASSANDRA-19761) * Support dictionary lookup for CassandraPasswordValidator (CASSANDRA-19762) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 954a87e9d2..546333dfcf 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1823,10 +1823,16 @@ unlogged_batch_across_partitions_warn_threshold: 10 # Audit logging - Logs every incoming CQL command request, authentication to a node. See the docs # on audit_logging for full details about the various configuration options and production tips. +# For BinAuditLogger and FileAuditLogger, the following optional parameters can be configured: +# - key_value_separator: Specifies the key-value separator to use for the log message. (Defaults to ":") +# - field_separator: Specifies the separator between fields in the log message. (Defaults to "|") audit_logging_options: enabled: false logger: - class_name: BinAuditLogger + # parameters: + # - key_value_separator: ":" + # field_separator: "|" # audit_logs_dir: # included_keyspaces: # excluded_keyspaces: system, system_schema, system_virtual_schema diff --git a/src/java/org/apache/cassandra/audit/AuditLogEntry.java b/src/java/org/apache/cassandra/audit/AuditLogEntry.java index 3fe53816b3..3f53611299 100644 --- a/src/java/org/apache/cassandra/audit/AuditLogEntry.java +++ b/src/java/org/apache/cassandra/audit/AuditLogEntry.java @@ -38,6 +38,8 @@ import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis; public class AuditLogEntry { + static final String DEFAULT_KEY_VALUE_SEPARATOR = ":"; + static final String DEFAULT_FIELD_SEPARATOR = "|"; private final InetAddressAndPort host = FBUtilities.getBroadcastAddressAndPort(); private final InetAddressAndPort source; private final String user; @@ -69,42 +71,49 @@ public class AuditLogEntry @VisibleForTesting public String getLogString() { - StringBuilder builder = new StringBuilder(100); - builder.append("user:").append(user) - .append("|host:").append(host); + return getLogString(DEFAULT_KEY_VALUE_SEPARATOR, DEFAULT_FIELD_SEPARATOR); + } + String getLogString(String keyValueSeparator, String fieldSeparator) + { + StringBuilder builder = new StringBuilder(100); + builder.append("user").append(keyValueSeparator).append(user) + .append(fieldSeparator).append("host").append(keyValueSeparator).append(host); + + // Source is only expected to be null during testing + // in MacOS when running in-jvm dtests if (source != null) { - builder.append("|source:").append(source.getAddress()); + builder.append(fieldSeparator).append("source").append(keyValueSeparator).append(source.getAddress()); if (source.getPort() > 0) { - builder.append("|port:").append(source.getPort()); + builder.append(fieldSeparator).append("port").append(keyValueSeparator).append(source.getPort()); } } - builder.append("|timestamp:").append(timestamp) - .append("|type:").append(type) - .append("|category:").append(type.getCategory()); + builder.append(fieldSeparator).append("timestamp").append(keyValueSeparator).append(timestamp) + .append(fieldSeparator).append("type").append(keyValueSeparator).append(type) + .append(fieldSeparator).append("category").append(keyValueSeparator).append(type.getCategory()); if (batch != null) { - builder.append("|batch:").append(batch); + builder.append(fieldSeparator).append("batch").append(keyValueSeparator).append(batch); } if (StringUtils.isNotBlank(keyspace)) { - builder.append("|ks:").append(keyspace); + builder.append(fieldSeparator).append("ks").append(keyValueSeparator).append(keyspace); } if (StringUtils.isNotBlank(scope)) { - builder.append("|scope:").append(scope); + builder.append(fieldSeparator).append("scope").append(keyValueSeparator).append(scope); } if (StringUtils.isNotBlank(operation)) { - builder.append("|operation:").append(operation); + builder.append(fieldSeparator).append("operation").append(keyValueSeparator).append(operation); } if (metadata != null && !metadata.isEmpty()) { - metadata.forEach((key, value) -> builder.append('|').append(key).append(':').append(value)); + metadata.forEach((key, value) -> builder.append(fieldSeparator).append(key).append(keyValueSeparator).append(value)); } return builder.toString(); } @@ -203,9 +212,9 @@ public class AuditLogEntry if (clientState != null) { - if (clientState.getRemoteAddress() != null) + InetSocketAddress addr = clientState.getRemoteAddress(); + if (addr != null) { - InetSocketAddress addr = clientState.getRemoteAddress(); source = InetAddressAndPort.getByAddressOverrideDefaults(addr.getAddress(), addr.getPort()); } diff --git a/src/java/org/apache/cassandra/audit/AuditLogManager.java b/src/java/org/apache/cassandra/audit/AuditLogManager.java index bebe4ec70c..85f754f019 100644 --- a/src/java/org/apache/cassandra/audit/AuditLogManager.java +++ b/src/java/org/apache/cassandra/audit/AuditLogManager.java @@ -176,6 +176,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene IAuditLogger oldLogger = auditLogger; auditLogger = new NoOpAuditLogger(Collections.emptyMap()); oldLogger.stop(); + logger.info("Audit logging is disabled."); } /** @@ -214,6 +215,7 @@ public class AuditLogManager implements QueryEvents.Listener, AuthEvents.Listene // ensure oldLogger's stop() is called after we swap it with new logger, // otherwise, we might be calling log() on the stopped logger. oldLogger.stop(); + logger.info("Audit logging is enabled."); } private void updateAuditLogOptions(final AuditLogOptions options, final AuditLogFilter filter) diff --git a/src/java/org/apache/cassandra/audit/BinAuditLogger.java b/src/java/org/apache/cassandra/audit/BinAuditLogger.java index e624446c74..ee237c6021 100644 --- a/src/java/org/apache/cassandra/audit/BinAuditLogger.java +++ b/src/java/org/apache/cassandra/audit/BinAuditLogger.java @@ -31,6 +31,9 @@ import org.apache.cassandra.utils.ObjectSizes; import org.apache.cassandra.utils.binlog.BinLog; import org.apache.cassandra.utils.concurrent.WeightedQueue; +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_FIELD_SEPARATOR; +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_KEY_VALUE_SEPARATOR; + public class BinAuditLogger implements IAuditLogger { public static final long CURRENT_VERSION = 0; @@ -38,9 +41,24 @@ public class BinAuditLogger implements IAuditLogger public static final String AUDITLOG_MESSAGE = "message"; private static final Logger logger = LoggerFactory.getLogger(BinAuditLogger.class); + private final String keyValueSeparator; + private final String fieldSeparator; + private volatile BinLog binLog; public BinAuditLogger(AuditLogOptions auditLoggingOptions) + { + this(auditLoggingOptions, DEFAULT_KEY_VALUE_SEPARATOR, DEFAULT_FIELD_SEPARATOR); + } + + public BinAuditLogger(Map params) + { + this(DatabaseDescriptor.getAuditLoggingOptions(), + getFromParamsOrDefault(params, "key_value_separator", DEFAULT_KEY_VALUE_SEPARATOR), + getFromParamsOrDefault(params, "field_separator", DEFAULT_FIELD_SEPARATOR)); + } + + BinAuditLogger(AuditLogOptions auditLoggingOptions, String keyValueSeparator, String fieldSeparator) { this.binLog = new BinLog.Builder().path(File.getPath(auditLoggingOptions.audit_logs_dir)) .rollCycle(auditLoggingOptions.roll_cycle) @@ -50,11 +68,8 @@ public class BinAuditLogger implements IAuditLogger .archiveCommand(auditLoggingOptions.archive_command) .maxArchiveRetries(auditLoggingOptions.max_archive_retries) .build(false); - } - - public BinAuditLogger(Map params) - { - this(DatabaseDescriptor.getAuditLoggingOptions()); + this.keyValueSeparator = keyValueSeparator; + this.fieldSeparator = fieldSeparator; } /** @@ -91,7 +106,14 @@ public class BinAuditLogger implements IAuditLogger { return; } - binLog.logRecord(new Message(auditLogEntry.getLogString())); + binLog.logRecord(new Message(auditLogEntry.getLogString(keyValueSeparator, fieldSeparator))); + } + + static String getFromParamsOrDefault(Map params, String key, String defaultValue) + { + return params != null + ? params.getOrDefault(key, defaultValue) + : defaultValue; } diff --git a/src/java/org/apache/cassandra/audit/FileAuditLogger.java b/src/java/org/apache/cassandra/audit/FileAuditLogger.java index a5fffcbb1a..e1d151fdd7 100644 --- a/src/java/org/apache/cassandra/audit/FileAuditLogger.java +++ b/src/java/org/apache/cassandra/audit/FileAuditLogger.java @@ -23,6 +23,9 @@ import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_FIELD_SEPARATOR; +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_KEY_VALUE_SEPARATOR; + /** * Synchronous, file-based audit logger; just uses the standard logging mechansim. */ @@ -31,10 +34,18 @@ public class FileAuditLogger implements IAuditLogger protected static final Logger logger = LoggerFactory.getLogger(FileAuditLogger.class); private volatile boolean enabled; + private final String keyValueSeparator; + private final String fieldSeparator; public FileAuditLogger(Map params) { enabled = true; + keyValueSeparator = params != null + ? params.getOrDefault("key_value_separator", DEFAULT_KEY_VALUE_SEPARATOR) + : DEFAULT_KEY_VALUE_SEPARATOR; + fieldSeparator = params != null + ? params.getOrDefault("field_separator", DEFAULT_FIELD_SEPARATOR) + : DEFAULT_FIELD_SEPARATOR; } @Override @@ -48,7 +59,7 @@ public class FileAuditLogger implements IAuditLogger { // don't bother with the volatile read of enabled here. just go ahead and log, other components // will check the enbaled field. - logger.info(auditLogEntry.getLogString()); + logger.info(auditLogEntry.getLogString(keyValueSeparator, fieldSeparator)); } @Override diff --git a/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java b/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java index 14d6ef72c6..0337291a9d 100644 --- a/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java @@ -85,7 +85,7 @@ public class MutualTlsAuthenticator implements IAuthenticator private final DurationSpec.IntMinutesBound certificateValidityWarnThreshold; // key for the 'identity' value in AuthenticatedUser metadata map. - static final String METADATA_IDENTITY_KEY = "identity"; + public static final String METADATA_IDENTITY_KEY = "identity"; public MutualTlsAuthenticator(Map parameters) { diff --git a/test/unit/org/apache/cassandra/audit/AuditLogEntryTest.java b/test/unit/org/apache/cassandra/audit/AuditLogEntryTest.java new file mode 100644 index 0000000000..40285fe310 --- /dev/null +++ b/test/unit/org/apache/cassandra/audit/AuditLogEntryTest.java @@ -0,0 +1,92 @@ +/* + * 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.audit; + +import java.net.InetSocketAddress; + +import com.google.common.collect.ImmutableMap; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.auth.AuthenticatedUser; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.QueryState; + +import static org.apache.cassandra.auth.MutualTlsAuthenticator.METADATA_IDENTITY_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link AuditLogEntry} + */ +public class AuditLogEntryTest +{ + AuditLogEntry entry; + + @BeforeClass + public static void beforeClass() + { + DatabaseDescriptor.daemonInitialization(); + } + + @Before + public void setup() + { + InetSocketAddress remoteAddress = new InetSocketAddress("127.0.0.1", 9999); + + ImmutableMap userMetadata = ImmutableMap.of(METADATA_IDENTITY_KEY, "cassandra_user_identity"); + AuthenticatedUser mockAuthenticatedUser = mock(AuthenticatedUser.class); + when(mockAuthenticatedUser.getName()).thenReturn("cassandra_user"); + when(mockAuthenticatedUser.getMetadata()).thenReturn(userMetadata); + + ClientState mockClientState = mock(ClientState.class); + when(mockClientState.getRemoteAddress()).thenReturn(remoteAddress); + when(mockClientState.getUser()).thenReturn(mockAuthenticatedUser); + when(mockClientState.getKeyspace()).thenReturn("test_keyspace"); + + QueryState mockQueryState = mock(QueryState.class); + when(mockQueryState.getClientState()).thenReturn(mockClientState); + + entry = new AuditLogEntry.Builder(mockQueryState) + .setOperation("LOGIN SUCCESSFUL") + .setType(AuditLogEntryType.LOGIN_SUCCESS) + .build(); + } + + @Test + public void testDefaultGetLogString() + { + assertThat(entry.getLogString()).matches("user:cassandra_user\\|host:/127.0.0.1:7012\\|" + + "source:/127.0.0.1\\|port:9999\\|timestamp:\\d+\\|" + + "type:LOGIN_SUCCESS\\|category:AUTH\\|" + + "operation:LOGIN SUCCESSFUL\\|identity:cassandra_user_identity"); + } + + @Test + public void testGetLogStringWithCustomSeparators() + { + assertThat(entry.getLogString("=", " ")).matches("user=cassandra_user host=/127.0.0.1:7012 " + + "source=/127.0.0.1 port=9999 timestamp=\\d+ " + + "type=LOGIN_SUCCESS category=AUTH " + + "operation=LOGIN SUCCESSFUL identity=cassandra_user_identity"); + } +} diff --git a/test/unit/org/apache/cassandra/audit/FileAuditLoggerTest.java b/test/unit/org/apache/cassandra/audit/FileAuditLoggerTest.java new file mode 100644 index 0000000000..7fdac175f6 --- /dev/null +++ b/test/unit/org/apache/cassandra/audit/FileAuditLoggerTest.java @@ -0,0 +1,75 @@ +/* + * 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.audit; + +import java.util.Map; + +import org.junit.Test; + +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_FIELD_SEPARATOR; +import static org.apache.cassandra.audit.AuditLogEntry.DEFAULT_KEY_VALUE_SEPARATOR; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Unit tests for the {@link FileAuditLogger} class + */ +public class FileAuditLoggerTest +{ + @Test + public void testEnabled() + { + FileAuditLogger fileAuditLogger = new FileAuditLogger(null); + assertThat(fileAuditLogger.isEnabled()).isTrue(); + fileAuditLogger.stop(); + assertThat(fileAuditLogger.isEnabled()).isFalse(); + } + + @Test + public void testUseDefaultSeparators() + { + AuditLogEntry mockAuditLogEntry = mock(AuditLogEntry.class); + + // null map configuration + new FileAuditLogger(null).log(mockAuditLogEntry); + verify(mockAuditLogEntry, times(1)).getLogString(DEFAULT_KEY_VALUE_SEPARATOR, DEFAULT_FIELD_SEPARATOR); + + // empty map configuration + new FileAuditLogger(Map.of()).log(mockAuditLogEntry); + verify(mockAuditLogEntry, times(2)).getLogString(DEFAULT_KEY_VALUE_SEPARATOR, DEFAULT_FIELD_SEPARATOR); + } + + @Test + public void testCustomKeyValueSeparator() + { + AuditLogEntry mockAuditLogEntry = mock(AuditLogEntry.class); + new FileAuditLogger(Map.of("key_value_separator", "=")).log(mockAuditLogEntry); + verify(mockAuditLogEntry, times(1)).getLogString("=", DEFAULT_FIELD_SEPARATOR); + } + + @Test + public void testCustomFieldSeparator() + { + AuditLogEntry mockAuditLogEntry = mock(AuditLogEntry.class); + new FileAuditLogger(Map.of("field_separator", "_")).log(mockAuditLogEntry); + verify(mockAuditLogEntry, times(1)).getLogString(DEFAULT_KEY_VALUE_SEPARATOR, "_"); + } +}