redis: Add authentication support v3

[AmazingPP]

Replaces: #13898

Changes since v2:

• Made authentication failure retry interval consistent with connection failure.

Changes since v1:

• Reworked async mode to use a state machine for connection and authentication.
• Authentication is now retried on failure.
• Handled reply == NULL in the authentication callback.
• Added a guard: if a Redis username is set without a password, ignore the username and log a warning.

Manual testing

I manually tested the changes in both sync and async modes across different scenarios. All expected results matched actual results.

Case	Scenario	Result
1	No auth	Success, logs written
2	Wrong password	Connection failed, error logged
3	Correct password	Success, logs written
4	Missing password	Connection failed, error logged
5	ACL (valid)	Success, logs written
6	ACL (wrong password)	Connection failed, error logged
7	ACL (no username provided)	Connection failed, error logged
8	Re-authentication	Connection recovered, logs written after re-auth

Contribution style:

• I have read the contributing guide lines at
  https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html

Our Contribution agreements:

• I have signed the Open Information Security Foundation contribution agreement at
  https://suricata.io/about/contribution-agreement/ (note: this is only required once)

Changes (if applicable):

• I have updated the User Guide (in doc/userguide/) to reflect the changes made
• I have updated the JSON schema (in etc/schema.json) to reflect all logging changes
  (including schema descriptions)
• I have created a ticket at
  https://redmine.openinfosecfoundation.org/projects/suricata/issues

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7062

Describe changes:

• Add authentication support to the Redis logging output.
• Authentication configuration defaults to null for backward compatibility.

[jasonish]

The async behavior with respect to reconnecting does seem OK. However, there does appear to be memory leak on authentication failure:

Indirect leak of 464 byte(s) in 1 object(s) allocated from:
    #0 0x7f955771fa45 in realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:81
    #1 0x7f9557d1fb55 in redisAsyncConnectWithOptions (/usr/lib/libhiredis.so.1.3.0+0xdb55) (BuildId: 383a1070a76ec29d0aa8be9d86d0c46764be928a)
    #2 0x7f9557d1fca3 in redisAsyncConnect (/usr/lib/libhiredis.so.1.3.0+0xdca3) (BuildId: 383a1070a76ec29d0aa8be9d86d0c46764be928a)
    #3 0x5558b3bc4c25 in SCConfLogReopenAsyncRedis /home/jason/oisf/dev/suricata/review/src/util-log-redis.c:277
    #4 0x5558b3bc522c in SCLogRedisWriteAsync /home/jason/oisf/dev/suricata/review/src/util-log-redis.c:336
    #5 0x5558b3bc68d6 in LogFileWriteRedis /home/jason/oisf/dev/suricata/review/src/util-log-redis.c:551
    #6 0x5558b3bcdbd8 in LogFileWrite /home/jason/oisf/dev/suricata/review/src/util-logopenfile.c:993
    #7 0x5558b3a377b6 in OutputJSONBuffer /home/jason/oisf/dev/suricata/review/src/output-json.c:1005
    #8 0x5558b3a2e7d2 in JsonStatsLogger /home/jason/oisf/dev/suricata/review/src/output-json-stats.c:355
    #9 0x5558b3a39e77 in OutputStatsLog /home/jason/oisf/dev/suricata/review/src/output-stats.c:93
    #10 0x5558b352b275 in StatsOutput /home/jason/oisf/dev/suricata/review/src/counters.c:819
    #11 0x5558b35284f0 in StatsMgmtThread /home/jason/oisf/dev/suricata/review/src/counters.c:424
    #12 0x7f955765e11a in asan_thread_start /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:239
    #13 0x7f95570969ca  (/usr/lib/libc.so.6+0x969ca) (BuildId: 4fe011c94a88e8aeb6f2201b9eb369f42b4a1e9e)

to reproduce I started Redis with authentication required, but did not enter auth details into my Suricata.yaml and started Suricata. On exit, asan presented this leak.

When not async, and the authentication details are not configured we still appear to spam the console with authentication failure log messages.

[jasonish]

@AmazingPP Are you using the Redis support in any sort of production capacity? I'm wondering if keeping non-async suppport is worth it. Seems like a bad idea to block a thread for logging to a socket IMO.

[AmazingPP]

there does appear to be memory leak on authentication failure

Thanks — good catch. It seems that the exception handling branch in SCRedisAsyncEchoCommandCallback has set an incorrect state (REDIS_STATE_DISCONNECTED), resulting in the async context not being released correctly.

When not async, and the authentication details are not configured we still appear to spam the console with authentication failure log messages.

How about adding an ECHO command in SCConfLogReopenSyncRedis to verify the connection is actually ready before writing logs? — that should prevent the spam in sync mode.

Are you using the Redis support in any sort of production capacity? I'm wondering if keeping non-async suppport is worth it. Seems like a bad idea to block a thread for logging to a socket IMO.

I’m only using the async Redis support in production. The sync mode has noticeable performance issues when handling large log bursts under high traffic. Would you be open to deprecating or even removing the sync version entirely? Let me know what you think.

[AmazingPP]

Replaced by #14002