Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#226 Audit Prepare statements #241

Merged
merged 3 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
## Version 2.12.0

* Test with java11 (only flavor ecaudit_c4.0) - #214
* Audit Prepare statements

## Version 2.11.0

Expand Down
5 changes: 5 additions & 0 deletions conf/audit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,8 @@ whitelist_cache_update_interval_in_ms: 20000
# Maximum number of entries in the whitelist cache
# Default to 10 x the value of roles_cache_max_entries (specified in cassandra.yaml)
whitelist_cache_max_entries: 10000

# Whether to suppress the auditing of prepare statements
# Default is to suppress the audit statements this is to match the previous versions which do not audit prepare statements

suppress_prepare_statements: true
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.ericsson.bss.cassandra.ecaudit.entry.PreparedAuditOperation;
import com.ericsson.bss.cassandra.ecaudit.entry.factory.AuditEntryBuilderFactory;
import com.ericsson.bss.cassandra.ecaudit.entry.suppressor.BoundValueSuppressor;
import com.ericsson.bss.cassandra.ecaudit.entry.suppressor.PrepareAuditOperation;
import com.ericsson.bss.cassandra.ecaudit.facade.Auditor;
import com.ericsson.bss.cassandra.ecaudit.utils.Exceptions;
import org.apache.cassandra.cql3.BatchQueryOptions;
Expand Down Expand Up @@ -108,6 +109,31 @@ public void auditRegular(String operation, ClientState state, Status status, lon
}
}

/**
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
* Audit a regular CQL statement.
*
* @param operation the CQL statement to audit
* @param state the client state accompanying the statement
* @param status the statement operation status
* @param timestamp the system timestamp for the request
*/
public void auditPrepare(String operation, ClientState state, Status status, long timestamp)
{
if (auditor.shouldLogForStatus(status) && auditor.shouldLogPrepareStatements())
{
AuditEntry logEntry = entryBuilderFactory.createEntryBuilder(operation, state)
.client(state.getRemoteAddress())
.coordinator(FBUtilities.getJustBroadcastAddress())
.user(state.getUser().getName())
.operation(new PrepareAuditOperation(operation))
.status(status)
.timestamp(timestamp)
.build();

auditor.audit(logEntry);
}
}

/**
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
* Audit a prepared statement.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ public void setWhitelistCacheActiveUpdate(boolean whitelistCacheActiveUpdate)
yamlConfig.setWhitelistCacheActiveUpdate(whitelistCacheActiveUpdate);
}

public boolean isSuppressPrepareStatements()
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
{
loadConfigIfNeeded();
return yamlConfig.isSuppressPrepareStatements();
}

private synchronized void loadConfigIfNeeded()
{
if (yamlConfig == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public final class AuditYamlConfig
public Integer whitelist_cache_update_interval_in_ms;
public Integer whitelist_cache_max_entries;
public Boolean whitelist_cache_active_update;
public Boolean suppress_prepare_statements;

static AuditYamlConfig createWithoutFile()
{
Expand Down Expand Up @@ -170,4 +171,11 @@ public void setWhitelistCacheActiveUpdate(Boolean whitelistCacheActiveUpdate)
{
this.whitelist_cache_active_update = whitelistCacheActiveUpdate;
}

public Boolean isSuppressPrepareStatements()
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
{
return suppress_prepare_statements == null
? Boolean.TRUE
: suppress_prepare_statements;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2018 Telefonaktiebolaget LM Ericsson
*
* Licensed 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 com.ericsson.bss.cassandra.ecaudit.entry.suppressor;

import com.ericsson.bss.cassandra.ecaudit.common.record.AuditOperation;

public class PrepareAuditOperation implements AuditOperation
{
private final String operationString;

/**
* Construct a new audit operation.
* @param operationString the operation/statement to wrap.
*/
public PrepareAuditOperation(String operationString)
{
this.operationString = "Prepared: " + operationString;
}

@Override
public String getOperationString()
{
return operationString;
}

