Merge remote-tracking branch 'origin/topic/johanna/ignore-checksums-nets'

* origin/topic/johanna/ignore-checksums-nets:
  Do not lookup ignore_checksums_nets for every packet
This commit is contained in:
Tim Wojtulewicz 2021-08-06 13:29:30 -07:00
commit cdfa50ddec
15 changed files with 114 additions and 8 deletions

12
CHANGES
View file

@ -1,3 +1,15 @@
4.2.0-dev.70 | 2021-08-06 13:29:30 -0700
* Do not lookup ignore_checksums_nets for every packet (Johanna Amann, Corelight)
This could lead to a noticeable (single-percent) performance
improvement.
Most of the functionality for this is in the packet analyzers that now
cache ignore_chesksums_nets.
Based on a patch by Arne Welzel (Corelight).
4.2.0-dev.68 | 2021-08-04 09:57:32 +0100
* Use unordered_map to store sessions for performance reasons. This might lead to an 8-9% speedup of Zeek.

View file

@ -1 +1 @@
4.2.0-dev.68
4.2.0-dev.70

View file

@ -25,6 +25,13 @@ type string_any_table: table[string] of any;
## directly and then remove this alias.
type string_set: set[string];
## A set of subnets.
##
## .. todo:: We need this type definition only for declaring builtin functions
## via ``bifcl``. We should extend ``bifcl`` to understand composite types
## directly and then remove this alias.
type subnet_set: set[subnet];
## A set of addresses.
##
## .. todo:: We need this type definition only for declaring builtin functions

View file

@ -9,6 +9,14 @@ const IPPROTO_IPIP : count = 4;
const IPPROTO_IPV6 : count = 41;
const IPPROTO_GRE : count = 47;
function analyzer_option_change_ignore_checksums_nets(ID: string, new_value: set[subnet], location: string) : set[subnet]
{
if ( ID == "ignore_checksums_nets" )
PacketAnalyzer::__set_ignore_checksums_nets(new_value);
return new_value;
}
event zeek_init() &priority=20
{
PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_IP, IPPROTO_IPIP, PacketAnalyzer::ANALYZER_IPTUNNEL);
@ -19,4 +27,6 @@ event zeek_init() &priority=20
PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_IP, IPPROTO_UDP, PacketAnalyzer::ANALYZER_UDP);
PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_IP, IPPROTO_ICMP, PacketAnalyzer::ANALYZER_ICMP);
PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_IP, IPPROTO_ICMP6, PacketAnalyzer::ANALYZER_ICMP);
Option::set_change_handler("ignore_checksums_nets", analyzer_option_change_ignore_checksums_nets, 5);
}

View file

@ -4,6 +4,7 @@ module PacketAnalyzer;
#include "zeek/packet_analysis/Analyzer.h"
#include "zeek/packet_analysis/Manager.h"
#include "zeek/packet_analysis/protocol/ip/IPBasedAnalyzer.h"
%%}
@ -47,3 +48,13 @@ function try_register_packet_analyzer_by_name%(parent: string, identifier: count
parent_analyzer->RegisterProtocol(identifier, child_analyzer);
return zeek::val_mgr->True();
%}
## Internal function that is used to update the core-mirror of the script-level `ignore_checksums_nets` variable.
function PacketAnalyzer::__set_ignore_checksums_nets%(v: subnet_set%) : bool
%{
if ( v->GetType()->Tag() != zeek::TYPE_TABLE )
zeek::emit_builtin_error("update_ignore_checksums_net() requires a table/set argument");
zeek::packet_analysis::IP::IPBasedAnalyzer::SetIgnoreChecksumsNets(zeek::IntrusivePtr{zeek::NewRef{}, v->AsTableVal()});
return zeek::val_mgr->True();
%}

View file

