k8ssandra / management-api-for-apache-cassandra

RESTful / Secure Management Sidecar for Apache Cassandra
Apache License 2.0
72 stars 52 forks source link

NullPointerException in cassandra.audit.AuditLogManager.querySuccess when querying endpoints #309

Open vanessagits opened 1 year ago

vanessagits commented 1 year ago

NullPointerException in cassandra.audit.AuditLogManager.querySuccess when querying endpoints via mgmtapi

cassandra version: 4.1.1 mgmtapi version: 0.1.59

cassandra.yaml:audit_logging_options: cassandra.yaml- enabled: true cassandra.yaml- logger: cassandra.yaml: - class_name: FileAuditLogger

When querying endpoints (/api/v0/metadata/endpoints) via mgmtapi, 200 OK is returned. However, NPE is reported by AuditLogManager.

ERROR [epollEventLoopGroup-5-3] 2023-05-10 15:53:30,301 NoSpamLogger.java:111 - Failed notifying listeners
java.lang.NullPointerException: null
        at org.apache.cassandra.audit.AuditLogManager.querySuccess(AuditLogManager.java:244)
        at org.apache.cassandra.cql3.QueryEvents.notifyQuerySuccess(QueryEvents.java:78)
        at org.apache.cassandra.transport.messages.QueryMessage.execute(QueryMessage.java:117)
        at org.apache.cassandra.transport.Message$Request.execute(Message.java:254)
        at org.apache.cassandra.transport.UnixSocketServer41x$UnixSockMessage.channelRead0(UnixSocketServer41x.java:106)
        at org.apache.cassandra.transport.UnixSocketServer41x$UnixSockMessage.channelRead0(UnixSocketServer41x.java:86)
        at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:99)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)

Reference: https://issues.apache.org/jira/browse/CASSANDRA-18518

┆Issue is synchronized with this Jira Story by Unito ┆Issue Number: MAPI-34

emerkle826 commented 1 year ago

This is an issue with 4.0.x Cassandra images as well it seems

SarthakSahu commented 9 months ago

@emerkle826 @Miles-Garnsey @adejanovski Could you please share a fix plan for this bug.

emerkle826 commented 9 months ago

@SarthakSahu Sorry, we haven't had a chance to get around to this. I'll try to get a few cycles to take a look soon, but it won't be for a couple of weeks from now. I'll try to add another comment again when we get a chance to take a look more into this issue.

SarthakSahu commented 6 months ago

@emerkle826 is there any update.

burmanm commented 6 months ago

From my short investigation, this NPE is fine and can be ignored. It happens because the AuditLogManager expects an "AuditLogContext" with a "AuditLogEntryType" and our CQL commands do not have such (and Cassandra itself doesn't support arbitrary AuditLogEntries).

So the workaround is to set in the shim the public AuditLogContext getAuditLogContext() to something that's not true, like:

return new AuditLogContext(AuditLogEntryType.DESCRIBE)

But then you have to understand those describe audit log entries are coming from management-api and are not necessarily describe events. There's no way to have our own type of something generic like "external" sadly. This could be something to propose to Cassandra of course, but that would only work in future versions.

@emerkle826 The patch itself is simple, but I can't think of a good entryType from here: https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/audit/AuditLogEntryType.java that would match all the management-api queries we do. I mean, for some we could definitely have a correct one (like create user), but we don't have a strategy to set that currently in our queries - and that would require some more work. I can make the ticket and PR to Cassandra proper though.

emerkle826 commented 6 months ago

There's no way to have our own type of something generic like "external" sadly. This could be something to propose to Cassandra of course, but that would only work in future versions.

Thanks for the investigation @burmanm I understand the issue with Management API not having/providing the AuditLogContext, but I don't see how a "generic" or "external" type will satisfy the needs of an Audit. I assume the point of the AuditLogContext is so that you know what types of queries are being done (and when and by whom). I don't know that Cassandra needs to support a generic type, because what would that tell you? Or is the idea that maybe Cassnadra doesn't have a category for the log, but something external to Cassandra will understand what it is?

Just some additional thoughts....

burmanm commented 6 months ago

Well, mainly I would say the idea is that Cassandra at least has logged that something happened. Otherwise our mgmt-api queries are not logged into audit logging, meaning the log is not complete - and if audit log isn't logging everything, how trustworthy is it?

vanessagits commented 2 months ago

Any of the mgmt-api commands that correspond to Cassandra database commands with existing audit log entry type coud be mapped to the same audit log entry type? Taking example mgmt-api get endpoints, this would get mapped to audit log entry type SELECT. Does Cassandra CEP-38 (CQL Management API) potentially bring synergies here as well?

emerkle826 commented 2 months ago

Does Cassandra CEP-38 (CQL Management API) potentially bring synergies here as well?

It should, but I have not had much of a chance to really dig into that CEP unfortunately