spicy-redis: Separate error replies from success

scripts/base/protocols/redis/main.zeek

@@ -22,8 +22,10 @@ export {
 		id: conn_id &log;
 		## The Redis command.
 		cmd: Command &log;
+		## If the command was successful. Only set if the server responded.
+		success: bool &log &optional;
 		## The reply for the command.
-		reply: ServerData &log &optional;
+		reply: ReplyData &log &optional;
 	};
 
 	## A default logging policy hook for the stream.
@@ -198,16 +200,9 @@ function reply_num(c: connection): count
 	return resp_num;
 	}
 
-event Redis::reply(c: connection, data: Redis::ServerData)
+# Logs up to and including the last seen command from the last reply
+function log_from(c: connection, previous_reply_num: count)
 	{
-	if ( ! c?$redis_state )
-		make_new_state(c);
-
-	local previous_reply_num = c$redis_state$current_reply;
-	c$redis_state$current_reply = reply_num(c);
-	set_state(c, F);
-
-	c$redis$reply = data;
 	# Log each of the pending replies to this point - we will not go
 	# back.
 	while ( previous_reply_num < c$redis_state$current_reply )
@@ -234,6 +229,34 @@ event Redis::reply(c: connection, data: Redis::ServerData)
 		}
 	}
 
+event Redis::reply(c: connection, data: ReplyData)
+	{
+	if ( ! c?$redis_state )
+		make_new_state(c);
+
+	local previous_reply_num = c$redis_state$current_reply;
+	c$redis_state$current_reply = reply_num(c);
+	set_state(c, F);
+
+	c$redis$reply = data;
+	c$redis$success = T;
+	log_from(c, previous_reply_num);
+	}
+
+event Redis::error(c: connection, data: ReplyData)
+	{
+	if ( ! c?$redis_state )
+		make_new_state(c);
+
+	local previous_reply_num = c$redis_state$current_reply;
+	c$redis_state$current_reply = reply_num(c);
+	set_state(c, F);
+
+	c$redis$reply = data;
+	c$redis$success = F;
+	log_from(c, previous_reply_num);
+	}
+
 hook finalize_redis(c: connection)
 	{
 	if ( c$redis_state$violation )

scripts/base/protocols/redis/spicy-events.zeek

@@ -52,12 +52,9 @@ export {
 		known: KnownCommand &optional;
 	};
 
-	## Generic server data returned from the server.
+	## A generic Redis reply from the client.
-	type ServerData: record {
+	type ReplyData: record {
-		## Did the server reply without erroring?
+		value: string &log &optional;
-		success: bool &log;
-		## The string response, if it was a simple string or error
-		data: string &log &optional;
 	};
 }
 
@@ -89,9 +86,18 @@ global auth_command: event(c: connection, command: AuthCommand);
 ## cmd: The command sent to the server.
 global command: event(c: connection, cmd: Command);
 
-## Generated for every response sent by the Redis server to the client.
+## Generated for every successful response sent by the Redis server to the
+## client.
 ##
 ## c: The connection.
 ##
 ## data: The server data sent to the client.
-global reply: event(c: connection, data: ServerData);
+global reply: event(c: connection, data: ReplyData);
+
+## Generated for every error response sent by the Redis server to the
+## client.
+##
+## c: The connection.
+##
+## data: The server data sent to the client.
+global error: event(c: connection, data: ReplyData);

src/analyzer/protocol/redis/CMakeLists.txt

@@ -1,5 +1,5 @@
 spicy_add_analyzer(
     NAME Redis
     PACKAGE_NAME spicy-redis
-    SOURCES resp.spicy resp.evt redis.spicy zeek_redis.spicy
+    SOURCES resp.spicy resp.evt redis.spicy
-    MODULES RESP Redis Zeek_Redis)
+    MODULES RESP Redis)

src/analyzer/protocol/redis/redis.spicy

