From 0507f6005c705d2cddff591797049a6ef2e7eb8e Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 21 Dec 2021 17:09:39 +0100 Subject: [PATCH 1/2] Adding test for BitTorrent tracker. 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. --- .../output | 4 ++ testing/btest/Traces/bittorrent/tracker.pcap | Bin 0 -> 1614 bytes .../base/protocols/bittorrent/tracker.zeek | 45 ++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output create mode 100644 testing/btest/Traces/bittorrent/tracker.pcap create mode 100644 testing/btest/scripts/base/protocols/bittorrent/tracker.zeek 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..b5fa94bb24 --- /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 HTTP/1., { + +} diff --git a/testing/btest/Traces/bittorrent/tracker.pcap b/testing/btest/Traces/bittorrent/tracker.pcap new file mode 100644 index 0000000000000000000000000000000000000000..a0bccfe87f5d6c4cff95d1321bc6ef574efa5aae GIT binary patch literal 1614 zcma)+ZA?>F7{^b`dqW9gU*0ZV#oS+*{jQ-V6-G zK5S!T#wc?-Q5R%pAg@+P*s=)yfLo9cg1QN^kPKgDTyb%b2I$UhbzSHSHm&yn@1bWVfZs^eW{a7A6J`^s`9BcW4S=_?5+q_N!!1naI&hqf^WwAWj4Dp*ZFQ7p4Ot@Gs zu9FxWnT&RN=SdhELk*XY4`p;@_aJKf(IC6+LZOSukQy_*fD}QP*pgec+Q*rj-YvIw zMC;8DwA(C!E``St2#w`BjZ%=7 zE)r)51;wPHv>-!PnlIPY@(q-cR!YSZiHvVH=qX$SHA*30L)DT04fL>*R*K{aBAG;- zAQbWm%4jqjXtb;vHYk&%lGGHTP|Sz5kffC+8aL8V!zax(bvPQkHq;n!0x|d`RsK2~ z5#MAsFkdH(4F;M*lSZg9lNx4|5X)4n^W#NvA}%MBGSLc*M&CsEICF)Wq|I>zRilu~ zrE)H($OMgo6fK%lU};cq)<)l(12?<(9 zkqV5^Qzppea5+rzn>H8UX~|7CijGt|ZIt9@qu$K16B|tNhcCK{e@*HG;@Q9%?q+1~ zjZ9%Exr%KmsQ3Z(0KC#J#L(Gcc{$FEX(Ujff~1zNQea|v(wmp+1_MOr1MBH{gC5tB zTu#0b*Vf<)jHCnvPE;X%NU{^)eiU3zM4PYkr(`>F1_!clPYWiq1X)uO(cpDeqGTvL z?)%v2TVLk=lBJ55CJt0!yF$Yb!=;<;iIM@81#nJxg`D9nX~IfFL%YtE26uIb1c!%* z96RN#}X%RUR|`#K4DoFLiQBSPyJK7?l(K{zjQC)!#U%| zrI`uti?Wr@(R28{rf1fo^F9TBQGJcm%{Oe;*~eZ1pL5?0YCCl!dAN15ugRf$y!M&w z`I-FzyC23h-dPOm5jhmoJ4-BQ?YH-QFfvAuzxeI^)w7H1Q3?B$Q~sxCQ@wglHIwbK zj=I0c4^$s)%`5z2@6sprQlp~TA7A2JjL_D_%%FV6a-vEm=6?V7r>74_VwbTy&s%q= z=z`k5T9YOBq)PASzC8NOUfTSizbQWNh}k5Q{4wsa^0BG7b2N!rYa-{fPK-jWf^H6BrYcYS9`Aqy6q zobE@xf-&K$!skr~+G;}7G*K^0c-uazD^3&cvfBnPxF~&xHc{MrY_!^s=|Bq~z1@LM zY-Dt(y7L29SB5+P7lq@b=(eToK$KUx^l#2(tD;n=B9WV-ZluV8eyhI*uK@5bARsfz literal 0 HcmV?d00001 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 From c2cff6dac78cfbd85d2dc724db1c195e391c1a8e Mon Sep 17 00:00:00 2001 From: Avinal Kumar Date: Sat, 13 Nov 2021 13:47:44 +0530 Subject: [PATCH 2/2] Switch BitTorrent analyzer to Zeek's regex engine - 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. --- .../protocol/bittorrent/BitTorrentTracker.cc | 80 ++++++++++++------- .../output | 2 +- 2 files changed, 50 insertions(+), 32 deletions(-) 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 index b5fa94bb24..927395085a 100644 --- a/testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output +++ b/testing/btest/Baseline/scripts.base.protocols.bittorrent.tracker/output @@ -1,4 +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 HTTP/1., { +[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, { }