Some small tweaks to the HTTP analyzer.

From ticket #339.

CHANGES

@@ -1,3 +1,8 @@
+1.6-dev.27 Thu Jan 20 13:52:25 PST 2011
+
+  * Fine-tuning of the HTTP analyzer in terms of raising protocol
+    violations and  interrupted transfers. (Gregor Maier)
+
 1.6-dev.21 Wed Jan 19 17:36:02 PST 2011
 
     * Added 4 new BiFs and a new record type for testing the entropy

VERSION

@@ -1 +1 @@
-1.6-dev.21
+1.6-dev.27

policy/http-header.bro

@@ -2,6 +2,8 @@
 
 # Prints out detailed HTTP headers.
 
+@load http
+
 module HTTP;
 
 export {

src/HTTP.cc

@@ -16,16 +16,20 @@
 
 const bool DEBUG_http = false;
 
+// The EXPECT_*_NOTHING states are used to prevent further parsing. Used if a
+// message was interrupted. 
 enum {
 	EXPECT_REQUEST_LINE,
 	EXPECT_REQUEST_MESSAGE,
 	EXPECT_REQUEST_TRAILER,
+	EXPECT_REQUEST_NOTHING,
 };
 
 enum {
 	EXPECT_REPLY_LINE,
 	EXPECT_REPLY_MESSAGE,
 	EXPECT_REPLY_TRAILER,
+	EXPECT_REPLY_NOTHING,
 };
 
 HTTP_Entity::HTTP_Entity(HTTP_Message *arg_message, MIME_Entity* parent_entity, int arg_expect_body)
@@ -851,7 +855,23 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 					HTTP_Event("crud_trailing_HTTP_request",
 						   new_string_val(line, end_of_line));
 				else
+					{
+					// We do see HTTP requests with a
+					// trailing EOL that's not accounted
+					// for by the content-length. This
+					// will lead to a call to this method
+					// with len==0 while we are expecting
+					// a new request. Since HTTP servers
+					// handle such requests gracefully,
+					// we should do so as well. 
+					if ( len == 0 )
+					    Weird("empty_http_request");
+					else 
+						{
 						ProtocolViolation("not a http request line");
+						request_state = EXPECT_REQUEST_NOTHING;
+						}
+					}
 				}
 			break;
 
@@ -861,6 +881,9 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 
 		case EXPECT_REQUEST_TRAILER:
 			break;
+
+		case EXPECT_REQUEST_NOTHING:
+			break;
 		}
 		}
 	else
@@ -873,6 +896,8 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 
 				if ( unanswered_requests.empty() )
 					Weird("unmatched_HTTP_reply");
+				else
+					ProtocolConfirmation();
 
 				reply_state = EXPECT_REPLY_MESSAGE;
 				reply_ongoing = 1;
@@ -885,7 +910,10 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 						len);
 				}
 			else
+				{
 				ProtocolViolation("not a http reply line");
+				reply_state = EXPECT_REPLY_NOTHING;
+				}
 
 			break;
 
@@ -895,6 +923,9 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 
 		case EXPECT_REPLY_TRAILER:
 			break;
+
+		case EXPECT_REPLY_NOTHING:
+			break;
 		}
 		}
 	}
@@ -1042,6 +1073,7 @@ int HTTP_Analyzer::HTTP_RequestLine(const char* line, const char* end_of_line)
 		// HTTP methods for distributed authoring.
 		"PROPFIND", "PROPPATCH", "MKCOL", "DELETE", "PUT",
 		"COPY", "MOVE", "LOCK", "UNLOCK",
+		"POLL", "REPORT", "SUBSCRIBE", "BMOVE",
 
 		"SEARCH",
 
@@ -1256,6 +1288,9 @@ void HTTP_Analyzer::RequestMade(const int interrupted, const char* msg)
 
 	num_request_lines = 0;
 
+	if ( interrupted )
+		request_state = EXPECT_REQUEST_NOTHING;
+	else 
 		request_state = EXPECT_REQUEST_LINE;
 	}
 
@@ -1285,6 +1320,9 @@ void HTTP_Analyzer::ReplyMade(const int interrupted, const char* msg)
 		reply_reason_phrase = 0;
 		}
 
+	if ( interrupted )
+		reply_state = EXPECT_REPLY_NOTHING;
+	else
 		reply_state = EXPECT_REPLY_LINE;
 	}
 

src/bro.bif

@@ -1365,12 +1365,17 @@ function skip_http_entity_data%(c: connection, is_orig: bool%): any
 		{
 		Analyzer* ha = c->FindAnalyzer(id);
 
+		if ( ha )
+			{
 			if ( ha->GetTag() == AnalyzerTag::HTTP )
 				static_cast<HTTP_Analyzer*>(ha)->SkipEntityData(is_orig);
 			else
 				run_time("non-HTTP analyzer associated with connection record");
 			}
+		else
+			run_time("could not find analyzer for skip_http_entity_data");
 
+		}
 	else
 		run_time("no analyzer associated with connection record");