@ -77,7 +77,7 @@ void ICMPAnalyzer::DeliverPacket(Connection* c, double t, bool is_orig, int rema
const std::unique_ptr<IP_Hdr>& ip = pkt->ip_hdr;
if ( ! zeek::detail::ignore_checksums &&
! zeek::id::find_val<TableVal>("ignore_checksums_nets")->Contains(ip->IPHeaderSrcAddr()) &&
! GetIgnoreChecksumsNets()->Contains(ip->IPHeaderSrcAddr()) &&
remaining >= len )
{
int chksum = 0;

View file

@ -1,6 +1,7 @@
// See the file "COPYING" in the main distribution directory for copyright.
#include "zeek/packet_analysis/protocol/ip/IP.h"
#include "zeek/packet_analysis/protocol/ip/IPBasedAnalyzer.h"
#include "zeek/NetVar.h"
#include "zeek/IP.h"
#include "zeek/Discard.h"
@ -128,7 +129,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
return false;
if ( ! packet->l2_checksummed && ! detail::ignore_checksums && ip4 &&
! zeek::id::find_val<TableVal>("ignore_checksums_nets")->Contains(packet->ip_hdr->IPHeaderSrcAddr()) &&
! IPBasedAnalyzer::GetIgnoreChecksumsNets()->Contains(packet->ip_hdr->IPHeaderSrcAddr()) &&
detail::in_cksum(reinterpret_cast<const uint8_t*>(ip4), ip_hdr_len) != 0xffff )
{
Weird("bad_IP_checksum", packet);

View file

@ -282,3 +282,18 @@ void IPBasedAnalyzer::DumpPortDebug()
DBG_LOG(DBG_ANALYZER, " %d/%s: %s", mapping.first, transport_proto_string(transport), s.c_str());
}
}
TableValPtr IPBasedAnalyzer::ignore_checksums_nets_table = nullptr;
void IPBasedAnalyzer::SetIgnoreChecksumsNets(TableValPtr t)
{
IPBasedAnalyzer::ignore_checksums_nets_table = t;
}
TableValPtr IPBasedAnalyzer::GetIgnoreChecksumsNets()
{
if ( ! IPBasedAnalyzer::ignore_checksums_nets_table )
IPBasedAnalyzer::ignore_checksums_nets_table = zeek::id::find_val<TableVal>("ignore_checksums_nets");
return IPBasedAnalyzer::ignore_checksums_nets_table;
}

View file

@ -7,6 +7,7 @@
#include "zeek/packet_analysis/Analyzer.h"
#include "zeek/analyzer/Tag.h"
#include "zeek/ID.h"
namespace zeek::analyzer::pia { class PIA; }
@ -61,6 +62,25 @@ public:
*/
void DumpPortDebug();
/**
* Updates the internal pointer to the script-level variable `ignore_checksums_nets`.
* This is used to prevent repeated (costly) lookup of the script-level variable
* by IP-based analyzers.
*
* @param t New value of ignore_checksums_nets
*/
static void SetIgnoreChecksumsNets(TableValPtr t);
/**
* Gets the interpal pointer to the script-level variable `ignore_checksums_nets`.
* This is used to prevent repeated (costly) lookup of the script-level variable
* by IP-based analyzers.
*
* @return Current value of `ignore_checksums_nets`.
*/
static TableValPtr GetIgnoreChecksumsNets();
protected:
/**
@ -178,6 +198,7 @@ private:
TransportProto transport;
uint32_t server_port_mask;
static TableValPtr ignore_checksums_nets_table;
};
}

View file

@ -18,7 +18,6 @@ TCPAnalyzer::TCPAnalyzer() : IPBasedAnalyzer("TCP", TRANSPORT_TCP, TCP_PORT_MASK
void TCPAnalyzer::Initialize()
{
ignored_nets = zeek::id::find_val<TableVal>("ignore_checksums_nets");
}
SessionAdapter* TCPAnalyzer::MakeSessionAdapter(Connection* conn)
@ -164,7 +163,7 @@ bool TCPAnalyzer::ValidateChecksum(const IP_Hdr* ip, const struct tcphdr* tp,
{
if ( ! run_state::current_pkt->l3_checksummed &&
! detail::ignore_checksums &&
! ignored_nets->Contains(ip->IPHeaderSrcAddr()) &&
! GetIgnoreChecksumsNets()->Contains(ip->IPHeaderSrcAddr()) &&
caplen >= len && ! endpoint->ValidChecksum(tp, len, ip->IP4_Hdr()) )
{
adapter->Weird("bad_TCP_checksum");

View file

@ -86,8 +86,6 @@ private:
bool ValidateChecksum(const IP_Hdr* ip, const struct tcphdr* tp,
analyzer::tcp::TCP_Endpoint* endpoint,
int len, int caplen, TCPSessionAdapter* adapter);
TableValPtr ignored_nets;
};
}

View file

@ -108,7 +108,7 @@ void UDPAnalyzer::DeliverPacket(Connection* c, double t, bool is_orig, int remai
auto validate_checksum =
! run_state::current_pkt->l3_checksummed &&
! zeek::detail::ignore_checksums &&
! zeek::id::find_val<TableVal>("ignore_checksums_nets")->Contains(ip->IPHeaderSrcAddr()) &&
! GetIgnoreChecksumsNets()->Contains(ip->IPHeaderSrcAddr()) &&
remaining >=len;
constexpr auto vxlan_len = 8;

View file

@ -0,0 +1,11 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
#separator \x09
#set_separator ,
#empty_field (empty)
#unset_field -
#path weird
#open XXXX-XX-XX-XX-XX-XX
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer source
#types time string addr port addr port string string bool string string
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.168.1.28 53246 35.221.46.9 80 bad_TCP_checksum - F zeek TCP
#close XXXX-XX-XX-XX-XX-XX

View file

@ -584,6 +584,7 @@
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (default_file_bof_buffer_size, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (default_file_timeout_interval, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (ignore_checksums_nets, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (ignore_checksums_nets, PacketAnalyzer::IP::analyzer_option_change_ignore_checksums_nets{ if (ignore_checksums_nets == PacketAnalyzer::IP::ID) PacketAnalyzer::__set_ignore_checksums_nets(PacketAnalyzer::IP::new_value)return (PacketAnalyzer::IP::new_value)}, 5)) -> <no result>
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (udp_content_delivery_ports_use_resp, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
0.000000 MetaHookPost CallFunction(Option::set_change_handler, <frame>, (udp_content_ports, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
0.000000 MetaHookPost CallFunction(PacketAnalyzer::register_packet_analyzer, <frame>, (PacketAnalyzer::ANALYZER_ETHERNET, 2048, PacketAnalyzer::ANALYZER_IP)) -> <no result>
@ -1635,6 +1636,7 @@
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (default_file_bof_buffer_size, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (default_file_timeout_interval, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (ignore_checksums_nets, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (ignore_checksums_nets, PacketAnalyzer::IP::analyzer_option_change_ignore_checksums_nets{ if (ignore_checksums_nets == PacketAnalyzer::IP::ID) PacketAnalyzer::__set_ignore_checksums_nets(PacketAnalyzer::IP::new_value)return (PacketAnalyzer::IP::new_value)}, 5))
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (udp_content_delivery_ports_use_resp, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
0.000000 MetaHookPre CallFunction(Option::set_change_handler, <frame>, (udp_content_ports, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
0.000000 MetaHookPre CallFunction(PacketAnalyzer::register_packet_analyzer, <frame>, (PacketAnalyzer::ANALYZER_ETHERNET, 2048, PacketAnalyzer::ANALYZER_IP))
@ -2685,6 +2687,7 @@
0.000000 | HookCallFunction Option::set_change_handler(default_file_bof_buffer_size, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
0.000000 | HookCallFunction Option::set_change_handler(default_file_timeout_interval, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
0.000000 | HookCallFunction Option::set_change_handler(ignore_checksums_nets, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
0.000000 | HookCallFunction Option::set_change_handler(ignore_checksums_nets, PacketAnalyzer::IP::analyzer_option_change_ignore_checksums_nets{ if (ignore_checksums_nets == PacketAnalyzer::IP::ID) PacketAnalyzer::__set_ignore_checksums_nets(PacketAnalyzer::IP::new_value)return (PacketAnalyzer::IP::new_value)}, 5)
0.000000 | HookCallFunction Option::set_change_handler(udp_content_delivery_ports_use_resp, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
0.000000 | HookCallFunction Option::set_change_handler(udp_content_ports, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
0.000000 | HookCallFunction PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_ETHERNET, 2048, PacketAnalyzer::ANALYZER_IP)

View file

@ -0,0 +1,18 @@
# @TEST-DOC: Use Config::set_value() to clear ignore_checksums_nets after having received a few packets. Expect a bad_TCP_checksum weird.log entry due to the following packets.
# @TEST-EXEC: zeek -b -r $TRACES/chksums/localhost-bad-chksum.pcap "ignore_checksums_nets += {192.168.0.0/16}" %INPUT
# @TEST-EXEC: btest-diff weird.log
@load base/frameworks/config
@load base/frameworks/notice
global packet_counter = 0;
event new_packet(c: connection, p: pkt_hdr)
{
++packet_counter;
if ( packet_counter > 3 )
{
local s: set[subnet] = set();
Config::set_value("ignore_checksums_nets", s);
}
}