Merge remote-tracking branch 'origin/topic/jsiwek/bit-1854-reassembler-improvements'

Includes small readability tweaks, see BIT-1854.

Closes BIT-1854.

* origin/topic/jsiwek/bit-1854-reassembler-improvements:
  BIT-1854: improve reassembly overlap checking
  BIT-1854: fix the 'tcp_excessive_data_without_further_acks' option

CHANGES

@@ -1,8 +1,20 @@
 
+2.5-435 | 2018-02-06 08:40:38 -0800
+
+  * BIT-1854: Improve reassembly overlap checking. (Corelight)
+
+  * BIT-1854: Fix the 'tcp_excessive_data_without_further_acks'
+    option. (Corelight)
+
+  * Make parsing of ServerKeyExchange work for D(TLS) < 1.2. (Johanna
+    Amann)
+
+  * Add more details to ssl_server_signature. (Johanna Amann)
+
 2.5-427 | 2018-02-05 15:09:14 -0800
 
   * BIT-1898: Fix problems with SumStats non-cluster.bro script.
-    Reported by Jim Mellander. (Jon Siwek)
+    Reported by Jim Mellander. (Corelight)
 
 2.5-424 | 2018-02-05 15:07:20 -0800
 

VERSION

@@ -1 +1 @@
-2.5-427
+2.5-435

src/Reassem.cc

@@ -10,9 +10,9 @@
 
 static const bool DEBUG_reassem = false;
 
-DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq,
+DataBlock::DataBlock(Reassembler* reass, const u_char* data,
-		     DataBlock* arg_prev, DataBlock* arg_next,
+                     uint64 size, uint64 arg_seq, DataBlock* arg_prev,
-		     ReassemblerType reassem_type)
+                     DataBlock* arg_next, ReassemblerType reassem_type)
 	{
 	seq = arg_seq;
 	upper = seq + size;
@@ -28,6 +28,9 @@ DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq,
 	if ( next )
 		next->prev = this;
 
+	reassembler = reass;
+	reassembler->size_of_all_blocks += size;
+
 	rtype = reassem_type;
 	Reassembler::sizes[rtype] += pad_size(size) + padded_sizeof(DataBlock);
 	Reassembler::total_size += pad_size(size) + padded_sizeof(DataBlock);
@@ -37,12 +40,11 @@ uint64 Reassembler::total_size = 0;
 uint64 Reassembler::sizes[REASSEM_NUM];
 
 Reassembler::Reassembler(uint64 init_seq, ReassemblerType reassem_type)
+	:  blocks(), last_block(), old_blocks(), last_old_block(),
+	  last_reassem_seq(init_seq), trim_seq(init_seq),
+	  max_old_blocks(0), total_old_blocks(0), size(0),
+	  rtype(reassem_type)
 	{
-	blocks = last_block = 0;
-	old_blocks = last_old_block = 0;
-	total_old_blocks = max_old_blocks = 0;
-	trim_seq = last_reassem_seq = init_seq;
-	rtype = reassem_type;
 	}
 
 Reassembler::~Reassembler()
@@ -57,6 +59,10 @@ void Reassembler::CheckOverlap(DataBlock *head, DataBlock *tail,
 	if ( ! head || ! tail )
 		return;
 
+	if ( seq == tail->upper )
+		// Special case check for common case of appending to the end.
+		return;
+
 	uint64 upper = (seq + len);
 
 	for ( DataBlock* b = head; b; b = b->next )
@@ -116,7 +122,7 @@ void Reassembler::NewBlock(double t, uint64 seq, uint64 len, const u_char* data)
 
 	if ( ! blocks )
 		blocks = last_block = start_block =
-			new DataBlock(data, len, seq, 0, 0, rtype);
+			new DataBlock(this, data, len, seq, 0, 0, rtype);
 	else
 		start_block = AddAndCheck(blocks, seq, upper_seq, data);
 
@@ -280,8 +286,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
 	// Special check for the common case of appending to the end.
 	if ( last_block && seq == last_block->upper )
 		{
-		last_block = new DataBlock(data, upper - seq, seq,
+		last_block = new DataBlock(this, data, upper - seq,
-					   last_block, 0, rtype);
+		                           seq, last_block, 0, rtype);
 		return last_block;
 		}
 
@@ -294,7 +300,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
 		{
 		// b is the last block, and it comes completely before
 		// the new block.
-		last_block = new DataBlock(data, upper - seq, seq, b, 0, rtype);
+		last_block = new DataBlock(this, data, upper - seq,
+		                           seq, b, 0, rtype);
 		return last_block;
 		}
 
@@ -303,7 +310,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
 	if ( upper <= b->seq )
 		{
 		// The new block comes completely before b.
-		new_b = new DataBlock(data, upper - seq, seq, b->prev, b, rtype);
+		new_b = new DataBlock(this, data, upper - seq, seq,
+		                      b->prev, b, rtype);
 		if ( b == blocks )
 			blocks = new_b;
 		return new_b;
@@ -314,7 +322,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
 		{
 		// The new block has a prefix that comes before b.
 		uint64 prefix_len = b->seq - seq;
-		new_b = new DataBlock(data, prefix_len, seq, b->prev, b, rtype);
+		new_b = new DataBlock(this, data, prefix_len, seq,
+		                      b->prev, b, rtype);
 		if ( b == blocks )
 			blocks = new_b;
 

src/Reassem.h

@@ -18,11 +18,14 @@ enum ReassemblerType {
 	REASSEM_NUM,
 };
 
+class Reassembler;
+
 class DataBlock {
 public:
-	DataBlock(const u_char* data, uint64 size, uint64 seq,
+	DataBlock(Reassembler* reass, const u_char* data,
-		  DataBlock* prev, DataBlock* next,
+	          uint64 size, uint64 seq,
-		  ReassemblerType reassem_type = REASSEM_UNKNOWN);
+	          DataBlock* prev, DataBlock* next,
+	          ReassemblerType reassem_type = REASSEM_UNKNOWN);
 
 	~DataBlock();
 
@@ -33,6 +36,8 @@ public:
 	uint64 seq, upper;
 	u_char* block;
 	ReassemblerType rtype;
+
+	Reassembler* reassembler; // Non-owning pointer back to parent.
 };
 
 class Reassembler : public BroObj {
@@ -96,6 +101,7 @@ protected:
 	uint64 trim_seq;	// how far we've trimmed
 	uint32 max_old_blocks;
 	uint32 total_old_blocks;
+	uint64 size_of_all_blocks;
 
 	ReassemblerType rtype;
 
@@ -105,6 +111,7 @@ protected:
 
 inline DataBlock::~DataBlock()
 	{
+	reassembler->size_of_all_blocks -= Size();
 	Reassembler::total_size -= pad_size(upper - seq) + padded_sizeof(DataBlock);
 	Reassembler::sizes[rtype] -= pad_size(upper - seq) + padded_sizeof(DataBlock);
 	delete [] block;

src/analyzer/protocol/tcp/TCP_Reassembler.cc

@@ -501,7 +501,7 @@ int TCP_Reassembler::DataSent(double t, uint64 seq, int len,
 		}
 
 	if ( tcp_excessive_data_without_further_acks &&
-	     NumUndeliveredBytes() > static_cast<uint64>(tcp_excessive_data_without_further_acks) )
+	     size_of_all_blocks > static_cast<uint64>(tcp_excessive_data_without_further_acks) )
 		{
 		tcp_analyzer->Weird("excessive_data_without_further_acks");
 		ClearBlocks();