spicy-redis: Abort parsing if server data comes first

Redis seems to only want client data first to request server data. The
DPD signature seems to pick up on some cases where server data comes
first, but is otherwise "valid" RESP. See if this helps lower FP rates.
This commit is contained in:
Evan Typanski 2025-01-29 09:52:56 -05:00
parent 90d56ce630
commit aef9fe11dc
4 changed files with 31 additions and 3 deletions

View file

@ -10,15 +10,29 @@ const MAX_SIZE = 1024 * 1024;
const MAX_RECURSION_DEPTH = 20; const MAX_RECURSION_DEPTH = 20;
public type ClientMessages = unit { public type ClientMessages = unit {
# The context here refers to whether we saw client data first. It is a one-time switch,
# either we have seen client data or not.
%context = bool;
on %init {
*self.context() = True;
}
: ClientData[]; : ClientData[];
}; };
public type ServerMessages = unit { public type ServerMessages = unit {
%context = bool;
on %init {
if (!*self.context()) {
throw "Server responses must come after a client request is seen";
}
}
: (ServerData &synchronize)[]; : (ServerData &synchronize)[];
}; };
public type ClientData = unit { public type ClientData = unit {
on %init() { self.start = self.input(); } on %init() {
self.start = self.input();
}
# Clients can only be an array or inline # Clients can only be an array or inline
ty: uint8 &convert=DataType($$) { ty: uint8 &convert=DataType($$) {
@ -49,7 +63,7 @@ type BulkStringArray = unit {
type BulkStringWithTy = unit { type BulkStringWithTy = unit {
# Need to consume the type here # Need to consume the type here
: uint8 &requires=$$=='$'; : uint8 &requires=$$ == '$';
length: RedisBytes &convert=$$.to_int(10) &requires=self.length <= int64(MAX_SIZE); length: RedisBytes &convert=$$.to_int(10) &requires=self.length <= int64(MAX_SIZE);
# NullBulkString is a BulkString with content unset # NullBulkString is a BulkString with content unset
@ -59,7 +73,6 @@ type BulkStringWithTy = unit {
: skip RedisBytes; : skip RedisBytes;
}; };
public type ServerData = unit { public type ServerData = unit {
%synchronize-after = b"\x0d\x0a"; %synchronize-after = b"\x0d\x0a";
var depth: uint8& = new uint8; var depth: uint8& = new uint8;

View file

@ -0,0 +1 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

Binary file not shown.

View file

@ -0,0 +1,14 @@
# @TEST-DOC: Test that Redis does not parse if it starts with the server data
#
# @TEST-EXEC: zeek -Cr $TRACES/redis/start-with-server.pcap base/protocols/redis %INPUT >output
# @TEST-EXEC: btest-diff output
event Redis::command(c: connection, is_orig: bool, command: Redis::Command)
{
print "BAD", command;
}
event Redis::server_data(c: connection, is_orig: bool, dat: Redis::ServerData)
{
print "BAD", dat;
}