diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index d0d3731463..0d4e532456 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -91,12 +91,6 @@ bool Analyzer::IsAnalyzer(const char* name) return strcmp(analyzer_mgr->GetComponentName(tag).c_str(), name) == 0; } -// Used in debugging output. -static string fmt_analyzer(Analyzer* a) - { - return string(a->GetAnalyzerName()) + fmt("[%d]", a->GetID()); - } - Analyzer::Analyzer(const char* name, Connection* conn) { Tag tag = analyzer_mgr->GetComponentTag(name); @@ -381,7 +375,11 @@ void Analyzer::ForwardEndOfData(bool orig) bool Analyzer::AddChildAnalyzer(Analyzer* analyzer, bool init) { - if ( HasChildAnalyzer(analyzer->GetAnalyzerTag()) ) + auto t = analyzer->GetAnalyzerTag(); + auto it = std::find(prevented.begin(), prevented.end(), t); + auto prevent = it != prevented.end(); + + if ( HasChildAnalyzer(t) || prevent ) { analyzer->Done(); delete analyzer; @@ -407,46 +405,67 @@ bool Analyzer::AddChildAnalyzer(Analyzer* analyzer, bool init) Analyzer* Analyzer::AddChildAnalyzer(Tag analyzer) { - if ( ! HasChildAnalyzer(analyzer) ) - { - Analyzer* a = analyzer_mgr->InstantiateAnalyzer(analyzer, conn); + if ( HasChildAnalyzer(analyzer) ) + return nullptr; - if ( a && AddChildAnalyzer(a) ) - return a; + auto it = std::find(prevented.begin(), prevented.end(), analyzer); + + if ( it != prevented.end() ) + return nullptr; + + Analyzer* a = analyzer_mgr->InstantiateAnalyzer(analyzer, conn); + + if ( a && AddChildAnalyzer(a) ) + return a; + + return nullptr; + } + +bool Analyzer::RemoveChild(const analyzer_list& children, ID id) + { + for ( const auto& i : children ) + { + if ( i->id != id ) + continue; + + if ( i->finished || i->removing ) + return false; + + DBG_LOG(DBG_ANALYZER, "%s disabling child %s", + fmt_analyzer(this).c_str(), fmt_analyzer(i).c_str()); + // We just flag it as being removed here but postpone + // actually doing that to later. Otherwise, we'd need + // to call Done() here, which then in turn might + // cause further code to be executed that may assume + // something not true because of a violation that + // triggered the removal in the first place. + i->removing = true; + return true; } - return 0; + return false; } -void Analyzer::RemoveChildAnalyzer(Analyzer* analyzer) +bool Analyzer::RemoveChildAnalyzer(ID id) { - LOOP_OVER_CHILDREN(i) - if ( *i == analyzer && ! (analyzer->finished || analyzer->removing) ) - { - DBG_LOG(DBG_ANALYZER, "%s disabling child %s", - fmt_analyzer(this).c_str(), fmt_analyzer(*i).c_str()); - // We just flag it as being removed here but postpone - // actually doing that to later. Otherwise, we'd need - // to call Done() here, which then in turn might - // cause further code to be executed that may assume - // something not true because of a violation that - // triggered the removal in the first place. - (*i)->removing = true; - return; - } + return RemoveChild(children, id) || RemoveChild(new_children, id); } -void Analyzer::RemoveChildAnalyzer(ID id) +bool Analyzer::Remove() { - LOOP_OVER_CHILDREN(i) - if ( (*i)->id == id && ! ((*i)->finished || (*i)->removing) ) - { - DBG_LOG(DBG_ANALYZER, "%s disabling child %s", - fmt_analyzer(this).c_str(), fmt_analyzer(*i).c_str()); - // See comment above. - (*i)->removing = true; - return; - } + assert(parent); + parent->RemoveChildAnalyzer(this); + return removing; + } + +void Analyzer::PreventChildren(Tag tag) + { + auto it = std::find(prevented.begin(), prevented.end(), tag); + + if ( it != prevented.end() ) + return; + + prevented.emplace_back(tag); } bool Analyzer::HasChildAnalyzer(Tag tag) diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index 141d420a82..bd959eb370 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -4,6 +4,7 @@ #define ANALYZER_ANALYZER_H #include +#include #include "Tag.h" @@ -318,6 +319,12 @@ public: */ bool IsFinished() const { return finished; } + /** + * Returns true if the analyzer has been flagged for removal and + * shouldn't be used anymore. + */ + bool Removing() const { return removing; } + /** * Returns the tag associated with the analyzer's type. */ @@ -349,19 +356,19 @@ public: /** * Adds a new child analyzer to the analyzer tree. If an analyzer of - * the same type already exists, the one passes in is silenty - * discarded. + * the same type already exists or is prevented, the one passed in is + * silently discarded. * * @param analyzer The ananlyzer to add. Takes ownership. - * @return false if analyzer type was already a child, else true. + * @return false if analyzer type was already a child or prevented, else true. */ bool AddChildAnalyzer(Analyzer* analyzer) { return AddChildAnalyzer(analyzer, true); } /** * Adds a new child analyzer to the analyzer tree. If an analyzer of - * the same type already exists, the one passes in is silenty - * discarded. + * the same type already exists or is prevented, the one passed in is + * silently discarded. * * @param tag The type of analyzer to add. * @return the new analyzer instance that was added. @@ -373,16 +380,30 @@ public: * child, in which case the method does nothing. * * @param analyzer The analyzer to remove. + * + * @return whether the child analyzer is scheduled for removal + * (and was not before). */ - void RemoveChildAnalyzer(Analyzer* analyzer); + bool RemoveChildAnalyzer(Analyzer* analyzer) + { return RemoveChildAnalyzer(analyzer->GetID()); } /** * Removes a child analyzer. It's ok for the analyzer to not to be a * child, in which case the method does nothing. * - * @param tag The type of analyzer to remove. + * @param id The type of analyzer to remove. + * + * @return whether the child analyzer is scheduled for removal + * (and was not before). */ - void RemoveChildAnalyzer(ID id); + virtual bool RemoveChildAnalyzer(ID id); + + /** + * Prevents an analyzer type from ever being added as a child. + * + * @param tag The type of analyzer to prevent. + */ + void PreventChildren(Tag tag); /** * Returns true if analyzer has a direct child of a given type. @@ -450,8 +471,10 @@ public: /** * Remove the analyzer form its parent. The analyzer must have a * parent associated with it. + * + * @return whether the analyzer is being removed */ - void Remove() { assert(parent); parent->RemoveChildAnalyzer(this); } + bool Remove(); /** * Appends a support analyzer to the current list. @@ -570,6 +593,13 @@ protected: friend class ::Connection; friend class tcp::TCP_ApplicationAnalyzer; + /** + * Return a string represantation of an analyzer, containing its name + * and ID. + */ + static string fmt_analyzer(const Analyzer* a) + { return string(a->GetAnalyzerName()) + fmt("[%d]", a->GetID()); } + /** * Associates a connection with this analyzer. Must be called if * using the default ctor. @@ -644,10 +674,10 @@ protected: void AppendNewChildren(); /** - * Returns true if the analyzer has been flagged for removal and - * shouldn't be used otherwise anymore. + * Returns true if the child analyzer is now scheduled to be + * removed (and was not before) */ - bool Removing() const { return removing; } + bool RemoveChild(const analyzer_list& children, ID id); private: // Internal method to eventually delete a child analyzer that's @@ -670,6 +700,7 @@ private: SupportAnalyzer* resp_supporters; analyzer_list new_children; + std::vector prevented; bool protocol_confirmed; diff --git a/src/analyzer/protocol/pia/PIA.cc b/src/analyzer/protocol/pia/PIA.cc index 8f5e23a1ce..1becfb2cb0 100644 --- a/src/analyzer/protocol/pia/PIA.cc +++ b/src/analyzer/protocol/pia/PIA.cc @@ -282,10 +282,11 @@ void PIA_TCP::ActivateAnalyzer(analyzer::Tag tag, const Rule* rule) return; } - if ( Parent()->HasChildAnalyzer(tag) ) + analyzer::Analyzer* a = Parent()->AddChildAnalyzer(tag); + + if ( ! a ) return; - analyzer::Analyzer* a = Parent()->AddChildAnalyzer(tag); a->SetSignature(rule); // We have two cases here: diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index 5be893fd8e..df4c149e89 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/src/analyzer/protocol/tcp/TCP.cc @@ -205,6 +205,15 @@ analyzer::Analyzer* TCP_Analyzer::FindChild(Tag arg_tag) return 0; } +bool TCP_Analyzer::RemoveChildAnalyzer(ID id) + { + auto rval = analyzer::TransportLayerAnalyzer::RemoveChildAnalyzer(id); + + if ( rval ) + return rval; + + return RemoveChild(packet_children, id); + } void TCP_Analyzer::EnableReassembly() { @@ -1209,8 +1218,28 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig, // Handle child_packet analyzers. Note: This happens *after* the // packet has been processed and the TCP state updated. - LOOP_OVER_GIVEN_CHILDREN(i, packet_children) - (*i)->NextPacket(len, data, is_orig, rel_data_seq, ip, caplen); + analyzer_list::iterator next; + + for ( auto i = packet_children.begin(); i != packet_children.end(); i = next ) + { + auto child = *i; + next = ++i; + + if ( child->IsFinished() || child->Removing() ) + { + --i; + + if ( child->Removing() ) + child->Done(); + + DBG_LOG(DBG_ANALYZER, "%s deleted child %s", + fmt_analyzer(this).c_str(), fmt_analyzer(child).c_str()); + packet_children.erase(i); + delete child; + } + else + child->NextPacket(len, data, is_orig, rel_data_seq, ip, caplen); + } if ( ! reassembling ) ForwardPacket(len, data, is_orig, rel_data_seq, ip, caplen); diff --git a/src/analyzer/protocol/tcp/TCP.h b/src/analyzer/protocol/tcp/TCP.h index 95ef5c72d7..be0be4da82 100644 --- a/src/analyzer/protocol/tcp/TCP.h +++ b/src/analyzer/protocol/tcp/TCP.h @@ -36,6 +36,7 @@ public: Analyzer* FindChild(ID id) override; Analyzer* FindChild(Tag tag) override; + bool RemoveChildAnalyzer(ID id) override; // True if the connection has closed in some sense, false otherwise. int IsClosed() const { return orig->did_close || resp->did_close; } diff --git a/src/zeek.bif b/src/zeek.bif index 71c49f1c9e..48e18b5fec 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -4228,11 +4228,18 @@ function file_mode%(mode: count%): string ## ## aid: The analyzer ID. ## +## err_if_no_conn: Emit an error message if the connection does not exit. +## +## prevent: Prevent the same analyzer type from being attached in the future. +## This is useful for preventing a the same analyzer from being +## automatically attached in the future, e.g. as a result of a +## DPD signature suddenly matching. +## ## Returns: True if the connection identified by *cid* exists and has analyzer -## *aid*. +## *aid* and it is scheduled for removal. ## ## .. zeek:see:: Analyzer::schedule_analyzer Analyzer::name -function disable_analyzer%(cid: conn_id, aid: count, err_if_no_conn: bool &default=T%) : bool +function disable_analyzer%(cid: conn_id, aid: count, err_if_no_conn: bool &default=T, prevent: bool &default=F%) : bool %{ Connection* c = sessions->FindConnection(cid); if ( ! c ) @@ -4249,8 +4256,11 @@ function disable_analyzer%(cid: conn_id, aid: count, err_if_no_conn: bool &defau return val_mgr->GetBool(0); } - a->Remove(); - return val_mgr->GetBool(1); + if ( prevent ) + a->Parent()->PreventChildren(a->GetAnalyzerTag()); + + auto rval = a->Remove(); + return val_mgr->GetBool(rval); %} ## Informs Zeek that it should skip any further processing of the contents of diff --git a/testing/btest/Baseline/bifs.disable_analyzer-early/out b/testing/btest/Baseline/bifs.disable_analyzer-early/out new file mode 100644 index 0000000000..6a5ffcae8d --- /dev/null +++ b/testing/btest/Baseline/bifs.disable_analyzer-early/out @@ -0,0 +1,6 @@ +proto confirm, Analyzer::ANALYZER_HTTP +T +http_request, GET, /style/enhanced.css +total http messages, { +[[orig_h=192.168.1.104, orig_p=1673/tcp, resp_h=63.245.209.11, resp_p=80/tcp]] = 1 +} diff --git a/testing/btest/Baseline/bifs.disable_analyzer-tcp-packet-children/out b/testing/btest/Baseline/bifs.disable_analyzer-tcp-packet-children/out new file mode 100644 index 0000000000..70cd4de8e9 --- /dev/null +++ b/testing/btest/Baseline/bifs.disable_analyzer-tcp-packet-children/out @@ -0,0 +1,2 @@ +triggered packets, [orig_h=192.168.1.104, orig_p=1673/tcp, resp_h=63.245.209.11, resp_p=80/tcp], 1, T +T diff --git a/testing/btest/Baseline/bifs.disable_analyzer/out b/testing/btest/Baseline/bifs.disable_analyzer/out new file mode 100644 index 0000000000..1d57ac49fd --- /dev/null +++ b/testing/btest/Baseline/bifs.disable_analyzer/out @@ -0,0 +1,6 @@ +proto confirm, Analyzer::ANALYZER_HTTP +http_request, GET, /style/enhanced.css +T +total http messages, { +[[orig_h=192.168.1.104, orig_p=1673/tcp, resp_h=63.245.209.11, resp_p=80/tcp]] = 1 +} diff --git a/testing/btest/bifs/disable_analyzer-early.zeek b/testing/btest/bifs/disable_analyzer-early.zeek new file mode 100644 index 0000000000..bab832a489 --- /dev/null +++ b/testing/btest/bifs/disable_analyzer-early.zeek @@ -0,0 +1,34 @@ +# @TEST-EXEC: zeek -b -r $TRACES/http/pipelined-requests.trace %INPUT >out +# @TEST-EXEC: btest-diff out + +@load base/protocols/http + +global msg_count: table[conn_id] of count &default=0; + +event protocol_confirmation(c: connection, atype: Analyzer::Tag, aid: count) &priority=10 + { + if ( atype != Analyzer::ANALYZER_HTTP ) + return; + + print "proto confirm", atype; + print disable_analyzer(c$id, aid, T, T); + } + +event http_request(c: connection, method: string, original_URI: string, unescaped_URI: string, version: string) + { + ++msg_count[c$id]; + print "http_request", method, original_URI; + } + +event http_reply(c: connection, version: string, code: count, reason: string) + { + ++msg_count[c$id]; + print "http_reply", code; + } + +event zeek_done() + { + print "total http messages", msg_count; + } + + diff --git a/testing/btest/bifs/disable_analyzer-tcp-packet-children.zeek b/testing/btest/bifs/disable_analyzer-tcp-packet-children.zeek new file mode 100644 index 0000000000..b719eb010f --- /dev/null +++ b/testing/btest/bifs/disable_analyzer-tcp-packet-children.zeek @@ -0,0 +1,16 @@ +# @TEST-EXEC: zeek -b -r $TRACES/http/pipelined-requests.trace %INPUT >out +# @TEST-EXEC: btest-diff out + +@load base/protocols/http + +event connection_established(c: connection) + { + set_current_conn_packets_threshold(c$id, 1, T); + } + +event conn_packets_threshold_crossed(c: connection, threshold: count, is_orig: bool) + { + print "triggered packets", c$id, threshold, is_orig; + set_current_conn_packets_threshold(c$id, threshold + 1, T); + print disable_analyzer(c$id, current_analyzer(), T); + } diff --git a/testing/btest/bifs/disable_analyzer.zeek b/testing/btest/bifs/disable_analyzer.zeek new file mode 100644 index 0000000000..d3abbe893e --- /dev/null +++ b/testing/btest/bifs/disable_analyzer.zeek @@ -0,0 +1,34 @@ +# @TEST-EXEC: zeek -b -r $TRACES/http/pipelined-requests.trace %INPUT >out +# @TEST-EXEC: btest-diff out + +@load base/protocols/http + +global msg_count: table[conn_id] of count &default=0; + +event protocol_confirmation(c: connection, atype: Analyzer::Tag, aid: count) &priority=10 + { + if ( atype != Analyzer::ANALYZER_HTTP ) + return; + + print "proto confirm", atype; + } + +event http_request(c: connection, method: string, original_URI: string, unescaped_URI: string, version: string) + { + ++msg_count[c$id]; + print "http_request", method, original_URI; + print disable_analyzer(c$id, current_analyzer(), T, T); + } + +event http_reply(c: connection, version: string, code: count, reason: string) + { + ++msg_count[c$id]; + print "http_reply", code; + } + +event zeek_done() + { + print "total http messages", msg_count; + } + +