[Bro] ssl binpac analyzer -- patches

[Tobias Kiesling]

Jimmy,

your suggestion sounds good. I implemented the changes and tested them with
my ssl analyzer. Everything worked as expected. :-)

Attached you can find the patch for these changes (to be applied with -p0 in
the bro directory, after applying all of the other binpac patches that I
provided before). Could you test this further with whatever binpac analyzers
you have got?

Anyone else any comments on this issue (Ruoming)?

Tobias


On 5/30/07, jmzhou.ml at gmail.com <jmzhou.ml at gmail.com> wrote:
>
> On Tue, 29 May 2007, Tobias Kiesling wrote:
>
> > On 5/29/07, jmzhou.ml at gmail.com <jmzhou.ml at gmail.com> wrote:
> >
> >> Patch 6: I think there is another issue here: if some buffering request
> >> has been sent to the flow buffer, and the flow buffer is not ready with
> >> partial data, we will end up with entering the loop - a waste of CPU
> >> cycles.
> >
> >
> > Maybe you are right, but what would you suggest as change? Do you want
> to
> > check whether the buffer is ready before entering the loop? But then it
> has
> > to be ensured that the buffer is properly initialized in any case.  At
> the
> > moment I cannot see all the consequences of such a change. And do you
> think
> > that the impact on performance is really relevant?
>
> The cost can be a little bit expensive if there are many layers of
> parsing.
> You end up with many unnecessary calls to parsing functions and condition
> jumps. One possible approach is like this:
>
> . add a new boolean member have_pending_request to FlowBuffer, initialized
> as false.
>
> . set have_pending_request to true in call NewFrame and NewLine.
>
> . reset have_pending_request to false in call DiscardData.
>
> . change the while loop condition to:
>     while (flow_buffer->data_available() &&
>         (!flow_buffer->have_pending_request() || flow_buffer->ready()))
>
> Analysis:
>
>   1. The first time, data_available = true, !have_pending_request = true,
> we enter into the loop. Good.
>
>   2. All data are consumed, data_available = false, we do not enter into
> the loop. Good.
>
>   3. A request is not satisfied because of partial data: data_available =
> true, !have_pending_request = false, ready = false, we do not enter into
> the loop. Good.
>
>   4. Parsing is forced to stop because of exception. data_available =
> false.
> We do not enter into the loop. Good.
>
>   5. Parsing current dataunit has finished, we still have residual data -
> wrong data? data_available = true, !have_pending_request = false, ready =
> true (from last parsing). We enter into the loop, and start a new round of
> parsing. As expected. Good.
>
> So far so good. :-)
>
> Jimmy
>
> ____________________________________________________________
> The future is not set.  There is no fate but what we make
> for ourselves.             - Terminator II, Judgment Day
> ------------------------------------------------------------
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.ICSI.Berkeley.EDU/pipermail/bro/attachments/20070530/036d759d/attachment.html 
-------------- next part --------------
Index: src/binpac/lib/binpac_buffer.cc
===================================================================
--- src/binpac/lib/binpac_buffer.cc	(revision 14)
+++ src/binpac/lib/binpac_buffer.cc	(working copy)
@@ -32,6 +32,7 @@
 
 	data_seq_at_orig_data_end_ = 0;
 	eof_ = false;
+	have_pending_request_ = false;
 
 	buffer_n_ = 0;
 
@@ -119,6 +120,7 @@
 	mode_ = LINE_MODE;
 	frame_length_ = 0;
 	chunked_ = false;
+	have_pending_request_ = true;
 	if ( state_ == FRAME_0 )
 		ResetLineState();
 	MarkOrCopyLine();
@@ -130,6 +132,7 @@
 	mode_ = FRAME_MODE;
 	frame_length_ = frame_length;
 	chunked_ = chunked;
+	have_pending_request_ = true;
 	MarkOrCopyFrame();
 	}
 
@@ -148,6 +151,7 @@
 	{
 	mode_ = UNKNOWN_MODE;
 	message_complete_ = false;
+	have_pending_request_ = false;
 	orig_data_begin_ = orig_data_end_ = 0;
 
 	buffer_n_ = 0;
Index: src/binpac/lib/binpac_buffer.h
===================================================================
--- src/binpac/lib/binpac_buffer.h	(revision 14)
+++ src/binpac/lib/binpac_buffer.h	(working copy)
@@ -82,6 +82,8 @@
 	bool eof() const	{ return eof_; }
 	void set_eof();
 
+	bool have_pending_request() const { return have_pending_request_; }
+
 protected:
 	// Reset the buffer for a new message
 	void NewMessage();
@@ -136,6 +138,7 @@
 
 	int 	data_seq_at_orig_data_end_;
 	bool 	eof_;
+	bool    have_pending_request_;
 };
 
 typedef FlowBuffer *flow_buffer_t;
Index: src/binpac/pac_flow.cc
===================================================================
--- src/binpac/pac_flow.cc	(revision 14)
+++ src/binpac/pac_flow.cc	(working copy)
@@ -277,9 +277,11 @@
 		env_->RValue(begin_of_data), 
 		env_->RValue(end_of_data)); 
 
-	out_cc->println("while ( %s->data_available() )",
+	out_cc->println("while ( %s->data_available() && ",
 		env_->LValue(flow_buffer_id));
 	out_cc->inc_indent();
+	out_cc->println("( !%s->have_pending_request() || %s->ready() ) )",
+		env_->LValue(flow_buffer_id), env_->LValue(flow_buffer_id));
 	out_cc->println("{");
 
 	// Generate a new dataunit if necessary