[NEW] Log authentication failures with client address

[sonicloong]

The problem/use-case that the feature addresses

Failed password authentications could indicate there might be a client that used trial-and-error to guess the Redis password. Having logs with the client address for authentication failure events would help the server admin to detect if the client is potentially an attacker.

Description of the feature

Log authentication failures with client address.

Alternatives you've considered

ACL LOG provides recent ACL security events, including authentication failures. However these events will be lost when the Redis server crashes or is shut down. Also there are ways to push those events out or reset them. So having the events in server logs would be a better option for auditing purposes.

Additional information

Happy to submit a PR if this request makes sense.

[artikell]

Printing logs is a relatively cautious operation, and this proposal has a point to consider: how to determine the information that logs should record. If there are a large number of illegal requests, there may be a large number of logs, which will ultimately affect server storage.

In practical scenarios, a service is generally required to regularly read ACL LOG to record relevant information.

[sonicloong]

If there are a large number of illegal requests, there may be a large number of logs, which will ultimately affect server storage.

One approach to prevent flooding the log files is to rate-limit such logs. The idea is similar to

valkey/src/networking.c

Line 3687 in 7809df0

if (llabs(now-logged_time) > 60) {

ACL LOG has a size limit (acllog-max-len) and the events are not persisted, so attacks could still fly under the radar even when there's regular ACL LOG polling. For example if someone has access to user A, they can make a few brute-force authentication attempts to user B, then send massive commands that are not permitted to user A, in order to saturate the ACL LOG, so that the authentication failure events would be evicted, before the next ACL LOG polling happens. Another scenario would be if the malicious client eventually manages to guess the admin user's password, it can reset ACL LOG or shut down the server to wipe the events out.

In my opinion recording the auth failure events in server logs and ACL LOG serve for different purposes, and we can have both. Server logs can be used for automated monitoring and auditing on potential malicious events, while ACL LOG can be used for getting more details when such malicious events are detected.

[PingXie]

@sonicloong, have you considered implementing a metric-based approach instead?

I see a couple of issues with relying heavily on logging:

1. Generally, logging errors at the per-request level can inadvertently open up a DDoS attack vector, which contradicts the intent of enhancing security. I believe @artikell also hinted at this concern.
2. Currently, Valkey lacks structured logging support. The free-form log messages we currently utilize are cumbersome to handle. Additionally, analyzing these logs would necessitate extensive post-processing.

Here's what I suggest for a metric-based system, outlined at a high level:

1. Track the total number of authentication failures;
2. If needed, bucketize these failures by type, e.g., bad password, expired credentials, etc.;
3. For each bucket, if any, track the top N clients based on their frequency of errors;
4. Introduce a new ACL subcommand to report these metrics;
5. Ensure all tracking is optional and gated behind server configs.

This approach would provide a more structured and manageable way to monitor security-related events without overwhelming the system with log data.

[sonicloong]

@PingXie Great suggestions. I think what you proposed would provide useful insights in addition to what ACL LOG provides today, and I'd be happy to introduce a new ACL subcommand for doing that.

What I'd also like to propose is the option for writing auth event logs to disk for auditing purposes, in case the in-memory stats are reset by someone who manages to be authenticated to an admin user, or wiped out when the server is shut down. It would be great if Valkey can support certain level of audit logging, as many databases offer today.

Agree that structured logging would make logs easier to analyze. As for the DDoS concern, I believe it can be resolved by applying rate limiting on logging (there's an example I referred to in my previous comment), or only logging the first occurrence of identical events, keyed on (event type, client address).

To address the concerns, I think following are some requirements if Valkey plans to provide audit logging:

• There should be a config directive for users to enable/disable audit logging, disabled by default
• Auth events will be logged into a dedicated audit log file, whose path can be configured by users as well
• Audit logs in the file should be structured, and the structure needs to be defined
• Users should be able to configure the auditing options, such as event types, logging level, logging frequency (to mitigate DDoS impact), etc.

Server admins can then build monitoring service that watches audit log file changes and take actions.