@@ -367,3 +367,37 @@ public function make_auth(command: Command): Auth {
 public function is_auth(data: RESP::ClientData): bool {
     return data.command.known && *(data.command.known) == KnownCommand::AUTH && |data.command.raw| >= 2;
 }
+
+type ReplyData = struct {
+    value: optional<bytes>;
+};
+
+public function is_err(server_data: RESP::ServerData): bool {
+    return server_data.data?.simple_error || server_data.data?.bulk_error;
+}
+
+function bulk_string_content(bulk: RESP::BulkString): bytes {
+    if (bulk?.content)
+        return bulk.content;
+    else
+        return b"";
+}
+
+# Gets the server reply in a simpler form
+public function make_server_reply(data: RESP::ServerData): ReplyData {
+    local res: ReplyData = [$value = Null];
+    if (data.data?.simple_error)
+        res.value = data.data.simple_error.content;
+    else if (data.data?.bulk_error)
+        res.value = bulk_string_content(data.data.bulk_error);
+    else if (data.data?.simple_string)
+        res.value = data.data.simple_string.content;
+    else if (data.data?.bulk_string)
+        res.value = bulk_string_content(data.data.bulk_string);
+    else if (data.data?.verbatim_string)
+        res.value = bulk_string_content(data.data.verbatim_string);
+    else if (data.data?.boolean)
+        res.value = data.data.boolean.val ? b"T" : b"F";
+
+    return res;
+}

src/analyzer/protocol/redis/resp.evt

@@ -6,10 +6,8 @@ protocol analyzer Redis over TCP:
 
 import RESP;
 import Redis;
-import Zeek_Redis;
 
 export Redis::KnownCommand;
-export Zeek_Redis::ZeekServerData;
 
 on RESP::ClientData if ( Redis::is_set(self) ) -> event Redis::set_command($conn, Redis::make_set(self.command));
 on RESP::ClientData if ( Redis::is_get(self) ) -> event Redis::get_command($conn, Redis::make_get(self.command).key);
@@ -17,5 +15,6 @@ on RESP::ClientData if ( Redis::is_auth(self) ) -> event Redis::auth_command($co
 
 # All client data is a command
 on RESP::ClientData -> event Redis::command($conn, self.command);
-# Server data needs an event to attach request/responses
+
-on RESP::ServerData -> event Redis::reply($conn, Zeek_Redis::make_server_data(self));
+on RESP::ServerData if ( ! Redis::is_err(self) ) -> event Redis::reply($conn, Redis::make_server_reply(self));
+on RESP::ServerData if ( Redis::is_err(self) ) -> event Redis::error($conn, Redis::make_server_reply(self));

src/analyzer/protocol/redis/zeek_redis.spicy

@@ -1,23 +0,0 @@
-# See the file "COPYING" in the main distribution directory for copyright.
-#
-# Zeek-specific Redis handling
-
-module Zeek_Redis;
-
-import RESP;
-
-public type ZeekServerData = struct {
-    success: bool;
-    data: optional<bytes>;
-};
-
-public function make_server_data(data: RESP::ServerData): ZeekServerData {
-    local res: ZeekServerData = [$success = True, $data = Null];
-    if (data.data?.simple_error) {
-        res.success = False;
-        res.data = data.data.simple_error.content;
-    } else if (data.data?.simple_string) {
-        res.data = data.data.simple_string.content;
-    }
-    return res;
-}

testing/btest/Baseline/coverage.record-fields/out.default

@@ -605,11 +605,11 @@ connection {
              }
         * id: record conn_id, log=T, optional=F
             conn_id { ... }
-        * reply: record Redis::ServerData, log=T, optional=T
+        * reply: record Redis::ReplyData, log=T, optional=T
-            Redis::ServerData {
+            Redis::ReplyData {
-              * data: string, log=T, optional=T
+              * value: string, log=T, optional=T
-              * success: bool, log=T, optional=F
              }
+        * success: bool, log=T, optional=T
         * ts: time, log=T, optional=F
         * uid: string, log=T, optional=F
        }

testing/btest/Baseline/scripts.base.protocols.redis.almost-redis/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	53099	127.0.0.1	6379	AUTH	-	-	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	53099	127.0.0.1	6379	PING	-	-	T	OK

testing/btest/Baseline/scripts.base.protocols.redis.auth/output

@@ -2,9 +2,12 @@
 AUTH
 username: notauser
 password: notapassword
+Auth failed:, WRONGPASS invalid username-password pair or user is disabled.
 AUTH
 username: default
 password: defaultpassword
+Auth succeeded:, OK
 AUTH
 username: noone
 password: password
+Auth failed:, WRONGPASS invalid username-password pair or user is disabled.

testing/btest/Baseline/scripts.base.protocols.redis.client-reply-off-2conn/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	61211	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	::1	61212	::1	6379	PING	-	-	T	PONG

testing/btest/Baseline/scripts.base.protocols.redis.client-reply-off/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	60761	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	60761	::1	6379	CLIENT	-	-	-	-

testing/btest/Baseline/scripts.base.protocols.redis.client-skip-while-off/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56348	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56348	::1	6379	CLIENT	-	-	-	-

testing/btest/Baseline/scripts.base.protocols.redis.django-cloud/redis.log

@@ -5,11 +5,11 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_3	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_3	-	T	6
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_3	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_3	-	T	6
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	(empty)
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_1	1	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_2	2	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_3	6	T	OK
@@ -61,9 +61,9 @@ XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_49	608281864034267560872252163321295376887552831379210240000000000	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_50	30414093201713378043612608166064768844377641568960512000000000000	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_50	30414093201713378043612608166064768844377641568960512000000000000	T	OK
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	30414093201713378043612608166064768844377641568960512000000000000
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_50	-	T	30414093201713378043612608166064768844377641568960512000000000000
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_100	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	GET	:1:factorial_100	-	T	(empty)
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_1	1	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_2	2	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.4	50044	18.234.186.95	10625	SET	:1:factorial_3	6	T	OK

testing/btest/Baseline/scripts.base.protocols.redis.excessive-pipelining/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	57156	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	57156	::1	6379	PING	-	-	T	PONG

testing/btest/Baseline/scripts.base.protocols.redis.pipeline-with-quotes/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	63754	::1	6379	SET	key	"my value with spaces"	T	OK
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	63754	::1	6379	SET	key2	'my value with single quotes'	T	OK

testing/btest/Baseline/scripts.base.protocols.redis.pipelined-with-commands/redis.log

@@ -5,10 +5,10 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56731	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56731	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56731	::1	6379	SET	HI	3	T	OK
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56731	::1	6379	GET	HI	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	56731	::1	6379	GET	HI	-	T	3
 #close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.redis.pipelined/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	51122	::1	6379	PING	-	-	T	PONG
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	::1	51122	::1	6379	PING	-	-	T	PONG

testing/btest/Baseline/scripts.base.protocols.redis.pubsub/redis.log

@@ -5,7 +5,7 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	56162	127.0.0.1	6379	SUBSCRIBE	-	-	T	-
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	127.0.0.1	56163	127.0.0.1	6379	PUBLISH	-	-	T	-

testing/btest/Baseline/scripts.base.protocols.redis.stream/redis.log

@@ -5,10 +5,10 @@
 #unset_field	-
 #path	redis
 #open XXXX-XX-XX-XX-XX-XX
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	reply.success	reply.data
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd.name	cmd.key	cmd.value	success	reply.value
 #types	time	string	addr	port	addr	port	string	string	string	bool	string
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	1729622832637-0
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	1729622836953-0
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XADD	-	-	T	1729622840530-0
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	49992	127.0.0.1	6379	XRANGE	-	-	T	-
 #close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/redis/auth.zeek

@@ -16,3 +16,13 @@ event Redis::auth_command(c: connection, command: Redis::AuthCommand)
 
 	print fmt("password: %s", command$password);
 	}
+
+event Redis::reply(c: connection, data: Redis::ReplyData)
+	{
+	print "Auth succeeded:", data$value;
+	}
+
+event Redis::error(c: connection, data: Redis::ReplyData)
+	{
+	print "Auth failed:", data$value;
+	}

testing/btest/scripts/base/protocols/redis/start-with-server.zeek

@@ -11,7 +11,12 @@ event Redis::command(c: connection, command: Redis::Command)
 	print "BAD", command;
 	}
 
-event Redis::reply(c: connection, dat: Redis::ServerData)
+event Redis::reply(c: connection, dat: Redis::ReplyData)
+	{
+	print "BAD", dat;
+	}
+
+event Redis::error(c: connection, dat: Redis::ReplyData)
 	{
 	print "BAD", dat;
 	}