diff --git a/policy/heavy.http.bro b/policy/heavy.http.bro deleted file mode 100644 index f3be0bf058..0000000000 --- a/policy/heavy.http.bro +++ /dev/null @@ -1,3 +0,0 @@ -# $Id: heavy.http.bro 4723 2007-08-07 18:14:35Z vern $ - -redef http_sessions &write_expire = 5 hrs; diff --git a/policy/http-header.bro b/policy/http-header.bro index 3d676488ff..259031b024 100644 --- a/policy/http-header.bro +++ b/policy/http-header.bro @@ -2,6 +2,8 @@ # Prints out detailed HTTP headers. +@load http + module HTTP; export { diff --git a/policy/http.bro b/policy/http.bro index 90b0aa2daa..a5b13d7637 100644 --- a/policy/http.bro +++ b/policy/http.bro @@ -79,18 +79,8 @@ type http_session_info: record { const http_log = open_log_file("http") &redef; -# Called when an HTTP session times out. -global expire_http_session: - function(t: table[conn_id] of http_session_info, id: conn_id) - : interval; - -export { - # Indexed by conn_id. - # (Exported so that we can define a timeout on it.) - global http_sessions: table[conn_id] of http_session_info - &expire_func = expire_http_session - &read_expire = 15 min; -} +# Indexed by conn_id. +global http_sessions: table[conn_id] of http_session_info; global http_session_id = 0; @@ -202,30 +192,6 @@ event connection_state_remove(c: connection) delete http_sessions[c$id]; } -function expire_http_session(t: table[conn_id] of http_session_info, - id: conn_id): interval - { - ### FIXME: not really clear that we need this function at all ... - # - # One would think that connection_state_remove() already takes care - # of everything. However, without this expire-handler, some requests - # don't show up with the test-suite (but haven't reproduced with - # smaller traces) - Robin. - - local s = http_sessions[id]; - finish_stream(id, s$id, s$request_stream); - return 0 sec; - } - -# event connection_timeout(c: connection) -# { -# if ( ! maintain_http_sessions ) -# { -# local id = c$id; -# if ( [id$orig_h, id$resp_h] in http_sessions ) -# delete http_sessions[id$orig_h, id$resp_h]; -# } -# } # event http_stats(c: connection, stats: http_stats_rec) # { diff --git a/src/HTTP.cc b/src/HTTP.cc index 0cccf75103..85872f7c79 100644 --- a/src/HTTP.cc +++ b/src/HTTP.cc @@ -16,16 +16,21 @@ 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 +856,20 @@ 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 - ProtocolViolation("not a http request line"); + { + // 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"); + request_state = EXPECT_REQUEST_NOTHING; + } + } } break; @@ -861,6 +879,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 +894,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 +908,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 +921,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 +1071,8 @@ 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", + // More stuff + "POLL", "REPORT", "SUBSCRIBE", "BMOVE", "SEARCH", @@ -1055,7 +1086,7 @@ int HTTP_Analyzer::HTTP_RequestLine(const char* line, const char* end_of_line) if ( ! http_methods[i] ) { - // Weird("HTTP_unknown_method"); + //Weird("HTTP_unknown_method"); if ( RequestExpected() ) HTTP_Event("unknown_HTTP_method", new_string_val(line, end_of_line)); return 0; @@ -1256,7 +1287,10 @@ void HTTP_Analyzer::RequestMade(const int interrupted, const char* msg) num_request_lines = 0; - request_state = EXPECT_REQUEST_LINE; + if (interrupted) + request_state = EXPECT_REQUEST_NOTHING; + else + request_state = EXPECT_REQUEST_LINE; } void HTTP_Analyzer::ReplyMade(const int interrupted, const char* msg) @@ -1285,7 +1319,10 @@ void HTTP_Analyzer::ReplyMade(const int interrupted, const char* msg) reply_reason_phrase = 0; } - reply_state = EXPECT_REPLY_LINE; + if (interrupted) + reply_state = EXPECT_REPLY_NOTHING; + else + reply_state = EXPECT_REPLY_LINE; } void HTTP_Analyzer::RequestClash(Val* /* clash_val */) diff --git a/src/bro.bif b/src/bro.bif index 0de77bfc49..af841600c8 100644 --- a/src/bro.bif +++ b/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->GetTag() == AnalyzerTag::HTTP ) - static_cast(ha)->SkipEntityData(is_orig); + if (ha) + { + if ( ha->GetTag() == AnalyzerTag::HTTP ) + static_cast(ha)->SkipEntityData(is_orig); + else + run_time("non-HTTP analyzer associated with connection record"); + } else - run_time("non-HTTP analyzer associated with connection record"); - } + run_time("could not find analyzer for skip_http_entity_data"); + } else run_time("no analyzer associated with connection record");