@Override
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
public String getNakedOperationString()
{
return operationString;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public interface Auditor
*/
boolean shouldLogFailedBatchSummary();

boolean shouldLogPrepareStatements();

/**
* Sets the log timing strategy to use.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ public boolean shouldLogFailedBatchSummary()
return logTimingStrategy.shouldLogFailedBatchSummary();
}

@Override
public boolean shouldLogPrepareStatements()
{
return filter.shouldLogPrepareStatements();
}

@Override
public void setLogTimingStrategy(LogTimingStrategy logTimingStrategy)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,6 @@ public interface AuditFilter {
* For example, use this method to create any required keyspaces/tables.
*/
void setup();

boolean shouldLogPrepareStatements();
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,10 @@ public void setup()
{
// Intentionally left empty
}

@Override
public boolean shouldLogPrepareStatements()
{
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ public void setup()
whitelistDataAccess.setup();
}

@Override
public boolean shouldLogPrepareStatements() { return true; }

tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
/**
* Returns true if the supplied log entry's role or any other role granted to it (directly or indirectly) is
* white-listed for the log entry's specified operations and resource.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,10 @@ public void setup()
{
whitelist = auditConfig.getYamlWhitelist();
}

@Override
public boolean shouldLogPrepareStatements()
{
return !auditConfig.isSuppressPrepareStatements();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,9 @@ public void setup()
yamlFilter.setup();
roleFilter.setup();
}
@Override
public boolean shouldLogPrepareStatements()
{
return yamlFilter.shouldLogPrepareStatements();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,20 @@ private ResultMessage processBatchWithAudit(BatchStatement statement, List<Strin
public ResultMessage.Prepared prepare(String query, ClientState state, Map<String, ByteBuffer> customPayload)
throws RequestValidationException
{
return wrappedQueryHandler.prepare(query, state, customPayload);
long timestamp = System.currentTimeMillis();
auditAdapter.auditPrepare(query, state, Status.ATTEMPT, timestamp);
ResultMessage.Prepared preparedStatement;
try
{
preparedStatement = wrappedQueryHandler.prepare(query, state, customPayload);
}
catch (RuntimeException e)
{
auditAdapter.auditPrepare(query, state, Status.FAILED, timestamp);
throw e;
}

return preparedStatement;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public void testDefaultConfiguration()
assertThat(config.getWhitelistCacheValidity()).isEqualTo(DatabaseDescriptor.getRolesValidity());
assertThat(config.getWhitelistCacheUpdateInterval()).isEqualTo(DatabaseDescriptor.getRolesUpdateInterval());
assertThat(config.getWhitelistCacheMaxEntries()).isEqualTo(DatabaseDescriptor.getRolesCacheMaxEntries() * 10);
assertThat(config.isSuppressPrepareStatements()).isEqualTo(true);
}

@Test
Expand All @@ -122,6 +123,7 @@ public void testCustomConfiguration()
assertThat(config.getWhitelistCacheValidity()).isEqualTo(42);
assertThat(config.getWhitelistCacheUpdateInterval()).isEqualTo(41);
assertThat(config.getWhitelistCacheMaxEntries()).isEqualTo(40);
assertThat(config.isSuppressPrepareStatements()).isEqualTo(false);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public void testPrepareAndGetPrepared()

verify(mockHandler, times(1)).prepare(eq(query), eq(mockClientState), eq(customPayload));
verify(mockHandler, times(1)).getPrepared(eq(statementId));
verify(mockAdapter, times(1)).auditPrepare(eq(query), eq(mockClientState), eq(Status.ATTEMPT), longThat(isCloseToNow()));
}

@Test
Expand Down
1 change: 1 addition & 0 deletions ecaudit/src/test/resources/mock_configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ bound_value_suppressor: SuppressBlobs
whitelist_cache_validity_in_ms: 42
whitelist_cache_update_interval_in_ms: 41
whitelist_cache_max_entries: 40
suppress_prepare_statements: false
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.ericsson.bss.cassandra.ecaudit.integration.querylogger;

import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -30,6 +31,8 @@
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.Appender;
import com.datastax.oss.driver.api.core.CqlSession;
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
import com.datastax.oss.driver.api.core.servererrors.InvalidQueryException;
import com.datastax.oss.driver.api.core.servererrors.UnauthorizedException;
import com.ericsson.bss.cassandra.ecaudit.logger.Slf4jAuditLogger;
import com.ericsson.bss.cassandra.ecaudit.test.daemon.CassandraDaemonForAuditTest;
Expand All @@ -41,6 +44,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -95,6 +99,30 @@ public void testBasicStatement()
assertThat(getLogEntries()).containsOnly("client:'127.0.0.1'|user:'anonymous'|status:'ATTEMPT'|operation:'INSERT INTO school.students (key, value) VALUES (42, 'Kalle')'");
}

@Test
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
public void testPrepareStatement()
{
givenTable("school", "students");
reset(mockAuditAppender);

PreparedStatement prepared = session.prepare("INSERT INTO school.students (key, value) VALUES (?, ?)");
session.execute(prepared.bind(42, "Kalle"));
assertThat(getLogEntries()).containsOnly( "client:'127.0.0.1'|user:'anonymous'|status:'ATTEMPT'|operation:'Prepared: INSERT INTO school.students (key, value) VALUES (?, ?)'",
"client:'127.0.0.1'|user:'anonymous'|status:'ATTEMPT'|operation:'INSERT INTO school.students (key, value) VALUES (?, ?)[42, 'Kalle']'");
}

@Test
tommystendahl marked this conversation as resolved.
Show resolved Hide resolved
public void testFailedPrepareStatement()
{
givenTable("school", "students");
reset(mockAuditAppender);

assertThatExceptionOfType(InvalidQueryException.class).isThrownBy(() -> session.prepare("INSERT INTO school.invalidestudents (key, value) VALUES (?, ?)"));

assertThat(getLogEntries()).containsOnly( "client:'127.0.0.1'|user:'anonymous'|status:'ATTEMPT'|operation:'Prepared: INSERT INTO school.invalidestudents (key, value) VALUES (?, ?)'",
"client:'127.0.0.1'|user:'anonymous'|status:'FAILED'|operation:'Prepared: INSERT INTO school.invalidestudents (key, value) VALUES (?, ?)'");
}

@Test
public void testGrantFails()
{
Expand Down
Loading
Loading