From: Cynthia Leonard (cyleonar) Date: Fri, 5 Jun 2020 16:54:16 +0000 (+0000) Subject: Merge pull request #2243 in SNORT/snort3 from ~CYLEONAR/snort3:master to master X-Git-Tag: 3.0.1-5~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=59cfcb6ac6564174bdb9673a3f32e65607fcebc3;p=thirdparty%2Fsnort3.git Merge pull request #2243 in SNORT/snort3 from ~CYLEONAR/snort3:master to master Squashed commit of the following: commit b2403b08ffe3bba0d23569f5b7a973943481e689 Author: Cynthia Leonard Date: Fri Jun 5 12:45:15 2020 -0400 Revert "Merge pull request #2017 in SNORT/snort3 from ~SUNIMUKH/snort3:drop_servicability to master" This reverts commit 0ab74bbcee6d23bbb9e136bfaf796230f1252cdb. --- diff --git a/src/actions/actions.cc b/src/actions/actions.cc index 2f28c9aea..357890890 100644 --- a/src/actions/actions.cc +++ b/src/actions/actions.cc @@ -121,14 +121,12 @@ void Actions::execute(Actions::Type action, Packet* p, const OptTreeNode* otn, case Actions::DROP: p->active->drop_packet(p); - p->active->set_drop_reason("ips"); alert(p, otn); SetTags(p, otn, event_id); break; case Actions::BLOCK: p->active->block_session(p); - p->active->set_drop_reason("ips"); alert(p, otn); SetTags(p, otn, event_id); break; diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc index a64397b75..675472761 100644 --- a/src/file_api/file_cache.cc +++ b/src/file_api/file_cache.cc @@ -274,13 +274,11 @@ bool FileCache::apply_verdict(Packet* p, FileContext* file_ctx, FileVerdict verd case FILE_VERDICT_BLOCK: // can't block session inside a session act->set_delayed_action(Active::ACT_BLOCK, true); - act->set_drop_reason("file"); break; case FILE_VERDICT_REJECT: // can't reset session inside a session act->set_delayed_action(Active::ACT_RESET, true); - act->set_drop_reason("file"); break; case FILE_VERDICT_STOP_CAPTURE: file_ctx->stop_file_capture(); diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc index 54ddaefd9..4b8514427 100644 --- a/src/file_api/file_lib.cc +++ b/src/file_api/file_lib.cc @@ -349,13 +349,6 @@ void FileContext::finish_signature_lookup(Packet* p, bool final_lookup, FilePoli FileCache* file_cache = FileService::get_file_cache(); if (file_cache) file_cache->apply_verdict(p, this, verdict, false, policy); - - if ( PacketTracer::is_active() and ( verdict == FILE_VERDICT_BLOCK - or verdict == FILE_VERDICT_REJECT )) - { - PacketTracer::log("File: signature lookup verdict %s\n", - verdict == FILE_VERDICT_BLOCK ? "block" : "reject"); - } log_file_event(flow, policy); config_file_signature(false); file_stats->signatures_processed[get_file_type()][get_file_direction()]++; @@ -457,13 +450,6 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, FileCache* file_cache = FileService::get_file_cache(); if (file_cache) file_cache->apply_verdict(p, this, v, false, policy); - - if ( PacketTracer::is_active() and ( v == FILE_VERDICT_BLOCK - or v == FILE_VERDICT_REJECT )) - { - PacketTracer::log("File: file type verdict %s\n", - v == FILE_VERDICT_BLOCK ? "block" : "reject"); - } } log_file_event(flow, policy); diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index 31bddb9e8..3735a7519 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -470,10 +470,7 @@ unsigned FlowControl::process(Flow* flow, Packet* p) else p->active->block_again(); - p->active->set_drop_reason("session"); DetectionEngine::disable_all(p); - if ( PacketTracer::is_active() ) - PacketTracer::log("Session: session has been blocked, drop\n"); break; case Flow::FlowState::RESET: @@ -483,10 +480,7 @@ unsigned FlowControl::process(Flow* flow, Packet* p) p->active->reset_again(); Stream::blocked_flow(p); - p->active->set_drop_reason("session"); DetectionEngine::disable_all(p); - if ( PacketTracer::is_active() ) - PacketTracer::log("Session: session has been reset\n"); break; } diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 7b1005c62..db8f0a1d2 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -54,12 +54,12 @@ THREAD_LOCAL bool Active::s_suspend = false; THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; void Active::drop_packet(snort::Packet const*, bool) { } +PacketTracer::PacketTracer() { } PacketTracer::~PacketTracer() { } void PacketTracer::log(const char*, ...) { } void PacketTracer::open_file() { } void PacketTracer::dump_to_daq(Packet*) { } void PacketTracer::reset() { } -void Active::set_drop_reason(char const*) { } Packet::Packet(bool) { } Packet::~Packet() { } Flow::Flow() { memset(this, 0, sizeof(*this)); } diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index 5cc2d97f2..da1fc84bb 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -54,12 +54,12 @@ THREAD_LOCAL bool Active::s_suspend = false; THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; void Active::drop_packet(snort::Packet const*, bool) { } +PacketTracer::PacketTracer() = default; PacketTracer::~PacketTracer() = default; void PacketTracer::log(const char*, ...) { } void PacketTracer::open_file() { } void PacketTracer::dump_to_daq(Packet*) { } void PacketTracer::reset() { } -void Active::set_drop_reason(char const*) { } Packet::Packet(bool) { } Packet::~Packet() = default; FlowCache::FlowCache(const FlowCacheConfig& cfg) : config(cfg) { } diff --git a/src/main/CMakeLists.txt b/src/main/CMakeLists.txt index c5eaed25e..d1d61cd96 100644 --- a/src/main/CMakeLists.txt +++ b/src/main/CMakeLists.txt @@ -2,7 +2,6 @@ set (INCLUDES analyzer_command.h policy.h - snort.h snort_config.h snort_debug.h snort_types.h diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index dcc89bf67..37105a1ec 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -392,9 +392,6 @@ void Analyzer::post_process_daq_pkt_msg(Packet* p) DataBus::publish(FINALIZE_PACKET_EVENT, event); } - if (verdict == DAQ_VERDICT_BLOCK or verdict == DAQ_VERDICT_BLACKLIST) - p->active->send_reason_to_daq(*p); - p->pkth = nullptr; // No longer avail after finalize_message. { diff --git a/src/main/test/distill_verdict_test.cc b/src/main/test/distill_verdict_test.cc index f4d106df2..3e16dad49 100644 --- a/src/main/test/distill_verdict_test.cc +++ b/src/main/test/distill_verdict_test.cc @@ -26,7 +26,6 @@ #include "stubs.h" #include "main/analyzer.h" -#include "packet_io/sfdaq_instance.h" #include #include diff --git a/src/main/test/stubs.h b/src/main/test/stubs.h index 7c3581a76..75f564d42 100644 --- a/src/main/test/stubs.h +++ b/src/main/test/stubs.h @@ -152,7 +152,6 @@ bool SFDAQ::forwarding_packet(const DAQ_PktHdr_t*) { return false; } int SFDAQ::inject(DAQ_Msg_h, int, const uint8_t*, uint32_t) { return -1; } bool SFDAQ::can_inject() { return false; } bool SFDAQ::can_inject_raw() { return false; } -int SFDAQInstance::set_packet_verdict_reason(DAQ_Msg_h, uint8_t) { return 0; } DetectionEngine::DetectionEngine() { } DetectionEngine::~DetectionEngine() { } void DetectionEngine::onload() { } diff --git a/src/network_inspectors/packet_tracer/packet_tracer.cc b/src/network_inspectors/packet_tracer/packet_tracer.cc index 59e51dc97..a027f90bb 100644 --- a/src/network_inspectors/packet_tracer/packet_tracer.cc +++ b/src/network_inspectors/packet_tracer/packet_tracer.cc @@ -31,7 +31,6 @@ #include "detection/ips_context.h" #include "log/log.h" #include "log/messages.h" -#include "packet_io/active.h" #include "packet_io/sfdaq_instance.h" #include "protocols/eth.h" #include "protocols/icmp4.h" @@ -50,6 +49,11 @@ using namespace snort; // static variables // ----------------------------------------------------------------------------- +static const uint8_t VERDICT_REASON_NO_BLOCK = 2; /* Not blocking packet; all enum defined after this indicates blocking */ + +// FIXIT-M currently non-thread-safe accesses being done in packet threads against this +static std::unordered_map reasons = { {VERDICT_REASON_NO_BLOCK, PacketTracer::PRIORITY_UNSET} }; + // FIXIT-M refactor the way this is used so all methods are members called against this pointer THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; @@ -63,6 +67,12 @@ static bool config_status = false; // static functions // ----------------------------------------------------------------------------- +void PacketTracer::register_verdict_reason(uint8_t reason_code, uint8_t priority) +{ + assert(reasons.find(reason_code) == reasons.end()); + reasons[reason_code] = priority; +} + void PacketTracer::set_log_file(const std::string& file) { log_file = file; } @@ -110,18 +120,18 @@ void PacketTracer::dump(Packet* p) if (s_pkt_trace->daq_activated) s_pkt_trace->dump_to_daq(p); - if ((s_pkt_trace->buff_len > 0) - and (s_pkt_trace->user_enabled or s_pkt_trace->shell_enabled)) - { - const char* drop_reason = p->active->get_drop_reason(); - if (drop_reason) - PacketTracer::log("Verdict Reason: %s\n", drop_reason); + if (s_pkt_trace->user_enabled or s_pkt_trace->shell_enabled) LogMessage(s_pkt_trace->log_fh, "%s\n", s_pkt_trace->buffer); - } s_pkt_trace->reset(); } +void PacketTracer::set_reason(uint8_t reason) +{ + if ( reasons[reason] > reasons[s_pkt_trace->reason] ) + s_pkt_trace->reason = reason; +} + void PacketTracer::log(const char* format, ...) { if (is_paused()) @@ -230,6 +240,10 @@ void PacketTracer::activate(const Packet& p) // non-static functions // ----------------------------------------------------------------------------- +// constructor +PacketTracer::PacketTracer() +{ reason = VERDICT_REASON_NO_BLOCK; } + // destructor PacketTracer::~PacketTracer() { @@ -391,7 +405,7 @@ void PacketTracer::open_file() void PacketTracer::dump_to_daq(Packet* p) { assert(p); - p->daq_instance->set_packet_trace_data(p->daq_msg, + p->daq_instance->modify_flow_pkt_trace(p->daq_msg, reason, (uint8_t *)buffer, buff_len + 1); } @@ -399,10 +413,12 @@ void PacketTracer::reset() { buff_len = 0; buffer[0] = '\0'; + reason = VERDICT_REASON_NO_BLOCK; for ( unsigned i = 0; i < mutes.size(); i++ ) mutes[i] = false; } + // -------------------------------------------------------------------------- // unit tests // -------------------------------------------------------------------------- @@ -414,6 +430,8 @@ void PacketTracer::reset() class TestPacketTracer : public PacketTracer { public: + uint8_t dump_reason = VERDICT_REASON_NO_BLOCK; + static void thread_init() { PacketTracer::_thread_init(); } @@ -438,8 +456,14 @@ public: static bool is_paused() { return ((TestPacketTracer*)s_pkt_trace)->pause_count; } + static uint8_t get_reason() + { return ((TestPacketTracer*)s_pkt_trace)->reason; } + + static uint8_t get_dump_reason() + { return ((TestPacketTracer*)s_pkt_trace)->dump_reason; } + void dump_to_daq(Packet*) override - { } + { dump_reason = reason; } static std::vector get_mutes() { return ((TestPacketTracer*)s_pkt_trace)->mutes; } @@ -571,6 +595,42 @@ TEST_CASE("pause", "[PacketTracer]") TestPacketTracer::thread_term(); } +TEST_CASE("reasons", "[PacketTracer]") +{ + TestPacketTracer::thread_init(); + TestPacketTracer::set_daq_enable(true); + uint8_t low1 = 100, low2 = 101, high = 102; + TestPacketTracer::register_verdict_reason(low1, PacketTracer::PRIORITY_LOW); + TestPacketTracer::register_verdict_reason(low2, PacketTracer::PRIORITY_LOW); + TestPacketTracer::register_verdict_reason(high, PacketTracer::PRIORITY_HIGH); + + // Init + CHECK((TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK)); + + // Update + TestPacketTracer::set_reason(low1); + CHECK((TestPacketTracer::get_reason() == low1)); + + // Don't update if already set + TestPacketTracer::set_reason(VERDICT_REASON_NO_BLOCK); + CHECK((TestPacketTracer::get_reason() == low1)); + TestPacketTracer::set_reason(low2); + CHECK((TestPacketTracer::get_reason() == low1)); + + // Always update for high priority + TestPacketTracer::set_reason(high); + CHECK((TestPacketTracer::get_reason() == high)); + + // Dump resets reason + TestPacketTracer::dump(nullptr); + CHECK((TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK)); + + // Dump delivers reason to daq + CHECK((TestPacketTracer::get_dump_reason() == high)); + + TestPacketTracer::thread_term(); +} + TEST_CASE("verbosity", "[PacketTracer]") { TestPacketTracer::thread_init(); diff --git a/src/network_inspectors/packet_tracer/packet_tracer.h b/src/network_inspectors/packet_tracer/packet_tracer.h index 413cb3514..16fef4ff4 100644 --- a/src/network_inspectors/packet_tracer/packet_tracer.h +++ b/src/network_inspectors/packet_tracer/packet_tracer.h @@ -44,7 +44,14 @@ struct Packet; class PacketTracer { public: - PacketTracer() = default; + enum VerdictPriority : uint8_t + { + PRIORITY_UNSET = 0, + PRIORITY_LOW = 1, + PRIORITY_HIGH = 2 + }; + + PacketTracer(); virtual ~PacketTracer(); typedef uint8_t TracerMute; @@ -69,6 +76,8 @@ public: static SO_PUBLIC TracerMute get_mute(); + static SO_PUBLIC void register_verdict_reason(uint8_t reason_code, uint8_t priority); + static SO_PUBLIC void set_reason(uint8_t); static SO_PUBLIC void log(const char* format, ...) __attribute__((format (printf, 1, 2))); static SO_PUBLIC void log(TracerMute, const char* format, ...) __attribute__((format (printf, 2, 3))); @@ -80,6 +89,7 @@ protected: std::vector mutes; char buffer[max_buff_size]; unsigned buff_len = 0; + uint8_t reason; unsigned pause_count = 0; bool user_enabled = false; diff --git a/src/network_inspectors/reputation/reputation_inspect.cc b/src/network_inspectors/reputation/reputation_inspect.cc index 3fe290b12..d8f9e31c5 100644 --- a/src/network_inspectors/reputation/reputation_inspect.cc +++ b/src/network_inspectors/reputation/reputation_inspect.cc @@ -37,6 +37,8 @@ #include "reputation_parse.h" +#define VERDICT_REASON_REPUTATION 19 + using namespace snort; THREAD_LOCAL ProfileStats reputation_perf_stats; @@ -275,10 +277,12 @@ static void snort_reputation(ReputationConfig* config, Packet* p) // disable all preproc analysis and detection for this packet DetectionEngine::disable_all(p); act->block_session(p, true); - act->set_drop_reason("reputation"); reputationstats.blacklisted++; if (PacketTracer::is_active()) + { + PacketTracer::set_reason(VERDICT_REASON_REPUTATION); PacketTracer::log("Reputation: packet blacklisted, drop\n"); + } } else if (MONITORED_SRC == decision or MONITORED_DST == decision) @@ -449,6 +453,10 @@ static Module* mod_ctor() static void mod_dtor(Module* m) { delete m; } +static void reputation_init() +{ + PacketTracer::register_verdict_reason(VERDICT_REASON_REPUTATION, PacketTracer::PRIORITY_LOW); +} static Inspector* reputation_ctor(Module* m) { @@ -480,7 +488,7 @@ const InspectApi reputation_api = PROTO_BIT__ANY_IP, nullptr, // buffers nullptr, // service - nullptr, // pinit + reputation_init, // pinit nullptr, // pterm nullptr, // tinit nullptr, // tterm diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index a98f6d751..2b7cfa257 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -79,9 +79,6 @@ static THREAD_LOCAL ip_t* s_ipnet = nullptr; static THREAD_LOCAL send_t s_send = SFDAQ::inject; static ResetAction default_reset; -static int default_drop_reason_id = -1; - -static std::unordered_map drop_reason_id_map; //-------------------------------------------------------------------- // helpers @@ -778,7 +775,6 @@ void Active::reset() active_action = ACT_ALLOW; delayed_active_action = ACT_ALLOW; delayed_reject = nullptr; - drop_reason = nullptr; } void Active::clear_queue(Packet* p) @@ -796,37 +792,3 @@ void Active::execute(Packet* p) } } -void Active::set_default_drop_reason(uint8_t reason_id) -{ - default_drop_reason_id = reason_id; -} - -void Active::map_drop_reason_id(const char* verdict_reason, uint8_t id) -{ - drop_reason_id_map[verdict_reason] = id; -} - -void Active::set_drop_reason(const char* reason) -{ - if ( drop_reason == nullptr ) - drop_reason = reason; -} - -int Active::get_drop_reason_id() -{ - const auto iter = drop_reason_id_map.find(drop_reason); - if ( iter != drop_reason_id_map.end() ) - return iter->second; - - return default_drop_reason_id; -} - -void Active::send_reason_to_daq(Packet& p) -{ - if ( drop_reason == nullptr ) - return; - - int reason = get_drop_reason_id(); - if ( reason != -1 ) - p.daq_instance->set_packet_verdict_reason(p.daq_msg, reason); -} diff --git a/src/packet_io/active.h b/src/packet_io/active.h index 5c9ccc7fa..cc8e97e40 100644 --- a/src/packet_io/active.h +++ b/src/packet_io/active.h @@ -156,14 +156,6 @@ public: void reset(); - static void set_default_drop_reason(uint8_t reason_id); - static void map_drop_reason_id(const char* verdict_reason, uint8_t id); - void set_drop_reason(const char*); - void send_reason_to_daq(Packet&); - - const char* get_drop_reason() - { return drop_reason; } - private: static bool open(const char*); static void close(); @@ -177,8 +169,6 @@ private: void cant_drop(); - int get_drop_reason_id(); - private: static const char* act_str[ACT_MAX][AST_MAX]; static bool enabled; @@ -186,7 +176,6 @@ private: static THREAD_LOCAL bool s_suspend; int active_tunnel_bypass; - const char* drop_reason; bool prevent_trust_action; diff --git a/src/packet_io/sfdaq_instance.cc b/src/packet_io/sfdaq_instance.cc index 38735fc12..561dbaac3 100644 --- a/src/packet_io/sfdaq_instance.cc +++ b/src/packet_io/sfdaq_instance.cc @@ -317,21 +317,12 @@ int SFDAQInstance::modify_flow_opaque(DAQ_Msg_h msg, uint32_t opaque) return daq_instance_ioctl(instance, DIOCTL_SET_FLOW_OPAQUE, &d_sfo, sizeof(d_sfo)); } -int SFDAQInstance::set_packet_verdict_reason(DAQ_Msg_h msg, uint8_t verdict_reason) -{ - DIOCTL_SetPacketVerdictReason d_spvr; - - d_spvr.msg = msg; - d_spvr.verdict_reason = verdict_reason; - - return daq_instance_ioctl(instance, DIOCTL_SET_PACKET_VERDICT_REASON, &d_spvr, sizeof(d_spvr)); -} - -int SFDAQInstance::set_packet_trace_data(DAQ_Msg_h msg, uint8_t* buff, uint32_t buff_len) +int SFDAQInstance::modify_flow_pkt_trace(DAQ_Msg_h msg, uint8_t verdict_reason, uint8_t* buff, uint32_t buff_len) { DIOCTL_SetPacketTraceData d_sptd; d_sptd.msg = msg; + d_sptd.verdict_reason = verdict_reason; d_sptd.trace_data_len = buff_len; d_sptd.trace_data = buff; diff --git a/src/packet_io/sfdaq_instance.h b/src/packet_io/sfdaq_instance.h index d8b2249fa..19027cbbe 100644 --- a/src/packet_io/sfdaq_instance.h +++ b/src/packet_io/sfdaq_instance.h @@ -76,8 +76,8 @@ public: SO_PUBLIC int ioctl(DAQ_IoctlCmd cmd, void *arg, size_t arglen); SO_PUBLIC int modify_flow_opaque(DAQ_Msg_h, uint32_t opaque); - int set_packet_verdict_reason(DAQ_Msg_h msg, uint8_t verdict_reason); - int set_packet_trace_data(DAQ_Msg_h, uint8_t* buff, uint32_t buff_len); + int modify_flow_pkt_trace(DAQ_Msg_h, uint8_t verdict_reason, + uint8_t* buff, uint32_t buff_len); int add_expected(const Packet* ctrlPkt, const SfIp* cliIP, uint16_t cliPort, const SfIp* srvIP, uint16_t srvPort, IpProtocol, unsigned timeout_ms, unsigned /* flags */); diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.cc b/src/service_inspectors/dce_rpc/dce_smb_utils.cc index 7586b650f..bcaf0bd5a 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_utils.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_utils.cc @@ -30,7 +30,6 @@ #include "file_api/file_api.h" #include "main/snort.h" #include "main/snort_debug.h" -#include "network_inspectors/packet_tracer/packet_tracer.h" #include "packet_io/active.h" #include "utils/util.h" @@ -1445,7 +1444,6 @@ static void DCE2_SmbFinishFileBlockVerdict(DCE2_SmbSsnData* ssd) if ((verdict == FILE_VERDICT_BLOCK) || (verdict == FILE_VERDICT_REJECT)) { DCE2_SmbInjectDeletePdu(ssd->fb_ftracker); - DetectionEngine::get_current_packet()->active->set_drop_reason("smb"); } ssd->fb_ftracker = nullptr; @@ -1477,12 +1475,7 @@ static void DCE2_SmbFinishFileAPI(DCE2_SmbSsnData* ssd) FileVerdict verdict = DCE2_get_file_verdict(); if ((verdict == FILE_VERDICT_BLOCK) || (verdict == FILE_VERDICT_REJECT)) - { ssd->fb_ftracker = ftracker; - if (PacketTracer::is_active()) - PacketTracer::log("Dce2_smb: smb file verdict %s\n", - verdict == FILE_VERDICT_BLOCK ? "block" : "reject"); - } } } dce2_smb_stats.smb_files_processed++; @@ -1565,11 +1558,6 @@ static DCE2_Ret DCE2_SmbFileAPIProcess(DCE2_SmbSsnData* ssd, || (verdict == FILE_VERDICT_PENDING)) { ssd->fb_ftracker = ftracker; - if (verdict != FILE_VERDICT_PENDING and PacketTracer::is_active()) - { - PacketTracer::log("Dce2_smb: smb file verdict %s\n", - verdict == FILE_VERDICT_BLOCK ? "block" : "reject"); - } } } ftracker->ff_sequential_only = false; diff --git a/src/service_inspectors/ftp_telnet/ftp_data.cc b/src/service_inspectors/ftp_telnet/ftp_data.cc index a70fa93f1..45388297a 100644 --- a/src/service_inspectors/ftp_telnet/ftp_data.cc +++ b/src/service_inspectors/ftp_telnet/ftp_data.cc @@ -62,9 +62,6 @@ static void FTPDataProcess( if (data_ssn->packet_flags & FTPDATA_FLG_REST) { p->active->block_again(); - p->active->set_drop_reason("ftp"); - if (PacketTracer::is_active()) - PacketTracer::log("FTP: session reset, drop\n"); return; } diff --git a/src/stream/stream.cc b/src/stream/stream.cc index f9e2284de..430fd3100 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -33,7 +33,6 @@ #include "main/snort.h" #include "main/snort_config.h" #include "main/snort_debug.h" -#include "network_inspectors/packet_tracer/packet_tracer.h" #include "packet_io/active.h" #include "protocols/vlan.h" #include "stream/base/stream_module.h" @@ -181,12 +180,7 @@ void Stream::check_flow_closed(Packet* p) flow->set_state(Flow::FlowState::BLOCK); if ( !(p->packet_flags & PKT_STATELESS) ) - { drop_traffic(p, SSN_DIR_BOTH); - p->active->set_drop_reason("stream"); - if (PacketTracer::is_active()) - PacketTracer::log("Stream: pending block, drop\n"); - } flow->session_state &= ~STREAM_STATE_BLOCK_PENDING; } } @@ -325,10 +319,6 @@ void Stream::drop_flow(const Packet* p) if ( !(p->packet_flags & PKT_STATELESS) ) drop_traffic(p, SSN_DIR_BOTH); - - p->active->set_drop_reason("stream"); - if (PacketTracer::is_active()) - PacketTracer::log("Stream: session has been dropped\n"); } //------------------------------------------------------------------------- @@ -610,9 +600,6 @@ bool Stream::blocked_flow(Packet* p) DetectionEngine::disable_content(p); p->active->drop_packet(p); active_response(p, flow); - p->active->set_drop_reason("stream"); - if (PacketTracer::is_active()) - PacketTracer::log("Stream: session was already blocked\n"); return true; } return false; diff --git a/src/stream/tcp/tcp_event_logger.cc b/src/stream/tcp/tcp_event_logger.cc index fdeb83d7b..bbbab52cc 100644 --- a/src/stream/tcp/tcp_event_logger.cc +++ b/src/stream/tcp/tcp_event_logger.cc @@ -29,7 +29,6 @@ #include "detection/rules.h" #include "filters/sfrf.h" #include "main/snort_config.h" -#include "packet_tracer/packet_tracer.h" #include "tcp_module.h" @@ -39,37 +38,35 @@ struct tcp_event_sid { uint32_t event_id; uint32_t sid; - const char* event_description; }; // ffs returns 1 as bit position of lsb so event id array // has dummy entry for index 0 struct tcp_event_sid tcp_event_sids[] = { - { 0, 0, nullptr }, - { EVENT_SYN_ON_EST, STREAM_TCP_SYN_ON_EST, "SYN_ON_EST" }, - { EVENT_DATA_ON_SYN, STREAM_TCP_DATA_ON_SYN, "DATA_ON_SYN" }, - { EVENT_DATA_ON_CLOSED, STREAM_TCP_DATA_ON_CLOSED, "DATA_ON_CLOSED" }, - { EVENT_BAD_TIMESTAMP, STREAM_TCP_BAD_TIMESTAMP, "BAD_TIMESTAMP" }, - { EVENT_WINDOW_TOO_LARGE, STREAM_TCP_WINDOW_TOO_LARGE, "WINDOW_TOO_LARGE" }, - { EVENT_DATA_AFTER_RESET, STREAM_TCP_DATA_AFTER_RESET, "DATA_AFTER_RESET" }, - { EVENT_SESSION_HIJACK_CLIENT, STREAM_TCP_SESSION_HIJACKED_CLIENT, "SESSION_HIJACK_CLIENT" }, - { EVENT_SESSION_HIJACK_SERVER, STREAM_TCP_SESSION_HIJACKED_SERVER, "SESSION_HIJACK_SERVER" }, - { EVENT_DATA_WITHOUT_FLAGS, STREAM_TCP_DATA_WITHOUT_FLAGS, "DATA_WITHOUT_FLAGS" }, - { EVENT_4WHS, STREAM_TCP_4WAY_HANDSHAKE, "4WHS" }, - { EVENT_NO_TIMESTAMP, STREAM_TCP_NO_TIMESTAMP, "NO_TIMESTAMP" }, - { EVENT_BAD_RST, STREAM_TCP_BAD_RST, "BAD_RST" }, - { EVENT_BAD_FIN, STREAM_TCP_BAD_FIN, "BAD_FIN" }, - { EVENT_BAD_ACK, STREAM_TCP_BAD_ACK, "BAD_ACK" }, - { EVENT_DATA_AFTER_RST_RCVD, STREAM_TCP_DATA_AFTER_RST_RCVD, "DATA_AFTER_RST_RCVD" }, - { EVENT_WINDOW_SLAM, STREAM_TCP_WINDOW_SLAM, "WINDOW_SLAM" }, - { EVENT_NO_3WHS, STREAM_TCP_NO_3WHS, "NO_3WHS" }, - { EVENT_BAD_SEGMENT, STREAM_TCP_BAD_SEGMENT, "BAD_SEGMENT" }, - { EVENT_EXCESSIVE_OVERLAP, STREAM_TCP_EXCESSIVE_TCP_OVERLAPS, "EXCESSIVE_OVERLAP" }, - { EVENT_MAX_SMALL_SEGS_EXCEEDED, STREAM_TCP_SMALL_SEGMENT, "MAX_SMALL_SEGS_EXCEEDED" }, - { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr }, - { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr }, - { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr }, { 0, 0, nullptr } + { 0, 0 }, + { EVENT_SYN_ON_EST, STREAM_TCP_SYN_ON_EST }, + { EVENT_DATA_ON_SYN, STREAM_TCP_DATA_ON_SYN }, + { EVENT_DATA_ON_CLOSED, STREAM_TCP_DATA_ON_CLOSED }, + { EVENT_BAD_TIMESTAMP, STREAM_TCP_BAD_TIMESTAMP }, + { EVENT_WINDOW_TOO_LARGE, STREAM_TCP_WINDOW_TOO_LARGE }, + { EVENT_DATA_AFTER_RESET, STREAM_TCP_DATA_AFTER_RESET }, + { EVENT_SESSION_HIJACK_CLIENT, STREAM_TCP_SESSION_HIJACKED_CLIENT }, + { EVENT_SESSION_HIJACK_SERVER, STREAM_TCP_SESSION_HIJACKED_SERVER }, + { EVENT_DATA_WITHOUT_FLAGS, STREAM_TCP_DATA_WITHOUT_FLAGS }, + { EVENT_4WHS, STREAM_TCP_4WAY_HANDSHAKE }, + { EVENT_NO_TIMESTAMP, STREAM_TCP_NO_TIMESTAMP }, + { EVENT_BAD_RST, STREAM_TCP_BAD_RST }, + { EVENT_BAD_FIN, STREAM_TCP_BAD_FIN }, + { EVENT_BAD_ACK, STREAM_TCP_BAD_ACK }, + { EVENT_DATA_AFTER_RST_RCVD, STREAM_TCP_DATA_AFTER_RST_RCVD }, + { EVENT_WINDOW_SLAM, STREAM_TCP_WINDOW_SLAM }, + { EVENT_NO_3WHS, STREAM_TCP_NO_3WHS }, + { EVENT_BAD_SEGMENT, STREAM_TCP_BAD_SEGMENT }, + { EVENT_EXCESSIVE_OVERLAP, STREAM_TCP_EXCESSIVE_TCP_OVERLAPS }, + { EVENT_MAX_SMALL_SEGS_EXCEEDED, STREAM_TCP_SMALL_SEGMENT }, + { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 }, + { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 } }; void TcpEventLogger::log_internal_event(uint32_t eventSid) @@ -88,11 +85,8 @@ void TcpEventLogger::log_tcp_events() uint32_t idx = ffs(tcp_events); if ( idx ) { - DetectionEngine::queue_event(GID_STREAM_TCP, tcp_event_sids[idx].sid); - if ( PacketTracer::is_active() ) - PacketTracer::log("Stream: TCP normalization error in %s\n", - tcp_event_sids[idx].event_description); - tcp_events ^= tcp_event_sids[idx].event_id; + DetectionEngine::queue_event(GID_STREAM_TCP, tcp_event_sids[ idx ].sid); + tcp_events ^= tcp_event_sids[ idx ].event_id; tcpStats.events++; } } diff --git a/src/stream/tcp/tcp_normalizer.cc b/src/stream/tcp/tcp_normalizer.cc index 961981091..3c59f8531 100644 --- a/src/stream/tcp/tcp_normalizer.cc +++ b/src/stream/tcp/tcp_normalizer.cc @@ -30,6 +30,7 @@ #include "tcp_stream_session.h" #include "tcp_stream_tracker.h" + using namespace snort; THREAD_LOCAL PegCount tcp_norm_stats[PC_TCP_MAX][NORM_MODE_MAX]; @@ -101,7 +102,6 @@ bool TcpNormalizer::packet_dropper( { Packet* p = tsd.get_pkt(); p->active->drop_packet(p); - p->active->set_drop_reason("stream"); return true; } diff --git a/src/stream/tcp/tcp_state_fin_wait2.cc b/src/stream/tcp/tcp_state_fin_wait2.cc index ce70698b4..3e1f42aee 100644 --- a/src/stream/tcp/tcp_state_fin_wait2.cc +++ b/src/stream/tcp/tcp_state_fin_wait2.cc @@ -66,8 +66,8 @@ bool TcpStateFinWait2::ack_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk { if ( SEQ_GT(tsd.get_seg_ack(), trk.get_snd_nxt() ) ) { - trk.session->tel.set_tcp_event(EVENT_BAD_ACK); trk.normalizer.packet_dropper(tsd, NORM_TCP_BLOCK); + trk.session->tel.set_tcp_event(EVENT_BAD_ACK); trk.session->set_pkt_action_flag(ACTION_BAD_PKT); } else @@ -88,8 +88,8 @@ bool TcpStateFinWait2::data_seg_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker { if ( SEQ_GT(tsd.get_seg_ack(), trk.get_snd_nxt() ) ) { - trk.session->tel.set_tcp_event(EVENT_BAD_ACK); trk.normalizer.packet_dropper(tsd, NORM_TCP_BLOCK); + trk.session->tel.set_tcp_event(EVENT_BAD_ACK); trk.session->set_pkt_action_flag(ACTION_BAD_PKT); } else diff --git a/src/stream/tcp/tcp_state_syn_recv.cc b/src/stream/tcp/tcp_state_syn_recv.cc index 72356671b..0f1806e1c 100644 --- a/src/stream/tcp/tcp_state_syn_recv.cc +++ b/src/stream/tcp/tcp_state_syn_recv.cc @@ -180,8 +180,8 @@ bool TcpStateSynRecv::rst_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk) else { inc_tcp_discards(); - trk.session->tel.set_tcp_event(EVENT_BAD_RST); trk.normalizer.packet_dropper(tsd, NORM_TCP_BLOCK); + trk.session->tel.set_tcp_event(EVENT_BAD_RST); } // FIXIT-L might be good to create alert specific to RST with data