diff --git a/aux/binpac b/aux/binpac index 35406f2055..31203a3458 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit 35406f20555a3978a7ad56f6f977004f6bf47f9b +Subproject commit 31203a3458baa6bdfc355e260a102f9402fbe3e2 diff --git a/testing/btest/Baseline/plugins.binpac-flowbuffer-frame-length/output b/testing/btest/Baseline/plugins.binpac-flowbuffer-frame-length/output new file mode 100644 index 0000000000..877e1d742f --- /dev/null +++ b/testing/btest/Baseline/plugins.binpac-flowbuffer-frame-length/output @@ -0,0 +1 @@ +foo_message, 22, 17, 224 diff --git a/testing/btest/Traces/mmsX.pcap b/testing/btest/Traces/mmsX.pcap new file mode 100644 index 0000000000..4029195b20 Binary files /dev/null and b/testing/btest/Traces/mmsX.pcap differ diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/.btest-ignore b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/.btest-ignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/CMakeLists.txt b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/CMakeLists.txt new file mode 100644 index 0000000000..a2e5f4687b --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/CMakeLists.txt @@ -0,0 +1,19 @@ + +project(Zeek-Plugin-Foo-FOO) + +cmake_minimum_required(VERSION 2.6.3) + +if ( NOT ZEEK_DIST ) + message(FATAL_ERROR "ZEEK_DIST not set") +endif () + +set(CMAKE_MODULE_PATH ${ZEEK_DIST}/cmake) + +include(ZeekPlugin) + +zeek_plugin_begin(Foo FOO) +zeek_plugin_cc(src/Plugin.cc) +zeek_plugin_cc(src/FOO.cc) +zeek_plugin_bif(src/foo.bif) +zeek_plugin_pac(src/foo.pac) +zeek_plugin_end() diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/__load__.zeek b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/__load__.zeek new file mode 100644 index 0000000000..a10fe855df --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/__load__.zeek @@ -0,0 +1 @@ +@load ./main diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/main.zeek b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/main.zeek new file mode 100644 index 0000000000..5448ff8967 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/scripts/main.zeek @@ -0,0 +1,11 @@ +module Foo; + +const ports = { 102/tcp }; + +redef likely_server_ports += { ports }; + +event zeek_init() &priority=5 + { + Analyzer::register_for_ports(Analyzer::ANALYZER_FOO, ports); + } + diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.cc b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.cc new file mode 100644 index 0000000000..ac5fd3fd19 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.cc @@ -0,0 +1,65 @@ +#include "FOO.h" + +#include "analyzer/protocol/tcp/TCP_Reassembler.h" + +#include "Reporter.h" + +#include "foo.bif.h" + +using namespace analyzer::FOO; + +FOO_Analyzer::FOO_Analyzer(Connection* c) : tcp::TCP_ApplicationAnalyzer("FOO", c) + { + interp = new binpac::FOO::FOO_Conn(this); + had_gap = false; + } + +FOO_Analyzer::~FOO_Analyzer() + { + delete interp; + } + +void FOO_Analyzer::Done() + { + tcp::TCP_ApplicationAnalyzer::Done(); + + interp->FlowEOF(true); + interp->FlowEOF(false); + } + +void FOO_Analyzer::EndpointEOF(bool is_orig) + { + tcp::TCP_ApplicationAnalyzer::EndpointEOF(is_orig); + interp->FlowEOF(is_orig); + } + +void FOO_Analyzer::DeliverStream(int len, const u_char* data, bool orig) + { + tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig); + + assert(TCP()); + if ( TCP()->IsPartial() ) + return; + + if ( had_gap ) + // If only one side had a content gap, we could still try to + // deliver data to the other side if the script layer can handle this. + return; + + try + { + interp->NewData(orig, data, data + len); + } + catch ( const binpac::Exception& e ) + { + printf("Exception: %s\n", e.c_msg()); + ProtocolViolation(fmt("Binpac exception: %s", e.c_msg())); + } + } + +void FOO_Analyzer::Undelivered(uint64_t seq, int len, bool orig) + { + tcp::TCP_ApplicationAnalyzer::Undelivered(seq, len, orig); + had_gap = true; + interp->NewGap(orig, len); + } diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.h b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.h new file mode 100644 index 0000000000..bbad61c992 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/FOO.h @@ -0,0 +1,37 @@ +#ifndef ANALYZER_PROTOCOL_FOO_FOO_H +#define ANALYZER_PROTOCOL_FOO_FOO_H + +#include "foo.bif.h" + +#include "analyzer/protocol/tcp/TCP.h" + +#include "foo_pac.h" + +namespace analyzer { namespace FOO { + +class FOO_Analyzer : public tcp::TCP_ApplicationAnalyzer { +public: + FOO_Analyzer(Connection* conn); + virtual ~FOO_Analyzer(); + + // Overriden from Analyzer. + virtual void Done(); + + virtual void DeliverStream(int len, const u_char* data, bool orig); + virtual void Undelivered(uint64_t seq, int len, bool orig); + + // Overriden from tcp::TCP_ApplicationAnalyzer. + virtual void EndpointEOF(bool is_orig); + + static analyzer::Analyzer* InstantiateAnalyzer(Connection* conn) + { return new FOO_Analyzer(conn); } + +protected: + binpac::FOO::FOO_Conn* interp; + bool had_gap; + +}; + +} } // namespace analyzer::FOO + +#endif diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/Plugin.cc b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/Plugin.cc new file mode 100644 index 0000000000..089389eea3 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/Plugin.cc @@ -0,0 +1,26 @@ +#include "plugin/Plugin.h" +#include "analyzer/Component.h" + +#include "FOO.h" + +namespace plugin { +namespace Foo_FOO { + +class Plugin : public plugin::Plugin { +public: + plugin::Configuration Configure() + { + AddComponent(new ::analyzer::Component("FOO", + ::analyzer::FOO::FOO_Analyzer::InstantiateAnalyzer)); + + plugin::Configuration config; + config.name = "FOO::Foo"; + config.description = "Foo Analyzer analyzer"; + config.version.major = 1; + config.version.minor = 0; + return config; + } +} plugin; + +} +} diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.bif b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.bif new file mode 100644 index 0000000000..6b41bfbb07 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.bif @@ -0,0 +1,4 @@ +module Foo; + +event foo_message%(c: connection, is_orig: bool, len: count, plen: count, ptype: count%); + diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.pac b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.pac new file mode 100644 index 0000000000..3c3c8b412b --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length-plugin/src/foo.pac @@ -0,0 +1,60 @@ +%include binpac.pac +%include bro.pac + +%extern{ + #include "foo.bif.h" +%} + +analyzer FOO withcontext { + connection: FOO_Conn; + flow: FOO_Flow; +}; + +# Our connection consists of two flows, one in each direction. +connection FOO_Conn(bro_analyzer: BroAnalyzer) { + upflow = FOO_Flow(true); + downflow = FOO_Flow(false); +}; + +type HDR = record { + version: uint8; + reserved: uint8; + len: uint16; +} &byteorder=bigendian; + +type FOO_PDU(is_orig: bool) = record { + hdr: HDR; + plen: uint8; + ptype: uint8; + something: bytestring &restofdata; +} &byteorder=bigendian, &length=hdr.len; + +# Now we define the flow: +flow FOO_Flow(is_orig: bool) { + + flowunit = FOO_PDU(is_orig) withcontext(connection, this); + # datagram = FOO_PDU(is_orig) withcontext(connection, this); + +}; + +refine flow FOO_Flow += { + function proc_foo_message(msg: FOO_PDU): bool + %{ + // printf("FOO %d %d\n", msg->hdr()->len(), msg->hdr_len()); + connection()->bro_analyzer()->ProtocolConfirmation(); + BifEvent::Foo::generate_foo_message( + connection()->bro_analyzer(), + connection()->bro_analyzer()->Conn(), + is_orig(), + msg->hdr()->len(), + msg->plen(), + msg->ptype()); + return true; + %} + +}; + +refine typeattr FOO_PDU += &let { + proc: bool = $context.flow.proc_foo_message(this); +}; + diff --git a/testing/btest/plugins/binpac-flowbuffer-frame-length.zeek b/testing/btest/plugins/binpac-flowbuffer-frame-length.zeek new file mode 100644 index 0000000000..a2dd22aa04 --- /dev/null +++ b/testing/btest/plugins/binpac-flowbuffer-frame-length.zeek @@ -0,0 +1,40 @@ +# This test exercises a previous bug in binpac flowbuffer frame length +# boundary checks: + +# Incremental flowbuffer parsing sought to first parse the "minimum header +# length" required to get the full frame length, possibly from a record +# field, but generating the logic to parse that field could greedily +# bundle in additional boundary-checks for all subsequent fields of +# known-size. +# +# E.g. for flowunit parsing of this: +# +# type HDR = record { +# version: uint8; +# reserved: uint8; +# len: uint16; +# } &byteorder=bigendian; +# +# type FOO_PDU(is_orig: bool) = record { +# hdr: HDR; +# plen: uint8; +# ptype: uint8; +# something: bytestring &restofdata; +# } &byteorder=bigendian, &length=hdr.len; + +# The flowbuffer was correctly seeking to buffer 4 bytes and parse the +# "hdr.len" field, but the generated parsing logic for "hdr.len" included +# a boundary check all the way up to include "plen" and "ptype". + +# This causes out-of-bounds exceptions to be thrown for inputs that should +# actually be possible to incrementally parse via flowbuffer. + +# @TEST-EXEC: ${DIST}/aux/zeek-aux/plugin-support/init-plugin -u . Foo FOO +# @TEST-EXEC: cp -r %DIR/binpac-flowbuffer-frame-length-plugin/* . +# @TEST-EXEC: ./configure --zeek-dist=${DIST} && make +# @TEST-EXEC: ZEEK_PLUGIN_PATH=`pwd` zeek -r $TRACES/mmsX.pcap %INPUT >output + +event Foo::foo_message(c: connection, is_orig: bool, len: count, plen: count, ptype: count) + { + print "foo_message", len, plen, ptype; + }