From: Masud Hasan (mashasan) Date: Thu, 2 Dec 2021 19:25:39 +0000 (+0000) Subject: Pull request #3179: Stream splitter c X-Git-Tag: 3.1.19.0~19 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5aed3b65f21b4f81945cfe6f7e7bc134d8ea2df8;p=thirdparty%2Fsnort3.git Pull request #3179: Stream splitter c Merge in SNORT/snort3 from ~SMINUT/snort3:stream_splitter_c to master Squashed commit of the following: commit 2b537e6d3946a89abf9287644d1fb834bff8c4cc Author: Silviu Minut Date: Fri Nov 19 14:40:53 2021 -0500 stream: add PKT_MORE_TO_FLUSH flag and use it in TcpReassembler::scan_data_post_ack() to signal AtomSplitter whether to flush or not commit 59c24cb2b51268496d5818d4ab27e2929503e6b9 Author: Silviu Minut Date: Mon Nov 15 14:37:59 2021 -0500 rpc: remove RpcSplitter altogether and use LogSplitter instead commit b46a53d6200460ee1de5bd2f7531b729fce63fc6 Author: Silviu Minut Date: Fri Nov 5 09:21:33 2021 -0400 stream: fix issue with atom splitter not returning FLUSH commit 057931ddd0a9a85d4f8316cdb843113e82031774 Author: russ Date: Mon Oct 25 08:48:42 2021 -0400 stream_tcp: remove unnecessary special adjustment methods --- diff --git a/src/protocols/packet.h b/src/protocols/packet.h index a40ae23a3..1076e4138 100644 --- a/src/protocols/packet.h +++ b/src/protocols/packet.h @@ -84,7 +84,10 @@ class SFDAQInstance; #define PKT_HAS_PARENT 0x08000000 /* derived pseudo packet from current wire packet */ #define PKT_WAS_SET 0x10000000 /* derived pseudo packet (PDU) from current wire packet */ -#define PKT_UNUSED_FLAGS 0xE0000000 + +#define PKT_MORE_TO_FLUSH 0x20000000 /* when more data is available to StreamSplitter::scan */ + +#define PKT_UNUSED_FLAGS 0xC0000000 #define PKT_TS_OFFLOADED 0x01 diff --git a/src/service_inspectors/rpc_decode/rpc_decode.cc b/src/service_inspectors/rpc_decode/rpc_decode.cc index 458bc102f..6cc18550a 100644 --- a/src/service_inspectors/rpc_decode/rpc_decode.cc +++ b/src/service_inspectors/rpc_decode/rpc_decode.cc @@ -751,53 +751,6 @@ static int ConvertRPC(RpcSsnData* rsdata, Packet* p) return 0; } -//------------------------------------------------------------------------- -// splitter stuff: -// -// see above comments on MIN_CALL_BODY_SZ -// why flush_point == 28 instead of 32 IDK -// -// we don't set a flush point to flush_point (= 28 above) because that will -// cause the request to be segmented at that point. -// -// by setting max instead, we get the actual tcp segment(s) that total 32 -// or more bytes which is closer to the old set_flush_point() result (2 or -// more segments totaling at least 28 bytes) -// -// obviously, the correct way to do this is to look at the actual data and -// extract/determine the actual PDU lengths. TBD -//------------------------------------------------------------------------- - -class RpcSplitter : public StreamSplitter -{ -public: - RpcSplitter(bool c2s) : StreamSplitter(c2s) { } - - Status scan(Packet*, const uint8_t*, uint32_t len, - uint32_t, uint32_t* fp) override - { - - bytes_scanned += len; - if ( bytes_scanned < max(nullptr) ) - return SEARCH; - - *fp = len; - return FLUSH; - } - - unsigned max(Flow*) override - { return MIN_CALL_BODY_SZ; } - - // FIXIT-M this limits rpc flushes to 32 bytes per pdu, is that what we want? - unsigned adjust_to_fit(unsigned len) override - { - if ( len > max(nullptr) ) - return max(nullptr); - - return len; - } -}; - //------------------------------------------------------------------------- // class stuff //------------------------------------------------------------------------- @@ -813,7 +766,7 @@ public: bool get_buf(InspectionBuffer::Type, Packet*, InspectionBuffer&) override; StreamSplitter* get_splitter(bool c2s) override - { return c2s ? new RpcSplitter(c2s) : nullptr; } + { return c2s ? new LogSplitter(c2s) : nullptr; } }; RpcDecode::RpcDecode(RpcDecodeModule*) diff --git a/src/stream/stream_splitter.cc b/src/stream/stream_splitter.cc index e1938c100..721c66f98 100644 --- a/src/stream/stream_splitter.cc +++ b/src/stream/stream_splitter.cc @@ -71,37 +71,23 @@ AtomSplitter::AtomSplitter(bool b, uint16_t sz) : StreamSplitter(b) min = base + get_flush_bucket_size(); } -unsigned AtomSplitter::adjust_to_fit(unsigned len) -{ - return std::min(SnortConfig::get_conf()->max_pdu - bytes_scanned, len); -} - StreamSplitter::Status AtomSplitter::scan( - Packet*, const uint8_t*, uint32_t len, uint32_t, uint32_t* fp) + Packet*, const uint8_t*, uint32_t len, uint32_t flags, uint32_t* fp) { bytes_scanned += len; segs++; - if ( bytes_scanned < scan_footprint - && bytes_scanned < SnortConfig::get_conf()->max_pdu ) - return SEARCH; - - if ( segs >= 2 && bytes_scanned >= min ) + if ( segs >= 2 && bytes_scanned >= min && !(flags & PKT_MORE_TO_FLUSH) ) { *fp = len; + reset(); return FLUSH; } return SEARCH; } void AtomSplitter::reset() -{ segs = scan_footprint = bytes_scanned = 0; } - -void AtomSplitter::update() -{ - reset(); - min = base + get_flush_bucket_size(); -} +{ segs = bytes_scanned = 0; } //-------------------------------------------------------------------------- // log splitter diff --git a/src/stream/stream_splitter.h b/src/stream/stream_splitter.h index 89b934c2a..e057b29fb 100644 --- a/src/stream/stream_splitter.h +++ b/src/stream/stream_splitter.h @@ -82,15 +82,6 @@ public: virtual bool is_paf() { return false; } virtual unsigned max(Flow* = nullptr); - virtual unsigned adjust_to_fit(unsigned len) { return len; } - virtual void update() - { - scan_footprint = 0; - bytes_scanned = 0; - } - - void set_scan_footprint(unsigned fp) - { scan_footprint = fp; } bool to_server() { return c2s; } bool to_client() { return !c2s; } @@ -98,7 +89,6 @@ public: protected: StreamSplitter(bool b) : c2s(b) { } uint16_t get_flush_bucket_size(); - unsigned scan_footprint = 0; unsigned bytes_scanned = 0; private: @@ -114,8 +104,6 @@ public: AtomSplitter(bool, uint16_t size = 0); Status scan(Packet*, const uint8_t*, uint32_t, uint32_t, uint32_t*) override; - unsigned adjust_to_fit(unsigned len) override; - void update() override; private: void reset(); diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc index 0b0d64898..e0d704b5a 100644 --- a/src/stream/tcp/tcp_reassembler.cc +++ b/src/stream/tcp/tcp_reassembler.cc @@ -96,6 +96,17 @@ bool TcpReassembler::next_no_gap(const TcpSegmentNode& tsn) return tsn.next and (tsn.next->i_seq == tsn.i_seq + tsn.i_len); } +bool TcpReassembler::next_no_gap_c(const TcpSegmentNode& tsn) +{ + return tsn.next and (tsn.next->c_seq == tsn.c_seq + tsn.c_len); +} + +bool TcpReassembler::next_acked_no_gap_c(const TcpSegmentNode& tsn, const TcpReassemblerState& trs) +{ + return tsn.next and (tsn.next->c_seq == tsn.c_seq + tsn.c_len) + and SEQ_LT(tsn.next->c_seq, trs.tracker->r_win_base); +} + void TcpReassembler::update_next(TcpReassemblerState& trs, const TcpSegmentNode& tsn) { trs.sos.seglist.cur_rseg = next_no_gap(tsn) ? tsn.next : nullptr; @@ -549,10 +560,6 @@ int TcpReassembler::flush_to_seq( last_pdu = nullptr; } - // FIXIT-L must check because above may clear trs.sos.session - if ( trs.tracker->get_splitter() ) - trs.tracker->get_splitter()->update(); - // FIXIT-L abort should be by PAF callback only since recovery may be possible if ( trs.tracker->get_tf_flags() & TF_MISSING_PKT ) { @@ -585,9 +592,6 @@ int TcpReassembler::do_zero_byte_flush(TcpReassemblerState& trs, Packet* p, uint show_rebuilt_packet(trs, pdu); Analyzer::get_local_analyzer()->inspect_rebuilt(pdu); - - if ( trs.tracker->get_splitter() ) - trs.tracker->get_splitter()->update(); } return bytes_copied; @@ -691,9 +695,9 @@ int TcpReassembler::flush_stream( uint32_t bytes = 0; if ( trs.tracker->normalizer.is_tcp_ips_enabled() ) - bytes = get_q_sequenced(trs); + bytes = get_q_sequenced(trs); // num bytes in pre-ack mode else - bytes = get_q_footprint(trs); + bytes = get_q_footprint(trs); // num bytes in post-ack mode if ( bytes ) return flush_to_seq(trs, bytes, p, dir); @@ -852,6 +856,10 @@ int32_t TcpReassembler::scan_data_pre_ack(TcpReassemblerState& trs, uint32_t* fl continue; } + if ( next_no_gap_c(*tsn) ) + *flags |= PKT_MORE_TO_FLUSH; + else + *flags &= ~PKT_MORE_TO_FLUSH; int32_t flush_pt = paf_check( trs.tracker->get_splitter(), &trs.paf_state, p, tsn->payload(), tsn->c_len, total, tsn->c_seq, flags); @@ -928,13 +936,6 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f trs.sos.seglist.cur_rseg = trs.sos.seglist.cur_sseg; StreamSplitter* splitter = trs.tracker->get_splitter(); - if ( !splitter->is_paf() ) - { - // init splitter with current length of in sequence bytes.. - int32_t footprint = trs.tracker->r_win_base - trs.sos.seglist_base_seq; - if ( footprint > 0 ) - splitter->set_scan_footprint(footprint); - } uint32_t total = 0; TcpSegmentNode* tsn = trs.sos.seglist.cur_sseg; @@ -956,15 +957,20 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f while (tsn && *flags && SEQ_LT(tsn->c_seq, trs.tracker->r_win_base)) { // only flush acked data that fits in pdu reassembly buffer... - uint32_t flush_len; uint32_t end = tsn->c_seq + tsn->c_len; + uint32_t flush_len; + if ( SEQ_GT(end, trs.tracker->r_win_base)) - flush_len = splitter->adjust_to_fit(trs.tracker->r_win_base - tsn->c_seq); + flush_len = trs.tracker->r_win_base - tsn->c_seq; else - flush_len = splitter->adjust_to_fit(tsn->c_len); + flush_len = tsn->c_len; total += flush_len; + if ( next_acked_no_gap_c(*tsn, trs) ) + *flags |= PKT_MORE_TO_FLUSH; + else + *flags &= ~PKT_MORE_TO_FLUSH; int32_t flush_pt = paf_check(splitter, &trs.paf_state, p, tsn->payload(), flush_len, total, tsn->c_seq, flags); diff --git a/src/stream/tcp/tcp_reassembler.h b/src/stream/tcp/tcp_reassembler.h index 077987612..f6400e8c3 100644 --- a/src/stream/tcp/tcp_reassembler.h +++ b/src/stream/tcp/tcp_reassembler.h @@ -86,6 +86,8 @@ protected: void purge_to_seq(TcpReassemblerState&, uint32_t flush_seq); bool next_no_gap(const TcpSegmentNode&); + bool next_no_gap_c(const TcpSegmentNode&); + bool next_acked_no_gap_c(const TcpSegmentNode&, const TcpReassemblerState&); void update_next(TcpReassemblerState&, const TcpSegmentNode&); uint32_t perform_partial_flush(TcpReassemblerState&, snort::Packet*, uint32_t flushed = 0); }; diff --git a/src/stream/test/CMakeLists.txt b/src/stream/test/CMakeLists.txt index a5b7eb976..acd037a50 100644 --- a/src/stream/test/CMakeLists.txt +++ b/src/stream/test/CMakeLists.txt @@ -1,4 +1,5 @@ add_cpputest( stream_splitter_test - SOURCES ../stream_splitter.cc + SOURCES + ../stream_splitter.cc ) diff --git a/src/stream/test/stream_splitter_test.cc b/src/stream/test/stream_splitter_test.cc index 877f97086..a6a28e69b 100644 --- a/src/stream/test/stream_splitter_test.cc +++ b/src/stream/test/stream_splitter_test.cc @@ -75,48 +75,36 @@ uint16_t FlushBucket::get_size() TEST_GROUP(atom_splitter) { }; -TEST(atom_splitter, t2x256) +TEST(atom_splitter, search_search_flush) { - AtomSplitter s(true, 128); + AtomSplitter s(true, 16384); // default TcpStreamConfig::paf_max uint32_t fp = 0; - CHECK(s.scan(nullptr, nullptr, 256, 0, &fp) == StreamSplitter::SEARCH); + // not enough segments, not enough bytes - search + CHECK(s.scan(nullptr, nullptr, 16000, 0, &fp) == StreamSplitter::SEARCH); CHECK(fp == 0); - CHECK(s.scan(nullptr, nullptr, 256, 0, &fp) == StreamSplitter::FLUSH); - CHECK(fp == 256); -} - -TEST(atom_splitter, t3x64) -{ - AtomSplitter s(true, 128); - uint32_t fp = 0; - - CHECK(s.scan(nullptr, nullptr, 64, 0, &fp) == StreamSplitter::SEARCH); + // enough segments, not enough bytes - search + CHECK(s.scan(nullptr, nullptr, 300, 0, &fp) == StreamSplitter::SEARCH); CHECK(fp == 0); - CHECK(s.scan(nullptr, nullptr, 64, 0, &fp) == StreamSplitter::SEARCH); - CHECK(fp == 0); - - CHECK(s.scan(nullptr, nullptr, 64, 0, &fp) == StreamSplitter::FLUSH); - CHECK(fp == 64); + // enough segments and enough bytes - flush + CHECK(s.scan(nullptr, nullptr, 512, 0, &fp) == StreamSplitter::FLUSH); + CHECK(fp == 512); } -TEST(atom_splitter, t3x256_with_update) +TEST(atom_splitter, search_flush) { - AtomSplitter s(true, 128); + AtomSplitter s(true, 16384); // default TcpStreamConfig::paf_max uint32_t fp = 0; - CHECK(s.scan(nullptr, nullptr, 256, 0, &fp) == StreamSplitter::SEARCH); - CHECK(fp == 0); - - s.update(); - - CHECK(s.scan(nullptr, nullptr, 256, 0, &fp) == StreamSplitter::SEARCH); + // not enough segments, enough bytes - search + CHECK(s.scan(nullptr, nullptr, 17000, 0, &fp) == StreamSplitter::SEARCH); CHECK(fp == 0); - CHECK(s.scan(nullptr, nullptr, 256, 0, &fp) == StreamSplitter::FLUSH); - CHECK(fp == 256); + // enough segments and enough bytes - flush + CHECK(s.scan(nullptr, nullptr, 10, 0, &fp) == StreamSplitter::FLUSH); + CHECK(fp == 10); } //--------------------------------------------------------------------------