diff --git a/CHANGES b/CHANGES index e16f836de0..76b13f3617 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,26 @@ +4.2.0-dev.510 | 2022-01-03 13:54:52 -0700 + + * Switch BitTorrent analyzer to Zeek's regex engine (Avinal Kumar) + + - Removes dependency on + - Replaces regex function with Zeek's standard regex functions + - Some replacements are workaround, may be improved later via an + appropiate API + - Update test baseline to fix what seems to be capturing on a bug in the + existing code. + + Edit pass by Robin Sommer. Note that our test doesn't cover all the code + paths, but it does go through the one with the most substantial change. + + * Adding test for BitTorrent tracker. (Robin Sommer, Corelight) + + Our test trace is extracted from https://www.cloudshark.org/captures/b9089aac6eee. + + There actually seems to be a bug in the existing code: the URI passed to + bt_tracker_request() includes a partial HTTP version. This commits + includes the baseline as the current code produces it, we'll fix that in + a subsequent comment. + 4.2.0-dev.506 | 2022-01-03 09:33:43 -0800 * Expansion of the emerging cluster controller framework (Christian Kreibich, Corelight) diff --git a/VERSION b/VERSION index 5295d20f31..de438350df 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.2.0-dev.506 +4.2.0-dev.510 diff --git a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc b/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc index e2a7f8297c..92de0788c8 100644 --- a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc +++ b/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc @@ -2,10 +2,10 @@ #include "zeek/analyzer/protocol/bittorrent/BitTorrentTracker.h" -#include #include #include +#include "zeek/RE.h" #include "zeek/analyzer/protocol/bittorrent/events.bif.h" #include "zeek/analyzer/protocol/tcp/TCP_Reassembler.h" @@ -243,13 +243,23 @@ void BitTorrentTracker_Analyzer::DeliverWeird(const char* msg, bool orig) bool BitTorrentTracker_Analyzer::ParseRequest(char* line) { static bool initialized = false; - static regex_t r_get, r_get_end, r_hdr; + static RE_Matcher re_get("^GET[ \t]+"); + static RE_Matcher re_url("[^ \t]+"); + static RE_Matcher re_version("[ \t]+HTTP/[0-9.]+$"); + static RE_Matcher re_hdr("^[^: \t]+:[ ]*"); if ( ! initialized ) { - regcomp(&r_get, "^GET[ \t]+", REG_EXTENDED | REG_ICASE); - regcomp(&r_get_end, "[ \t]+HTTP/[0123456789.]+$", REG_EXTENDED | REG_ICASE); - regcomp(&r_hdr, "^[^: \t]+:[ ]*", REG_EXTENDED | REG_ICASE); + re_get.MakeCaseInsensitive(); + re_url.MakeCaseInsensitive(); + re_version.MakeCaseInsensitive(); + re_hdr.MakeCaseInsensitive(); + + re_get.Compile(); + re_url.Compile(); + re_version.Compile(); + re_hdr.Compile(); + initialized = true; } @@ -257,29 +267,32 @@ bool BitTorrentTracker_Analyzer::ParseRequest(char* line) { case detail::BTT_REQ_GET: { - regmatch_t match[1]; - if ( regexec(&r_get, line, 1, match, 0) ) + char* url_begin = nullptr; + char* url_end = nullptr; + + if ( auto len_get = re_get.MatchPrefix(line); len_get > 0 ) + { + url_begin = line + len_get; + + if ( auto len_url = re_url.MatchPrefix(url_begin); len_url > 0 ) + url_end = url_begin + len_url; + } + + if ( ! (url_begin && url_end) ) { AnalyzerViolation("BitTorrentTracker: invalid HTTP GET"); stop_orig = true; return false; } - regmatch_t match_end[1]; - if ( ! regexec(&r_get_end, line, 1, match_end, 0) ) + if ( auto version_len = re_version.MatchPrefix(url_end); version_len > 0 ) { - if ( match_end[0].rm_so <= match[0].rm_eo ) - { - AnalyzerViolation("BitTorrentTracker: invalid HTTP GET"); - stop_orig = true; - return false; - } - - keep_alive = (line[match_end[0].rm_eo - 1] == '1'); - line[match_end[0].rm_so] = 0; + // For keep_alive, check the last char of the matched string for the HTTP version. + keep_alive = (url_end[version_len - 1] == '1'); + *url_end = 0; } - RequestGet(&line[match[0].rm_eo]); + RequestGet(url_begin); req_state = detail::BTT_REQ_HEADER; } @@ -294,8 +307,8 @@ bool BitTorrentTracker_Analyzer::ParseRequest(char* line) break; } - regmatch_t match[1]; - if ( regexec(&r_hdr, line, 1, match, 0) ) + int len_hdr = re_hdr.MatchPrefix(line); + if ( len_hdr <= 0 ) { AnalyzerViolation("BitTorrentTracker: invalid HTTP request header"); stop_orig = true; @@ -303,7 +316,7 @@ bool BitTorrentTracker_Analyzer::ParseRequest(char* line) } *strchr(line, ':') = 0; // this cannot fail - see regex_hdr - RequestHeader(line, &line[match[0].rm_eo]); + RequestHeader(line, line + len_hdr); } break; @@ -344,12 +357,17 @@ void BitTorrentTracker_Analyzer::EmitRequest(void) bool BitTorrentTracker_Analyzer::ParseResponse(char* line) { static bool initialized = false; - static regex_t r_stat, r_hdr; + static RE_Matcher re_stat("^HTTP/[0-9.]* "); + static RE_Matcher re_hdr("^[^: \t]+:[ ]*"); if ( ! initialized ) { - regcomp(&r_stat, "^HTTP/[0123456789.]* ", REG_EXTENDED | REG_ICASE); - regcomp(&r_hdr, "^[^: \t]+:[ ]*", REG_EXTENDED | REG_ICASE); + re_stat.MakeCaseInsensitive(); + re_hdr.MakeCaseInsensitive(); + + re_stat.Compile(); + re_hdr.Compile(); + initialized = true; } @@ -366,15 +384,15 @@ bool BitTorrentTracker_Analyzer::ParseResponse(char* line) break; } - regmatch_t match[1]; - if ( regexec(&r_stat, line, 1, match, 0) ) + int len_stat = re_stat.MatchPrefix(line); + if ( len_stat <= 0 ) { AnalyzerViolation("BitTorrentTracker: invalid HTTP status"); stop_resp = true; return false; } - ResponseStatus(&line[match[0].rm_eo]); + ResponseStatus(line + len_stat); res_state = detail::BTT_RES_HEADER; } break; @@ -399,8 +417,8 @@ bool BitTorrentTracker_Analyzer::ParseResponse(char* line) } { - regmatch_t match[1]; - if ( regexec(&r_hdr, line, 1, match, 0) ) + int len_hdr = re_hdr.MatchPrefix(line); + if ( len_hdr <= 0 ) { AnalyzerViolation("BitTorrentTracker: invalid HTTP response header"); stop_resp = true; @@ -408,7 +426,7 @@ bool BitTorrentTracker_Analyzer::ParseResponse(char* line) } *strchr(line, ':') = 0; // this cannot fail - see regex_hdr - ResponseHeader(line, &line[match[0].rm_eo]); + ResponseHeader(line, line + len_hdr); } break; diff --git a/testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output b/testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output new file mode 100644 index 0000000000..927395085a --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +[orig_h=10.0.0.201, orig_p=49842/tcp, resp_h=91.189.95.21, resp_p=6969/tcp], /announce?info_hash=%e4%be%9eM%b8v%e3%e3%17%97x%b0%3e%90b%97%be%5c%8d%be&peer_id=-DE13F0-VnpZRF8ZP9iv&port=63448&uploaded=0&downloaded=0&left=1921843200&corrupt=0&key=764CA003&event=started&numwant=200&compact=1&no_peer_id=1&supportcrypto=1&redundant=0, { + +} diff --git a/testing/btest/Traces/bittorrent/tracker.pcap b/testing/btest/Traces/bittorrent/tracker.pcap new file mode 100644 index 0000000000..a0bccfe87f Binary files /dev/null and b/testing/btest/Traces/bittorrent/tracker.pcap differ diff --git a/testing/btest/scripts/base/protocols/bittorrent/tracker.zeek b/testing/btest/scripts/base/protocols/bittorrent/tracker.zeek new file mode 100644 index 0000000000..d12040a672 --- /dev/null +++ b/testing/btest/scripts/base/protocols/bittorrent/tracker.zeek @@ -0,0 +1,45 @@ +# @TEST-DOC: Basic functionality test for Bittorrent Tracker analyzer. + +# @TEST-EXEC: zeek -C -b -r $TRACES/bittorrent/tracker.pcap -s bittorrent.sig %INPUT >output +# @TEST-EXEC: btest-diff output + +# Zeek doesn't ship with scripts or DPD sigs for Bittorrent, so we need to provide what +# we need ourselves. + +event bt_tracker_request(c: connection, uri: string, headers: bt_tracker_headers) { + print c$id, uri, headers; +} + +@TEST-START-FILE bittorrent.sig + +# Reusing the old Bro 1.5 signatures here. + +signature dpd_bittorrenttracker_client { + ip-proto == tcp + payload /^.*\/announce\?.*info_hash/ + tcp-state originator +} + +signature dpd_bittorrenttracker_server { + ip-proto == tcp + payload /^HTTP\/[0-9]/ + tcp-state responder + requires-reverse-signature dpd_bittorrenttracker_client + enable "bittorrenttracker" +} + +signature dpd_bittorrent_peer1 { + ip-proto == tcp + payload /^\x13BitTorrent protocol/ + tcp-state originator +} + +signature dpd_bittorrent_peer2 { + ip-proto == tcp + payload /^\x13BitTorrent protocol/ + tcp-state responder + requires-reverse-signature dpd_bittorrent_peer1 + enable "bittorrent" +} + +@TEST-END-FILE