From f2aca331ecfc40718484e7984af974d4fc7c2fb5 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 30 May 2025 07:50:08 -0700 Subject: [PATCH] Redis: Add support for sending AUTH commands during connection --- .../storage/backend/redis/main.zeek | 7 ++ src/storage/backend/redis/Redis.cc | 94 ++++++++++++++++--- src/storage/backend/redis/Redis.h | 5 + .../out | 4 + .../out | 4 +- .../out | 4 +- .../base/frameworks/storage/redis-auth.zeek | 40 ++++++++ testing/scripts/run-redis-server | 29 ++++-- 8 files changed, 161 insertions(+), 26 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.storage.redis-auth/out create mode 100644 testing/btest/scripts/base/frameworks/storage/redis-auth.zeek diff --git a/scripts/policy/frameworks/storage/backend/redis/main.zeek b/scripts/policy/frameworks/storage/backend/redis/main.zeek index cf130d2b4e..77196a4a58 100644 --- a/scripts/policy/frameworks/storage/backend/redis/main.zeek +++ b/scripts/policy/frameworks/storage/backend/redis/main.zeek @@ -39,6 +39,13 @@ export { ## Timeout for operation requests sent to the backend. Operations that ## exceed this time will return :zeek:see:`Storage::TIMEOUT`. operation_timeout: interval &default=default_operation_timeout; + + ## A username to use for authentication the server is protected by an ACL. + username: string &optional; + + ## A username to use for authentication the server is protected by an ACL + ## or by a simple password. + password: string &optional; }; } diff --git a/src/storage/backend/redis/Redis.cc b/src/storage/backend/redis/Redis.cc index 578b4a61b4..4b2fae68ab 100644 --- a/src/storage/backend/redis/Redis.cc +++ b/src/storage/backend/redis/Redis.cc @@ -136,6 +136,19 @@ void redisINFO(redisAsyncContext* ctx, void* reply, void* privdata) { backend->HandleInfoResult(static_cast(reply)); } +/** + * Callback handler for AUTH commands. + * + * @param ctx The async context that called this callback. + * @param reply The reply from the server for the command. + * @param privdata A pointer to private data passed in the command. + */ +void redisAUTH(redisAsyncContext* ctx, void* reply, void* privdata) { + auto t = Tracer("auth"); + auto backend = static_cast(ctx->data); + backend->HandleAuthResult(static_cast(reply)); +} + // Because we called redisPollAttach in DoOpen(), privdata here is a // redisPollEvents object. We can go through that object to get the context's // data, which contains the backend. Because we overrode these callbacks in @@ -271,6 +284,14 @@ OperationResult Redis::DoOpen(OpenResultCallback* cb, RecordValPtr options) { struct timeval timeout = util::double_to_timeval(connect_timeout_opt); opt.connect_timeout = &timeout; + auto username_field = backend_options->GetField("username"); + if ( username_field ) + username = username_field->ToStdString(); + + auto password_field = backend_options->GetField("password"); + if ( password_field ) + password = password_field->ToStdString(); + // The connection request below should be operation #1. active_ops = 1; @@ -642,25 +663,74 @@ void Redis::HandleInfoResult(redisReply* reply) { CompleteCallback(open_cb, res); } +void Redis::HandleAuthResult(redisReply* reply) { + DBG_LOG(DBG_STORAGE, "Redis backend: auth event"); + --active_ops; + + if ( strncmp(reply->str, "OK", 2) != 0 ) { + std::string reason = util::fmt("AUTH command failed to authenticate: %s", reply->str); + CompleteCallback(open_cb, {ReturnCode::CONNECTION_FAILED, reason}); + freeReplyObject(reply); + + disconnect_reason = reason; + redisAsyncDisconnect(async_ctx); + return; + } + + freeReplyObject(reply); + SendInfoRequest(); +} + +void Redis::SendInfoRequest() { + DBG_LOG(DBG_STORAGE, "Redis backend: Sending INFO request"); + + // Request the INFO block from the server that should contain the version information. + int status = redisAsyncCommand(async_ctx, redisINFO, NULL, "INFO server"); + + if ( status == REDIS_ERR ) { + // TODO: do something with the error? + DBG_LOG(DBG_STORAGE, "INFO command failed: %s err=%d", async_ctx->errstr, async_ctx->err); + CompleteCallback(open_cb, {ReturnCode::OPERATION_FAILED, + util::fmt("INFO command failed to retrieve server info: %s", async_ctx->errstr)}); + return; + } + + ++active_ops; +} + void Redis::OnConnect(int status) { DBG_LOG(DBG_STORAGE, "Redis backend: connection event, status=%d", status); --active_ops; connected = false; if ( status == REDIS_OK ) { - // Request the INFO block from the server that should contain the version information. - status = redisAsyncCommand(async_ctx, redisINFO, NULL, "INFO server"); + bool made_auth_request = false; + // If the username and/or password are set, send an AUTH command. Fail to + // connect if the authentication fails. We want to pause here while opening. + if ( ! username.empty() && ! password.empty() ) { + status = redisAsyncCommand(async_ctx, redisAUTH, NULL, "AUTH %s %s", username.c_str(), password.c_str()); + made_auth_request = true; + } + else if ( ! password.empty() ) { + status = redisAsyncCommand(async_ctx, redisAUTH, NULL, "AUTH %s", password.c_str()); + made_auth_request = true; + } + + // This will be reset by the sync calls above if we make them and they fail. The + // condition will always be false if we don't make any auth call. if ( status == REDIS_ERR ) { - // TODO: do something with the error? - DBG_LOG(DBG_STORAGE, "INFO command failed: %s err=%d", async_ctx->errstr, async_ctx->err); - CompleteCallback(open_cb, - {ReturnCode::OPERATION_FAILED, - util::fmt("INFO command failed to retrieve server info: %s", async_ctx->errstr)}); + DBG_LOG(DBG_STORAGE, "AUTH command failed: %s err=%d", async_ctx->errstr, async_ctx->err); + CompleteCallback(open_cb, {ReturnCode::OPERATION_FAILED, + util::fmt("AUTH command failed to queue: %s", async_ctx->errstr)}); return; } - ++active_ops; + if ( made_auth_request ) + ++active_ops; + else + // This will be called from the handler of the auth event if that succeeds. + SendInfoRequest(); } else { DBG_LOG(DBG_STORAGE, "Redis backend: connection failed: %s err=%d", async_ctx->errstr, async_ctx->err); @@ -682,11 +752,9 @@ void Redis::OnDisconnect(int status) { } else { --active_ops; - - if ( disconnect_reason.empty() ) - EnqueueBackendLost("Client disconnected"); - else - EnqueueBackendLost(util::fmt("Client disconnected: %s", disconnect_reason.c_str())); + std::string msg = + util::fmt("Client disconnected%s%s", disconnect_reason.empty() ? "" : ": ", disconnect_reason.c_str()); + EnqueueBackendLost(msg); if ( close_cb ) CompleteCallback(close_cb, {ReturnCode::SUCCESS}); diff --git a/src/storage/backend/redis/Redis.h b/src/storage/backend/redis/Redis.h index a0110dd55d..fc699275b8 100644 --- a/src/storage/backend/redis/Redis.h +++ b/src/storage/backend/redis/Redis.h @@ -42,6 +42,7 @@ public: void HandleEraseResult(redisReply* reply, ResultCallback* callback); void HandleGeneric(redisReply* reply); void HandleInfoResult(redisReply* reply); + void HandleAuthResult(redisReply* reply); /** * Returns whether the backend is opened. @@ -63,6 +64,8 @@ private: OperationResult ParseReplyError(std::string_view op_str, std::string_view reply_err_str) const; OperationResult CheckServerVersion(); + void SendInfoRequest(); + redisAsyncContext* async_ctx = nullptr; // When running in sync mode, this is used to keep a queue of replies as @@ -77,6 +80,8 @@ private: std::string server_addr; std::string key_prefix; std::string disconnect_reason; + std::string username; + std::string password; std::atomic connected = false; std::atomic expire_running = false; diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-auth/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-auth/out new file mode 100644 index 0000000000..1e4bb61332 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-auth/out @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +open 1, [code=Storage::CONNECTION_FAILED, error_str=AUTH command failed to authenticate: WRONGPASS invalid username-password pair or user is disabled., value=] +open 2, [code=Storage::SUCCESS, error_str=, value=] +close, [code=Storage::SUCCESS, error_str=, value=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-disconnect/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-disconnect/out index b4f1618694..b4e7d3c209 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-disconnect/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-disconnect/out @@ -1,4 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. open_result, [code=Storage::SUCCESS, error_str=, value=] -Storage::backend_opened, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs]] -Storage::backend_lost, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs]], Server closed the connection +Storage::backend_opened, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs, acl_username=, password=]] +Storage::backend_lost, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs, acl_username=, password=]], Server closed the connection diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out index f50c10b828..edc4084d7c 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out @@ -9,5 +9,5 @@ get result same as originally inserted, T put result, [code=Storage::SUCCESS, error_str=, value=] get result, [code=Storage::SUCCESS, error_str=, value=value2345] get result same as overwritten, T -Storage::backend_opened, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs]] -Storage::backend_lost, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs]], Client disconnected +Storage::backend_opened, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs, acl_username=, password=]] +Storage::backend_lost, Storage::STORAGE_BACKEND_REDIS, [serializer=Storage::STORAGE_SERIALIZER_JSON, redis=[server_host=127.0.0.1, server_port=xxxx/tcp, server_unix_socket=, key_prefix=testing, connect_timeout=5.0 secs, operation_timeout=5.0 secs, acl_username=, password=]], Client disconnected diff --git a/testing/btest/scripts/base/frameworks/storage/redis-auth.zeek b/testing/btest/scripts/base/frameworks/storage/redis-auth.zeek new file mode 100644 index 0000000000..e28651e0b6 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/storage/redis-auth.zeek @@ -0,0 +1,40 @@ +# @TEST-DOC: Tests basic Redis storage backend functions in async mode + +# @TEST-REQUIRES: have-redis +# @TEST-PORT: REDIS_PORT + +# @TEST-EXEC: btest-bg-run redis-server run-redis-server ${REDIS_PORT%/tcp} testpassword +# @TEST-EXEC: zeek -b %INPUT > out +# @TEST-EXEC: btest-bg-wait -k 0 + +# @TEST-EXEC: btest-diff out + +@load base/frameworks/storage/sync +@load policy/frameworks/storage/backend/redis + +event zeek_init() + { + local opts: Storage::BackendOptions; + opts$redis = [ $server_host="127.0.0.1", $server_port=to_port(getenv( + "REDIS_PORT")), $key_prefix="testing", $password="notthepassword" ]; + + local key = "key1234"; + local value = "value5678"; + + # This should fail because the password doesn't match. + local res = Storage::Sync::open_backend(Storage::STORAGE_BACKEND_REDIS, opts, string, string); + print "open 1", res; + if ( res$code == Storage::SUCCESS ) + return; + + opts$redis$password = "testpassword"; + res = Storage::Sync::open_backend(Storage::STORAGE_BACKEND_REDIS, opts, string, string); + print "open 2", res; + + if ( res$code != Storage::SUCCESS ) + return; + + local backend = res$value; + res = Storage::Sync::close_backend(backend); + print "close", res; + } diff --git a/testing/scripts/run-redis-server b/testing/scripts/run-redis-server index ae11c36b13..f91af2be68 100755 --- a/testing/scripts/run-redis-server +++ b/testing/scripts/run-redis-server @@ -5,17 +5,28 @@ if ! redis-server --version; then exit 1 fi -if [ $# -ne 1 ]; then - echo "Usage $0 " >2 +if [ $# -lt 1 ]; then + echo "Usage $0 ()" >2 exit 1 fi listen_port=$1 -exec redis-server \ - --bind 127.0.0.1 \ - --port ${listen_port} \ - --loglevel verbose \ - --logfile redis.log \ - --pidfile redis.pid \ - --databases 1 +if [ $# -eq 1 ]; then + exec redis-server \ + --bind 127.0.0.1 \ + --port ${listen_port} \ + --loglevel verbose \ + --logfile redis.log \ + --pidfile redis.pid \ + --databases 1 +elif [ $# -eq 2 ]; then + exec redis-server \ + --bind 127.0.0.1 \ + --port ${listen_port} \ + --loglevel verbose \ + --logfile redis.log \ + --pidfile redis.pid \ + --databases 1 \ + --requirepass $2 +fi