Some small tweaks to the HTTP analyzer

From ticket #339 http://tracker.icir.org/bro/ticket/339 :

* Fixing a couple of minor issues in the HTTP analyzer, that made the
  analyzer raise a ProtocolViolation() on strange but seemingly legal
  HTTP traffic. Well, the traffic might not necessarily be adhering the
  RFC, but the server has understood it.

* Also stops parsing when the message is interrupted to prevent further
  parsing and ProtocolViolation() generation.

* skip_http_entity_data: check return value of  FindAnalyzer, since it
  can be NULL

In addition:
* http-headers.bro now loads http.bro

Testsuite changes:
* Added a new weird (empty_http_request). This shows up once in the medium
  testsuite.
* no change when running short testsuite
This commit is contained in:
Gregor Maier 2010-12-13 19:11:05 -08:00
parent b8ab0ebc22
commit 763a446182
3 changed files with 52 additions and 8 deletions

View file

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

View file

@ -16,16 +16,21 @@
const bool DEBUG_http = false; const bool DEBUG_http = false;
/* The EXPECT_*_NOTHING states are used to prevent further parsing. Used
* if a message was interrupted.
*/
enum { enum {
EXPECT_REQUEST_LINE, EXPECT_REQUEST_LINE,
EXPECT_REQUEST_MESSAGE, EXPECT_REQUEST_MESSAGE,
EXPECT_REQUEST_TRAILER, EXPECT_REQUEST_TRAILER,
EXPECT_REQUEST_NOTHING,
}; };
enum { enum {
EXPECT_REPLY_LINE, EXPECT_REPLY_LINE,
EXPECT_REPLY_MESSAGE, EXPECT_REPLY_MESSAGE,
EXPECT_REPLY_TRAILER, EXPECT_REPLY_TRAILER,
EXPECT_REPLY_NOTHING,
}; };
HTTP_Entity::HTTP_Entity(HTTP_Message *arg_message, MIME_Entity* parent_entity, int arg_expect_body) HTTP_Entity::HTTP_Entity(HTTP_Message *arg_message, MIME_Entity* parent_entity, int arg_expect_body)
@ -851,7 +856,20 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
HTTP_Event("crud_trailing_HTTP_request", HTTP_Event("crud_trailing_HTTP_request",
new_string_val(line, end_of_line)); new_string_val(line, end_of_line));
else else
{
// We do see HTTP requests with a trailing EOL that's not
// 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 request gracefully, we should do so as well.
if (len==0)
Weird("empty_http_request");
else
{
ProtocolViolation("not a http request line"); ProtocolViolation("not a http request line");
request_state = EXPECT_REQUEST_NOTHING;
}
}
} }
break; break;
@ -861,6 +879,9 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
case EXPECT_REQUEST_TRAILER: case EXPECT_REQUEST_TRAILER:
break; break;
case EXPECT_REQUEST_NOTHING:
break;
} }
} }
else else
@ -873,6 +894,8 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
if ( unanswered_requests.empty() ) if ( unanswered_requests.empty() )
Weird("unmatched_HTTP_reply"); Weird("unmatched_HTTP_reply");
else
ProtocolConfirmation();
reply_state = EXPECT_REPLY_MESSAGE; reply_state = EXPECT_REPLY_MESSAGE;
reply_ongoing = 1; reply_ongoing = 1;
@ -885,7 +908,10 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
len); len);
} }
else else
{
ProtocolViolation("not a http reply line"); ProtocolViolation("not a http reply line");
reply_state = EXPECT_REPLY_NOTHING;
}
break; break;
@ -895,6 +921,9 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
case EXPECT_REPLY_TRAILER: case EXPECT_REPLY_TRAILER:
break; break;
case EXPECT_REPLY_NOTHING:
break;
} }
} }
} }
@ -1042,6 +1071,8 @@ int HTTP_Analyzer::HTTP_RequestLine(const char* line, const char* end_of_line)
// HTTP methods for distributed authoring. // HTTP methods for distributed authoring.
"PROPFIND", "PROPPATCH", "MKCOL", "DELETE", "PUT", "PROPFIND", "PROPPATCH", "MKCOL", "DELETE", "PUT",
"COPY", "MOVE", "LOCK", "UNLOCK", "COPY", "MOVE", "LOCK", "UNLOCK",
// More stuff
"POLL", "REPORT", "SUBSCRIBE", "BMOVE",
"SEARCH", "SEARCH",
@ -1256,6 +1287,9 @@ void HTTP_Analyzer::RequestMade(const int interrupted, const char* msg)
num_request_lines = 0; num_request_lines = 0;
if (interrupted)
request_state = EXPECT_REQUEST_NOTHING;
else
request_state = EXPECT_REQUEST_LINE; request_state = EXPECT_REQUEST_LINE;
} }
@ -1285,6 +1319,9 @@ void HTTP_Analyzer::ReplyMade(const int interrupted, const char* msg)
reply_reason_phrase = 0; reply_reason_phrase = 0;
} }
if (interrupted)
reply_state = EXPECT_REPLY_NOTHING;
else
reply_state = EXPECT_REPLY_LINE; reply_state = EXPECT_REPLY_LINE;
} }

View file

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