Merge remote-tracking branch 'origin/topic/johanna/dpd-packet-limit'

* origin/topic/johanna/dpd-packet-limit:
  PIA - switch size to int64_t
  Introduce dpd_max_packets

CHANGES

@@ -1,3 +1,16 @@
+4.2.0-dev.201 | 2021-09-21 15:15:57 -0700
+
+  * PIA - switch size to int64_t (Johanna Amann, Corelight)
+
+    This brings the PIA size counter in line with the actual datatype used
+    on the scripting layer - both now use an int64_t.
+
+  * Introduce dpd_max_packets (Johanna Amann, Corelight)
+
+    dpd_max_packets is an additional setting that limits the maximum amount
+    of packets that dpd will cache; before dpd was only limited by buffer
+    size (but could cache a limitless amount of data-less packets).
+
 4.2.0-dev.198 | 2021-09-21 14:13:21 -0700
 
   * Use `brew update` instead of `brew update-reset` in CI macOS prepare script (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-4.2.0-dev.198
+4.2.0-dev.201

scripts/base/init-bare.zeek

@@ -4899,9 +4899,21 @@ const dpd_reassemble_first_packets = T &redef;
 ## connections will be able to analyze the session.
 ##
 ## .. zeek:see:: dpd_reassemble_first_packets dpd_match_only_beginning
-##    dpd_ignore_ports
+##    dpd_ignore_ports dpd_max_packets
 const dpd_buffer_size = 1024 &redef;
 
+## Maximum number of per-connection packets that will be buffered for dynamic
+## protocol detection. For each connection, Zeek buffers up to this amount
+## of packets in memory so that complete protocol analysis can start even after
+## the initial packets have already passed through (i.e., when a DPD signature
+## matches only later). However, once the buffer is full, data is deleted and lost
+## to analyzers that are activated afterwards. Then only analyzers that can deal
+## with partial connections will be able to analyze the session.
+##
+## .. zeek:see:: dpd_reassemble_first_packets dpd_match_only_beginning
+##    dpd_ignore_ports dpd_buffer_size
+const dpd_max_packets = 100 &redef;
+
 ## If true, stops signature matching if :zeek:see:`dpd_buffer_size` has been
 ## reached.
 ##

src/NetVar.cc

@@ -181,6 +181,7 @@ int sig_max_group_size;
 
 int dpd_reassemble_first_packets;
 int dpd_buffer_size;
+int dpd_max_packets;
 int dpd_match_only_beginning;
 int dpd_late_match_stop;
 int dpd_ignore_ports;
@@ -339,6 +340,7 @@ void init_net_var()
 
 	dpd_reassemble_first_packets = id::find_val("dpd_reassemble_first_packets")->AsBool();
 	dpd_buffer_size = id::find_val("dpd_buffer_size")->AsCount();
+	dpd_max_packets = id::find_val("dpd_max_packets")->AsCount();
 	dpd_match_only_beginning = id::find_val("dpd_match_only_beginning")->AsBool();
 	dpd_late_match_stop = id::find_val("dpd_late_match_stop")->AsBool();
 	dpd_ignore_ports = id::find_val("dpd_ignore_ports")->AsBool();

src/NetVar.h

@@ -83,6 +83,7 @@ extern int sig_max_group_size;
 
 extern int dpd_reassemble_first_packets;
 extern int dpd_buffer_size;
+extern int dpd_max_packets;
 extern int dpd_match_only_beginning;
 extern int dpd_late_match_stop;
 extern int dpd_ignore_ports;

src/analyzer/protocol/pia/PIA.cc

@@ -76,7 +76,7 @@ void PIA::AddToBuffer(Buffer* buffer, int len, const u_char* data, bool is_orig,
 
 void PIA::ReplayPacketBuffer(analyzer::Analyzer* analyzer)
 	{
-	DBG_LOG(DBG_ANALYZER, "PIA replaying %d total packet bytes", pkt_buffer.size);
+	DBG_LOG(DBG_ANALYZER, "PIA replaying %" PRIu64 " total packet bytes", pkt_buffer.size);
 
 	for ( DataBlock* b = pkt_buffer.head; b; b = b->next )
 		analyzer->DeliverPacket(b->len, b->data, b->is_orig, -1, b->ip, 0);
@@ -106,8 +106,9 @@ void PIA::PIA_DeliverPacket(int len, const u_char* data, bool is_orig, uint64_t
 	if ( (pkt_buffer.state == BUFFERING || new_state == BUFFERING) && len > 0 )
 		{
 		AddToBuffer(&pkt_buffer, seq, len, data, is_orig, ip);
-		if ( pkt_buffer.size > zeek::detail::dpd_buffer_size )
-			new_state = zeek::detail::dpd_match_only_beginning ? SKIPPING : MATCHING_ONLY;
+		if ( pkt_buffer.size > zeek::detail::dpd_buffer_size || ++pkt_buffer.chunks > zeek::detail::dpd_max_packets )
+			new_state = zeek::detail::dpd_match_only_beginning ?
+						SKIPPING : MATCHING_ONLY;
 		}
 
 	// FIXME: I'm not sure why it does not work with eol=true...
@@ -279,8 +280,9 @@ void PIA_TCP::DeliverStream(int len, const u_char* data, bool is_orig)
 	if ( stream_buffer.state == BUFFERING || new_state == BUFFERING )
 		{
 		AddToBuffer(&stream_buffer, len, data, is_orig);
-		if ( stream_buffer.size > zeek::detail::dpd_buffer_size )
-			new_state = zeek::detail::dpd_match_only_beginning ? SKIPPING : MATCHING_ONLY;
+		if ( stream_buffer.size > zeek::detail::dpd_buffer_size || ++stream_buffer.chunks > zeek::detail::dpd_max_packets )
+			new_state = zeek::detail::dpd_match_only_beginning ?
+						SKIPPING : MATCHING_ONLY;
 		}
 
 	DoMatch(data, len, is_orig, false, false, false, nullptr);
@@ -292,11 +294,17 @@ void PIA_TCP::Undelivered(uint64_t seq, int len, bool is_orig)
 	{
 	analyzer::tcp::TCP_ApplicationAnalyzer::Undelivered(seq, len, is_orig);
 
-	if ( stream_buffer.state == BUFFERING )
+	if ( stream_buffer.state != BUFFERING )
+		return;
+
 	// We use data=nil to mark an undelivered.
 	AddToBuffer(&stream_buffer, seq, len, nullptr, is_orig);
 
-	// No check for buffer overrun here. I think that's ok.
+	if ( ++stream_buffer.chunks > zeek::detail::dpd_max_packets )
+		{
+		stream_buffer.state = zeek::detail::dpd_match_only_beginning ? SKIPPING : MATCHING_ONLY;
+		DBG_LOG(DBG_ANALYZER, "PIA_TCP[%d] buffer chunks exceeded", GetID());
+		}
 	}
 
 void PIA_TCP::ActivateAnalyzer(analyzer::Tag tag, const zeek::detail::Rule* rule)
@@ -428,7 +436,7 @@ void PIA_TCP::DeactivateAnalyzer(analyzer::Tag tag)
 
 void PIA_TCP::ReplayStreamBuffer(analyzer::Analyzer* analyzer)
 	{
-	DBG_LOG(DBG_ANALYZER, "PIA_TCP replaying %d total stream bytes", stream_buffer.size);
+	DBG_LOG(DBG_ANALYZER, "PIA_TCP replaying %" PRIu64 " total stream bytes", stream_buffer.size);
 
 	for ( DataBlock* b = stream_buffer.head; b; b = b->next )
 		{

src/analyzer/protocol/pia/PIA.h

@@ -74,12 +74,14 @@ protected:
 			{
 			head = tail = nullptr;
 			size = 0;
+			chunks = 0;
 			state = INIT;
 			}
 
 		DataBlock* head;
 		DataBlock* tail;
-		int size;
+		int64_t size;
+		int64_t chunks;
 		State state;
 		};
 

src/event.bif

@@ -392,7 +392,7 @@ event protocol_confirmation%(c: connection, atype: Analyzer::Tag, aid: count%);
 ##        ``Analyzer::ANALYZER_HTTP`` means the HTTP analyzer determined that it's indeed
 ##        parsing an HTTP connection.
 ##
-## .. zeek:see:: dpd_buffer_size
+## .. zeek:see:: dpd_buffer_size dpd_max_packets
 event protocol_late_match%(c: connection, atype: Analyzer::Tag%);
 
 ## Generated when a protocol analyzer determines that a connection it is parsing