session/Manager: Emit explicit errors for FindConnection() with proto=65535

We silently broke users constructing conn_id records manually and
subsequently using them with lookup_connection() or connection_exists().

This is an attempt to at least report a runtime error about the situation
so it doesn't go completely unnoticed.
This commit is contained in:
Arne Welzel 2025-01-17 16:50:56 +01:00
parent ec0a85f553
commit 17836ef7d9
3 changed files with 76 additions and 1 deletions

View file

@ -9,6 +9,8 @@
#include <cstdlib>
#include "zeek/Conn.h"
#include "zeek/Func.h"
#include "zeek/IP.h"
#include "zeek/NetVar.h"
#include "zeek/Reporter.h"
#include "zeek/RuleMatcher.h"
@ -18,6 +20,7 @@
#include "zeek/packet_analysis/Manager.h"
#include "zeek/session/Session.h"
#include "zeek/telemetry/Manager.h"
#include "zeek/util.h"
zeek::session::Manager* zeek::session_mgr = nullptr;
@ -89,8 +92,15 @@ void Manager::Done() {}
Connection* Manager::FindConnection(Val* v) {
zeek::detail::ConnKey conn_key(v);
if ( ! conn_key.valid )
if ( ! conn_key.Valid() ) {
// Produce a loud error for invalid script-layer conn_id records.
const char* extra = "";
if ( conn_key.transport == UNKNOWN_IP_PROTO )
extra = ": the proto field has the \"unknown\" 65535 value. Did you forget to set it?";
zeek::emit_builtin_error(zeek::util::fmt("invalid connection ID record encountered%s", extra));
return nullptr;
}
return FindConnection(conn_key);
}

View file

@ -0,0 +1,7 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
1362692526.869344 error in <...>/lookup_connection.zeek, line 19: invalid connection ID record encountered: the proto field has the "unknown" 65535 value. Did you forget to set it? (lookup_connection(cid))
1362692526.869344 error in <...>/lookup_connection.zeek, line 19: connection ID not a known connection (lookup_connection(cid) and [orig_h=141.142.228.5, orig_p=59856/tcp, resp_h=192.150.187.43, resp_p=80/tcp, proto=65535])
1362692526.869344 error in <...>/lookup_connection.zeek, line 41: invalid connection ID record encountered: the proto field has the "unknown" 65535 value. Did you forget to set it? (connection_exists(my_id))
1362692526.869344 error in <...>/lookup_connection.zeek, line 51: invalid connection ID record encountered (lookup_connection(my_id))
1362692526.869344 error in <...>/lookup_connection.zeek, line 51: connection ID not a known connection (lookup_connection(my_id) and [orig_h=<uninitialized>, orig_p=<uninitialized>, resp_h=<uninitialized>, resp_p=<uninitialized>, proto=65535])
1362692526.869344 error in <...>/lookup_connection.zeek, line 57: invalid connection ID record encountered (connection_exists(my_id))

View file

@ -0,0 +1,58 @@
# @TEST-DOC: Test lookup_connection() and connection_exists()
#
# @TEST-EXEC: zeek -b -r $TRACES/http/get.trace %INPUT
# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
event new_connection(c: connection)
{
local c2 = lookup_connection(c$id);
assert c$uid == c2$uid;
local cid: conn_id;
cid$orig_h = c$id$orig_h;
cid$orig_p = c$id$orig_p;
cid$resp_h = c$id$resp_h;
cid$resp_p = c$id$resp_p;
# Produces an error on .stderr because cid$proto wasn't
# initialized and then returns a dummy record.
local c3 = lookup_connection(cid);
assert c3$history == "";
assert c3$id$orig_h == 0.0.0.0;
assert c3$id$orig_p == 0/udp;
cid$proto = c$id$proto;
local c4 = lookup_connection(cid);
assert c$uid == c4$uid;
}
event new_connection(c: connection)
{
# This needs to hold.
assert connection_exists(c$id);
local my_id: conn_id;
my_id$orig_h = c$id$orig_h;
my_id$orig_p = c$id$orig_p;
my_id$resp_h = c$id$resp_h;
my_id$resp_p = c$id$resp_p;
# Produces an error because cid$proto wasn't initialized.
assert ! connection_exists(my_id);
my_id$proto = c$id$proto;
assert connection_exists(my_id);
}
event new_connection(c: connection)
{
# This crashed previously!
local my_id: conn_id;
local c2 = lookup_connection(my_id);
assert c2$history == "";
assert c2$id$orig_h == 0.0.0.0;
assert c2$id$orig_p == 0/udp;
# This also crashed!
assert ! connection_exists(my_id);
}